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

Form not sent with same semantics as FormBody #333

Closed
Bilge opened this issue Jun 16, 2023 · 4 comments
Closed

Form not sent with same semantics as FormBody #333

Bilge opened this issue Jun 16, 2023 · 4 comments
Milestone

Comments

@Bilge
Copy link
Contributor

Bilge commented Jun 16, 2023

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
Copy link
Member

kelunik commented Jul 2, 2023

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

@kelunik kelunik added this to the 5.x milestone Jul 8, 2023
@Bilge
Copy link
Contributor Author

Bilge commented Aug 1, 2023

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

@Bilge
Copy link
Contributor Author

Bilge commented Aug 1, 2023

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
Copy link
Member

kelunik commented Aug 11, 2023

See also amphp/http#24.

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

Successfully merging a pull request may close this issue.

2 participants