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

GuzzleHttp\Psr7\Uri not encoding + (plus) symbol as expected #618

Open
edegaudenzi opened this issue Nov 15, 2024 · 1 comment
Open

GuzzleHttp\Psr7\Uri not encoding + (plus) symbol as expected #618

edegaudenzi opened this issue Nov 15, 2024 · 1 comment

Comments

@edegaudenzi
Copy link

edegaudenzi commented Nov 15, 2024

PHP version: x.y.z (hint: php --version)
8.3.6 (Ubuntu 24.04.1)
guzzlehttp/psr7 v2.7.0

Description
Trying to build a URI with this class the '+' (plus) symbol remains un-encoded, so that:
where=Name=="Guzzle+ Org"
becomes:
where=Name%3D%3D%22Guzzle+%20Org%22
instead of expected:
where=Name%3D%3D%22Guzzle%2B%20Org%22

How to reproduce

var_export(
    \GuzzleHttp\Psr7\Uri::withQueryValues(new \GuzzleHttp\Psr7\Uri('https://example.com/prefix/web/index.php'), ['where' => 'Name=="Guzzle+ Org"'])
);

Possible Solution
add '+' => '%2B' to private const QUERY_SEPARATORS_REPLACEMENT = ['=' => '%3D', '&' => '%26'];

Additional context
Issue with the solution is: the '+' symbol, technically, is a legal sub-delims symbol as RFC3986 calls them; the PHP function rawurlencode() used to perform this task is implementing RFC3986 so - correctly - by NOT encoding the '+' symbol.
Saying this, we then need to encode the '+' symbol BEFORE arriving to execute rawurlencode() (and theoretically not when used as sub-delimiter), pretty much as it already happens for '=' and '&': that's easy enough (see solution above).
Problem now is, will it affect any other existing logic? Is it a logic bomb?

@edegaudenzi
Copy link
Author

edegaudenzi commented Nov 20, 2024

I've moved a little forward, trying to make a preliminary study of what can be done to address this issue. I can see that the above-mentioned solution is actually pretty valid: adding the extra mapping '+' => '%2B' to private const QUERY_SEPARATORS_REPLACEMENT = ['=' => '%3D', '&' => '%26']; would not only solve the problem but it seems not affecting anything else.

The mechanism and the comments inside \GuzzleHttp\Psr7\Uri::generateQueryString() :

// Query string separators ("=", "&") within the key or value need to be encoded
// (while preventing double-encoding) before setting the query string. All other
// chars that need percent-encoding will be encoded by withQuery().

seem to suggest that a preliminary replacement is done before processing the parameters key and value individually via rawurlencode() (rawurlencode() is fed by a regex and will only replace what is not already encoded and non reserved chars). This means adding the + => %2B replacement should convert the content of the mentioned key and value contents correctly.

Do you guys have any thoughts on the topic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant