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

Handle parsing of form attributes with no value #10601

Merged
merged 3 commits into from
Mar 14, 2024
Merged

Conversation

jeremyg484
Copy link
Contributor

@jeremyg484 jeremyg484 commented 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.

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.
Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using a reactive HTTP Client for these tests?

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremyg484
Copy link
Contributor Author

Note that I have submitted a PR to Netty to fix the remaining cases that don't parse correctly: netty/netty#13904

Copy link

sonarcloud bot commented Mar 13, 2024

@sdelamo sdelamo merged commit 4a7e024 into 4.3.x Mar 14, 2024
15 checks passed
@sdelamo sdelamo deleted the issue-10446-partial-fix branch March 14, 2024 06:15
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 this pull request may close these issues.

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