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

Uploading big multipart files via jetty 12.0.5 with spring boot 3.2.1 cause problems #11310

Closed
duzenz opened this issue Jan 24, 2024 · 14 comments · Fixed by #11409
Closed

Uploading big multipart files via jetty 12.0.5 with spring boot 3.2.1 cause problems #11310

duzenz opened this issue Jan 24, 2024 · 14 comments · Fixed by #11409
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@duzenz
Copy link

duzenz commented Jan 24, 2024

Jetty version(s),
12.0.5

Jetty Environment
ee10

Java version/vendor (use: java -version)
java version "17.0.2" 2022-01-18 LTS

OS type/version
Windows

Description
We have a spring boot project which is using jetty server. After upgrading spring boot 3.2+ big file uploads with multipart file has broken something. Current endpoint is working like this:

  • There is a post endpoint and client can upload multipart files from there.
  • but all the files uploaded from this endpoint already uploaded to somewhere else with other application and we persist all of the md5 hashes of this files.
  • The endpoint compares the uploaded file hash and already persisted hash and if it equals, flow continues.
  • with spring boot 3.1.7 and jetty server 11.0.19 it is working correctly
  • after spring boot versions upgrade (3.2.0, 3.2.1, 3.2.2) this flow is broken uploaded file hash is not equal anymore.
  • This is only happening for with the big files (Tried with a file which is bigger than 1GB)
  • With small size files there is no problem.
  • tested the same flow with tomcat server and it is working
  • we think that it could be related with DigestUtils, but then we tried to compare byte arrays directly and it is still problematic

How to reproduce?

@duzenz duzenz added the Bug For general bugs on Jetty side label Jan 24, 2024
@joakime
Copy link
Contributor

joakime commented Jan 24, 2024

This is surprising, as we upload 1GB, 4GB, and 10GB files during unit testing with content validation (file hash) on both sides.

@gbrehmer
Copy link

its also occurring with smaller files (200 MB) sometimes (= not each time reproducible). Should we create an issue on Spring Boot side as well, if it can also be related to the spring internal adaption code to integrate jetty?

@janbartel
Copy link
Contributor

@lachlan-roberts any insight to this problem?

@lachlan-roberts lachlan-roberts self-assigned this Jan 29, 2024
@duzenz
Copy link
Author

duzenz commented Feb 12, 2024

do you have any updates on this issue?

@lachlan-roberts
Copy link
Contributor

Spring is not using the Jetty multipart parser.

Currently for Jetty 12 they are using EE10 servlets to adapt to the spring request/response which will then end up in their own org.springframework.http.codec.multipart.MultipartParser.

We are currently working on an effort to update the adaption for Jetty 12 in spring (see spring-projects/spring-framework#32097). So I will try to test your reproducer tested against the work there.

@gbrehmer
Copy link

on spring side it was decided to change implementation only with next spring minor version 6.2, which will be released end of the year. This is unfortunately a little longer than expected

@lachlan-roberts do you think that those problems should only occur with uploading such big files or is there a risk, that this can occur also with smaller files and during download? Because we currently try to decide whether Jetty 12 with Spring Boot 3.2 is prod ready or whether we have to rollback all services or only the one who handles such big files. Thank in advance for your feedback on this

@joakime
Copy link
Contributor

joakime commented Feb 12, 2024

Be aware that spring produces improved error messages from their org.springframework.http.codec.multipart.MultipartParser implementation based on the work on issue spring-projects/spring-framework#28067

That could also point to your client not producing to spec multipart/form-data content.
Spring only supports the spec rules and doesn't allow things like only using LF for end of line. (eg: a tool called postman does this and is what triggered the above issue to be filed at spring-framework)
Jetty's multipart implementation is more lenient to these kinds of nuances.

Check your logs and exceptions to see if this is the case (read the above issue at spring-framework for details).

@gbrehmer
Copy link

We have no exceptions or error logs in such a case. Our client is using python 3 requests lib and it was working fine with spring boot 3.1, spring framework 6.0 and Jetty 11 and files are binary only (tar.gz). The problem occurred for first time after the switching to Spring Boot 3.2, Spring 6.1 and Jetty 12

@lachlan-roberts
Copy link
Contributor

I managed to reproduce the issue.

And looks I was wrong and spring is using the Jetty multipart parser in this case.
Here is a stack trace.

  at org.eclipse.jetty.http.MultiPart$Parser.parseContent(MultiPart.java:1365)
	  at org.eclipse.jetty.http.MultiPart$Parser.parse(MultiPart.java:1072)
	  at org.eclipse.jetty.http.MultiPartFormData$Parser$1.parse(MultiPartFormData.java:229)
	  at org.eclipse.jetty.http.MultiPartFormData$Parser$1.parse(MultiPartFormData.java:219)
	  at org.eclipse.jetty.io.content.ContentSourceCompletableFuture.parse(ContentSourceCompletableFuture.java:104)
	  at org.eclipse.jetty.http.MultiPartFormData$Parser.parse(MultiPartFormData.java:244)
	  at org.eclipse.jetty.ee10.servlet.ServletMultiPartFormData.lambda$from$0(ServletMultiPartFormData.java:135)
	  at org.eclipse.jetty.ee10.servlet.ServletMultiPartFormData$$Lambda/0x00007feff846abf0.apply(Unknown Source:-1)
	  at org.eclipse.jetty.http.MultiPartFormData.from(MultiPartFormData.java:86)
	  at org.eclipse.jetty.ee10.servlet.ServletMultiPartFormData.from(ServletMultiPartFormData.java:104)
	  at org.eclipse.jetty.ee10.servlet.ServletMultiPartFormData.from(ServletMultiPartFormData.java:60)
	  at org.eclipse.jetty.ee10.servlet.ServletApiRequest.getParts(ServletApiRequest.java:508)
	  at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.parseRequest(StandardMultipartHttpServletRequest.java:93)
	  at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.<init>(StandardMultipartHttpServletRequest.java:86)
	  at org.springframework.web.multipart.support.StandardServletMultipartResolver.resolveMultipart(StandardServletMultipartResolver.java:112)
	  at org.springframework.web.servlet.DispatcherServlet.checkMultipart(DispatcherServlet.java:1227)
	  at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1061)
	  at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:979)
	  at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014)
	  at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:914)
	  at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:547)
	  at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:885)
	  at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
	  at org.eclipse.jetty.ee10.servlet.ServletHolder.handle(ServletHolder.java:736)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1614)
	  at org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter.doFilter(WebSocketUpgradeFilter.java:195)
	  at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1586)
	  at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
	  at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	  at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1586)
	  at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
	  at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	  at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1586)
	  at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	  at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	  at org.eclipse.jetty.ee10.servlet.FilterHolder.doFilter(FilterHolder.java:205)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1586)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler$MappedServlet.handle(ServletHandler.java:1547)
	  at org.eclipse.jetty.ee10.servlet.ServletChannel.dispatch(ServletChannel.java:797)
	  at org.eclipse.jetty.ee10.servlet.ServletChannel.handle(ServletChannel.java:428)
	  at org.eclipse.jetty.ee10.servlet.ServletHandler.handle(ServletHandler.java:464)
	  at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:571)
	  at org.eclipse.jetty.ee10.servlet.SessionHandler.handle(SessionHandler.java:703)
	  at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:761)
	  at org.eclipse.jetty.server.Server.handle(Server.java:179)
         ...
	  at java.lang.Thread.run(Thread.java:1583)

@joakime
Copy link
Contributor

joakime commented Feb 13, 2024

@lachlan-roberts heads up, there is work going on in the MultiPart parsers in PR #8059 right now.
If you can reproduce in a test case on that PR's branch that would be awesome!

I'm chasing a failing test in the core MultiPart.Parser in that PR.
The failing test has a part with over 1MB of random whitespace characters (sent as a file part with Content-Disposition: form-data; name="whitespace"; filename="whitespace.txt" and Content-Type: application/octet-stream)
The (VERY) preliminary analysis seems to show that two CR's about 2/3rds of the way through that one part are dropped (seemingly as the parser sees those CRs are part of the multipart/form-data syntax, not the part). but only when the multipart/form-data is sent via a ServerConnector using 8k buffers. (If the buffers are bigger, the test passes)

lachlan-roberts added a commit that referenced this issue Feb 14, 2024
@lachlan-roberts
Copy link
Contributor

@joakime I reproduced it in a Jetty test in https://github.com/jetty/jetty.project/tree/jetty-12.0.x-largeMultipartFailure 1c698e2
(you can cherry-pick this commit to your branch)

Another observation from this test is that the difference between the sent-content and the received-content always seems to start from a position in the array where the value is 13 (CR). So maybe it is the same issue you are investigating.

@sbordet
Copy link
Contributor

sbordet commented Feb 14, 2024

@lachlan-roberts: @joakime found the issue, and it was fixed it in 9321e5e.

@joakime
Copy link
Contributor

joakime commented Feb 14, 2024

@lachlan-roberts @sbordet I took the fix out of PR #11388 and made a new PR #11409 to fix this Issue here.

Please give it a review.

sbordet added a commit that referenced this issue Feb 21, 2024
…rts (#11409)

Fixed case in MultiPart.Parser where a small chunk contains part of the boundary.

Added and fixed related tests.

Signed-off-by: Simone Bordet <[email protected]>
Co-authored-by: Simone Bordet <[email protected]>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.7 - FROZEN Feb 21, 2024
@gbrehmer
Copy link

we can confirm that current snapshot version (12.0.7-20240222.000749-22) fixed the problem on our side
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

6 participants