Add no-value key handling only for form body #13998
Conversation
Motivation: This is a fix for issue netty#13981 that reports a changed behaviour of HttpPostStandardRequestDecoder after this PR - netty#13908 Because HttpPostStandardRequestDecoder changed the contract, some code implementations relying on certain parsing are failing Modification: This PR makes sure, that the edge case handling for form data happenes only when the content is in fact form data Result: Fixes netty#13981
.../src/test/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoderTest.java
Outdated
Show resolved
Hide resolved
...http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
Outdated
Show resolved
Hide resolved
...http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java
Outdated
Show resolved
Hide resolved
|
@gniadeck thanks! |
Motivation: This is a fix for issue #13981 that reports a changed behaviour of HttpPostStandardRequestDecoder after this PR - #13908 Because HttpPostStandardRequestDecoder changed the contract, some code implementations relying on certain parsing are failing Modification: This PR makes sure, that the edge case handling for form body happenes only when the content is in fact form body Result: Fixes #13981
### What changes were proposed in this pull request? The pr aims to upgrade `netty` from `4.1.109.Final` to `4.1.110.Final`. ### Why are the changes needed? - https://netty.io/news/2024/05/22/4-1-110-Final.html This version has brought some bug fixes and improvements, such as: Fix Zstd throws Exception on read-only volumes (netty/netty#13982) Add unix domain socket transport in netty 4.x via JDK16+ ([#13965](netty/netty#13965)) Backport #13075: Add the AdaptivePoolingAllocator ([#13976](netty/netty#13976)) Add no-value key handling only for form body ([#13998](netty/netty#13998)) Add support for specifying SecureRandom in SSLContext initialization ([#14058](netty/netty#14058)) - https://github.com/netty/netty/issues?q=milestone%3A4.1.110.Final+is%3Aclosed ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46744 from panbingkun/SPARK-48420. Authored-by: panbingkun <[email protected]> Signed-off-by: yangjie01 <[email protected]>
|
@gniadeck can I ask what the use case is, here? I'm working on rewriting the post body parsing ( netty-contrib/codec-multipart#25 ), so I'd like to understand whether I need to support non-standard parsing like this |
|
looking at the test for this PR, I'm not a big fan. It works by accident. Once your JSON has a |
|
@gniadeck Can you take a look at Jonas' comments? |
|
hi, sorry for taking long to response, and thanks for considering our use case in your contributions :) this fix was added simply to narrow the scope of the fix for multipart form body, to apply it only if the request has appropriate header. As I wrote in the original issue, when the bug was introduced, the JSON body was parsed as a name of You're right, that in the current implementation adding |
|
Yes, content sniffing is not the purpose of this class. |
4 participants
Motivation:
This is a fix for issue #13981 that reports a changed behaviour of HttpPostStandardRequestDecoder after this PR - #13908
Because HttpPostStandardRequestDecoder changed the contract, some code implementations relying on certain parsing are failing
Modification:
This PR makes sure, that the edge case handling for form body happenes only when the content is in fact form body
Result:
Fixes #13981