search

amphp / http-client

An advanced async HTTP client library for PHP, enabling efficient, non-blocking, and concurrent requests and responses.
https://amphp.org/http-client
MIT License
706 stars 66 forks source link

`Form` not sent with same semantics as `FormBody` #333

Closed Bilge closed 1 year ago

Bilge commented 1 year ago

I apologise I haven't been able to reduce this down to a simpler test case, but it seems very clear to me that something is not right when sending Form in beta 11 as with FormBody from the previous API iteration. I haven't been able to narrow down exactly what is different between the two requests, but I can reliably reproduce it with a test.

To test this, please try running SteamLoginTest::testSecureLoginCookie, which requires STEAM_USER and STEAM_PASSWORD to be set to a valid Steam username and password (you can get an account for free).

master will work, but switching over to connectors-v7, which has minimal changes, will fail on SteamLogin:99 with {"response":{"interval":5,"extended_error_message":""}}. Unfortunately this opaque error is all we have to work with.

kelunik commented 1 year ago

Some more info would be really helpful. Can you log the full bodies and do a diff there?

Bilge commented 1 year ago

I don't know where to capture the full body. Maybe I'll try some hardcore debugging later.

Bilge commented 1 year ago

OK I went hardcore, by placing a breakpoint deep within Http2ConnectionProcessor::request, and I found that the old (working) code would escape form field content whereas the new (broken) code does not.

For example, base64 characters such as + and / appear verbatim in the new, broken code whereas they are percent-encoded as %2B and %2F, respectively, in the old, working code. So that's probably the issue.

Note the headers are the same in both cases (in particular, content-type is application/x-www-form-urlencoded).

Moreover, note that I cannot manually escape the content because it will encode % signs to %25, causing them to become double-escaped and thus still fail.

kelunik commented 1 year ago

See also https://github.com/amphp/http/issues/24.