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

Parsing of application/x-www-form-urlencoded is broken, can result in Internal Server Error. #10446

Closed
marek-stanulewicz opened this issue Feb 2, 2024 · 2 comments · Fixed by #10601
Assignees

Comments

@marek-stanulewicz
Copy link

Expected Behavior

Parsing should conform to https://url.spec.whatwg.org/#application/x-www-form-urlencoded.

Actual Behaviour

Parsing does not support the case when a single sequence is just key. Moreover, if this key is followed by more sequences an exception is thrown and Internal Server Error is returned.

Steps To Reproduce

Just run the test.

Environment Information

Windows.
JDK Corretto 17.0.10.

Example Application

https://github.com/marek-stanulewicz/micronaut-issues/tree/www-form-urlencoding-parsing

Version

4.2.4

@marek-stanulewicz
Copy link
Author

It behaves differently than 3.10.1, which is even worse: https://github.com/marek-stanulewicz/micronaut-issues/tree/3.x-www-form-urlencoding-parsing

@sdelamo sdelamo assigned jeremyg484 and unassigned wetted Mar 8, 2024
jeremyg484 added a commit that referenced this issue Mar 12, 2024
MicronautHttpData.AttributeImpl is updated to provide an implementation of the setValue method.
This implementation is needed in the corner cases where Netty's HttpPostStandardRequestDecoder
successfully parses an attribute with no value such as in the body "a&b=2".

Tests are added to more thoroughly test the various forms of x-www-form-urlencoded bodies as
specified by https://url.spec.whatwg.org/#application/x-www-form-urlencoded. These tests
include scenarios where we know and expect that HttpPostStandardRequestDecoder will currently
fail to parse an attribute with a name but no value - namely when that attribute is the last
one in a given POST body. If HttpPostStandardRequestDecoder is patched in the future, then
these expected parsing failures can be moved into the success scenario instead.

This partially resolves #10446.
sdelamo added a commit that referenced this issue Mar 14, 2024
* Handle parsing of form attributes with no value

MicronautHttpData.AttributeImpl is updated to provide an implementation of the setValue method.
This implementation is needed in the corner cases where Netty's HttpPostStandardRequestDecoder
successfully parses an attribute with no value such as in the body "a&b=2".

Tests are added to more thoroughly test the various forms of x-www-form-urlencoded bodies as
specified by https://url.spec.whatwg.org/#application/x-www-form-urlencoded. These tests
include scenarios where we know and expect that HttpPostStandardRequestDecoder will currently
fail to parse an attribute with a name but no value - namely when that attribute is the last
one in a given POST body. If HttpPostStandardRequestDecoder is patched in the future, then
these expected parsing failures can be moved into the success scenario instead.

This partially resolves #10446.

* Use CharSequence version of Unpooled.copiedBuffer

* use BlockingHttpClient

---------

Co-authored-by: Sergio del Amo <sergio.delamo@softamo.com>
@jeremyg484
Copy link
Contributor

@marek-stanulewicz Thank you for the thorough reproducible tests - they were very helpful in getting this resolved.

Note that the change to Micronaut in #10601 is just a partial fix - no-value keys will work now except in cases where the key is the last in the POST body sequence. To fix that final case, we had to submit a patch to Netty - see netty/netty#13908. It looks like it should be in the 4.1.108 version of Netty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants