Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query param got stripped in request through proxy #384

Closed
SagarGi opened this issue Sep 16, 2024 · 8 comments · Fixed by #412
Closed

Query param got stripped in request through proxy #384

SagarGi opened this issue Sep 16, 2024 · 8 comments · Fixed by #412
Labels
bug Something isn't working

Comments

@SagarGi
Copy link

SagarGi commented Sep 16, 2024

Description

For a browser request :
http://localhost/stable300/index.php/apps/app_api/proxy/openproject-nextcloud-app/projects/demo-project/settings/project_storages/new?utf8=%E2%9C%93&storages_project_storage%5Bstorage_id%5D=2&button=

Through the proxy the storage_id seems to be stripped.

logs in proxy

ver': ('127.0.0.1', 9030), 'client': ('127.0.0.1', 37524), 'scheme': 'http', 'root_path': '', 'headers': '<...>', 'state': {}, 
'method': 'GET', 
'path': '/projects/demo-project/settings/project_storages/new', 
'raw_path': b'/projects/demo-project/settings/project_storages/new', 
'query_string': **b'storages_project_storage%5B%5D=2&utf8=%E2%9C%93&button='}**

Expected:

'query_string': **b'storages_project_storage%5Bstorage_id%5D=2&utf8=%E2%9C%93&button='

Actual:

'query_string': **b'storages_project_storage%5B%5D=2&utf8=%E2%9C%93&button='

Environment:

app_api version 3.2.0
nextcloud version 30.0.0 rc4

@SagarGi
Copy link
Author

SagarGi commented Sep 16, 2024

hi @andrey18106 not sure if this is a bug but i came across this thing.

@bigcat88
Copy link
Member

Sorry for the long delay. Is issue still valid?

If yes, we will schedule time to reproduce this, and if it confirmed as a bug, we will fix it of course.

@SagarGi
Copy link
Author

SagarGi commented Sep 30, 2024

Sorry for the long delay. Is issue still valid?

If yes, we will schedule time to reproduce this, and if it confirmed as a bug, we will fix it of course.

i can check again with the latest app_api and then confirm @bigcat88 if it still exists.

@SagarGi
Copy link
Author

SagarGi commented Sep 30, 2024

Hi @bigcat88 the issue still exists for version 5.0.0(master).

@edward-ly
Copy link
Contributor

edward-ly commented Oct 1, 2024

private function getUriEncodedParams(array $params): string {
$paramsContent = '';
foreach ($params as $key => $value) {
if (is_array($value)) {
foreach ($value as $oneArrayValue) {
$paramsContent .= $key . '[]=' . urlencode($oneArrayValue) . '&';
}
unset($params[$key]);
}
}
return $paramsContent . http_build_query($params);
}

It looks like the array keys are being stripped out of the query parameter values, although I don't recall what the purpose of this is when the http_build_query function itself is able to handle array parameters (including nested arrays) automatically. @bigcat88 or @andrey18106 do you have any ideas why we do this?

@SagarGi
Copy link
Author

SagarGi commented Oct 3, 2024

app_api/lib/Service/AppAPIService.php

Lines 261 to 272 in 2b37cec

private function getUriEncodedParams(array $params): string {
$paramsContent = '';
foreach ($params as $key => $value) {
if (is_array($value)) {
foreach ($value as $oneArrayValue) {
$paramsContent .= $key . '[]=' . urlencode($oneArrayValue) . '&';
}
unset($params[$key]);
}
}
return $paramsContent . http_build_query($params);
}
It looks like the array keys are being stripped out of the query parameter values, although I don't recall what the purpose of this is when the http_build_query function itself is able to handle array parameters (including nested arrays) automatically. @bigcat88 or @andrey18106 do you have any ideas why we do this?

yes the exact thing happened in my case. @edward-ly

@andrey18106 andrey18106 added the bug Something isn't working label Oct 3, 2024
@andrey18106
Copy link
Collaborator

app_api/lib/Service/AppAPIService.php

Lines 261 to 272 in 2b37cec
private function getUriEncodedParams(array $params): string {
$paramsContent = '';
foreach ($params as $key => $value) {
if (is_array($value)) {
foreach ($value as $oneArrayValue) {
$paramsContent .= $key . '[]=' . urlencode($oneArrayValue) . '&';
}
unset($params[$key]);
}
}
return $paramsContent . http_build_query($params);
}

It looks like the array keys are being stripped out of the query parameter values, although I don't recall what the purpose of this is when the http_build_query function itself is able to handle array parameters (including nested arrays) automatically. @bigcat88 or @andrey18106 do you have any ideas why we do this?

I think we are using the same algorithm in several integrations that wasn't changed for years.

@edward-ly
Copy link
Contributor

Hi @SagarGi, does applying the patch in the above PR fix the query string for you? If so, we can merge it and close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants