Detect Jetty 12 "max length exceeded" message for MaxUploadSizeExceededException

[Aliaksie]

Affects:

• org.springframework.boot -> 3.2.0
• org.springframework -> 6.1.1
• spring-boot-starter-jetty -> 3.2.0
• jetty -> 12.x

StandardMultipartHttpServletRequest contains the following code:

	protected void handleParseFailure(Throwable ex) {
		String msg = ex.getMessage();
		if (msg != null) {
			msg = msg.toLowerCase();
			if (msg.contains("size") && msg.contains("exceed")) {
				throw new MaxUploadSizeExceededException(-1, ex);
			}
		}
		throw new MultipartException("Failed to parse multipart servlet request", ex);
	}

That seems like it doesn't handle correctly the case for MaxUploadSizeExceededException for jetty server due to fact that original error from the Servlet API will be throw new ServletException(new BadMessageException("bad multipart", cause));, and the error from jetty-http for multipart will be throw new IllegalStateException("max length exceeded: %d".formatted(max));.

[sdeleuze]

Could you please provide a reproducer as an attached archive or a link to a repository?

[Aliaksie]

Could you please provide a reproducer as an attached archive or a link to a repository?

@sdeleuze sure please have a look there

there i've added code that you could use later for fix

@ExceptionHandler({MultipartException.class})
        public ResponseEntity<ErrorResponse> handleMultipartException(final HttpServletRequest request,
                                                                      final MultipartException e) {
            final Throwable rootCause = e.getRootCause();
            String msg = Optional.ofNullable(rootCause).map(Throwable::getMessage).orElse(null);
            if (msg == null) {
                return this.error(HttpStatus.BAD_REQUEST, request, e.getMessage());
            }

            msg = msg.toLowerCase();
            if (hasLengthError(msg) && msg.contains("exceed")) {
                return tooLarge(request, new MaxUploadSizeExceededException(-1, rootCause));
            }

            return this.error(HttpStatus.BAD_REQUEST, request, e.getMessage());
        }

        private static boolean hasLengthError(final String msg) {
            return msg.contains("size") || msg.contains("max length");
        }

Note: Be aware that test results may overlap with other client-related issues: "The connection was closed BEFORE response."

[jhoeller]

So if StandardMultipartHttpServletRequest.handleParseFailure checked for the keyword "length" in addition to "size", would that be sufficient? Or do we have an additional level of cause nesting to consider there as well?

[Aliaksie]

So if StandardMultipartHttpServletRequest.handleParseFailure checked for the keyword "length" in addition to "size", would that be sufficient? Or do we have an additional level of cause nesting to consider there as well?

in my understanding both should be taken into account, as I mentioned, in Jetty 12.x the nesting of the error and the error msg itself have changed... you can also see it from my screenshot below

[jhoeller]

Indeed, thanks for sharing that screenshot which illustrates it nicely!

[jhoeller]

Related: #28759 for Jetty 9.4's exception message.

[jhoeller]

Pushed a revision now for inclusion in the 6.1.3 release. It would be great if you could give the upcoming 6.1.x snapshot a try!

[Aliaksie]

Pushed a revision now for inclusion in the 6.1.3 release. It would be great if you could give the upcoming 6.1.x snapshot a try!

yes it works with 6.1.3-SNAPSHOT ! demo using snapshot updated.

small note: maybe it would be nice if it were like throw new MaxUploadSizeExceededException(-1, cause); but the way it is now is also good.

thanks!

[jhoeller]

Thanks for the immediate check!

I was wondering whether to throw the top-level exception or the relevant cause as well but ultimately settled with the top-level exception since that may contain further details, even if it was a specific cause that led to us to identify an upload size failure.