-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix Regression on ee9 / ee8 MultiPart parsing #12031
Fix Regression on ee9 / ee8 MultiPart parsing #12031
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seams a very specific check/failure.
What if the body is a single byte? The EOF can come at many illegal points and why do we need/have special handling just for 0 bytes?
...e9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java
Outdated
Show resolved
Hide resolved
+ More multipart parsing errors return 400, not 500 now.
I've added more tests for this regression, and fixed many of the 500 vs 400 status returns in the process. (more are returning 400 now) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to hangout on this as I don't think I'm communicating my concern.
...e9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/MultiPartFormInputStream.java
Outdated
Show resolved
Hide resolved
* Request.getParts() catches IOExceptions from multiParts.getParts() and wraps them in a BadMessageException to ensure that a 400 Bad Request response shows up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In Jetty 10 / Jetty 11 when a
multipart/form-data
is sent with an empty request body, it would result in an error status 400 (Bad Request).The implementation in ee9 / ee8 in Jetty 12 returns an error status 500.
Introduce test cases and fix to restore this behavior.