Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpClient] Fix over-encoding of URL parts to match browser's behavior#49299
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
kwisatz commentedMar 29, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
For what it's worth, this is breaking an API client to WooCommerce for us. With 5.4.20> GET /wp-json/wc/v3/orders?status%5B0%5D=processing&status%5B1%5D=completed&after=2023-03-29T10%3A35%3A09%2B02%3A00&before=2023-03-29T11%3A04%3A18%2B02%3A00 HTTP/2Host: gazon-naturel-en-rouleau.frpragma: No-cacheaccept:*/*authorization: Basic**********user-agent: Symfony HttpClient/Curlaccept-encoding: gzip* old SSL session ID is stale, removing* Connection state changed (MAX_CONCURRENT_STREAMS == 100)!< HTTP/2 200 With 5.4.21> GET /wp-json/wc/v3/orders?status[0]=processing&status[1]=completed&after=2023-03-29T10:35:09+02:00&before=2023-03-29T11:02:48+02:00 HTTP/2Host: gazon-naturel-en-rouleau.frpragma: No-cacheaccept:*/*authorization: Basic*************user-agent: Symfony HttpClient/Curlaccept-encoding: gzip* old SSL session ID is stale, removing* Connection state changed (MAX_CONCURRENT_STREAMS == 100)!< HTTP/2 400 This is how the request is constructed: $response =$client->request('GET',$this->currentShop['endpoint'], ['auth_basic' => [$this->currentShop['api_key'],$this->currentShop['api_secret'] ],'headers' => ['Pragma' =>'No-cache', ],'query' => ['status' => ['processing','completed'],'after' =>$dates['after']->modify('-5 minutes')->format('c'),'before' =>$dates['before']->modify('-5 minutes')->format('c'), ],'on_progress' =>function ()use ($downloadProgress):void {$downloadProgress->advance(); }, ]); |
nicolas-grekas commentedMar 29, 2023
Which part/character? You can know by generating the query string on your own to test. |
kwisatz commentedMar 29, 2023
It's the plus signs here. |
nicolas-grekas commentedMar 29, 2023
OK, so this is#49579, fixed in the next release. |
kwisatz commentedMar 29, 2023 via email
Indeed, I had seen that ticket actually but not made the connection, sorry. |
Kozzi11 commentedJun 15, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This has broken our aplication should not we stick confrontant to RFC? |
chalasr commentedJun 15, 2023
@Kozzi11 Please open a new issue if you think this introduced a bug. |
Kozzi11 commentedJun 15, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes, the RFC says these chars should be url-encoded:
But in practice, browser's don't encode them, and some servers don't expect them to be encoded either.
Seehttps://stackoverflow.com/questions/2366260/whats-valid-and-whats-not-in-a-uri-query for some pointers.