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
|
@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 <panbingkun@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>
|
@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? |
|
We can probably leave it like this but I won't port this PR to
contrib-multipart.
…On Wed, Mar 19, 2025, 22:58 Chris Vest ***@***.***> wrote:
@gniadeck <https://github.com/gniadeck> Can you take a look at Jonas'
comments?
—
Reply to this email directly, view it on GitHub
<#13998 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASIZILA5X4A37WTNNJZTN32VHSATAVCNFSM6AAAAABYGVZCWOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZYGI2TKMJZGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
[image: chrisvest]*chrisvest* left a comment (netty/netty#13998)
<#13998 (comment)>
@gniadeck <https://github.com/gniadeck> Can you take a look at Jonas'
comments?
—
Reply to this email directly, view it on GitHub
<#13998 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASIZILA5X4A37WTNNJZTN32VHSATAVCNFSM6AAAAABYGVZCWOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZYGI2TKMJZGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
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. |
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