Fix 16145: Oversized multipart uploads cannot be handled by Grails application code

[matrei]

Closes #16145

[Copilot]

Pull request overview

Adds early multipart resolution so upload failures and valid files remain accessible through Grails request handling.

Changes:

• Registers an early multipart filter.
• Preserves multipart requests through wrappers.
• Avoids premature multipart parsing in method-override handling.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
GrailsWebRequestFilter.java	Stores resolved multipart requests.
GrailsDispatcherServlet.groovy	Propagates multipart wrappers.
HiddenHttpMethodFilter.java	Changes multipart parameter handling.
HiddenHttpMethodFilterTests.groovy	Tests multipart filter behavior.
ControllersAutoConfiguration.java	Registers the multipart filter.
ControllersAutoConfigurationSpec.groovy	Verifies filter order.

Suppressed comments (2)

grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/ControllersAutoConfiguration.java:109

• MultipartFilter always calls cleanupMultipart in finally as soon as the initial filter chain returns. For an async MVC request, that return happens after async processing starts but before the worker necessarily reads the uploaded file, so globally installing this filter can delete temporary upload data prematurely; use an async-aware filter that defers cleanup until async completion.

        var multipartFilter = new MultipartFilter();

grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/ControllersAutoConfiguration.java:115

• The added test only calls this factory and checks the filter order; it never exercises multipart resolution. Add a servlet-container regression test with the configured size limit that verifies an oversized upload reaches the application's error handling and a valid security-wrapped upload still exposes getFile(...) and bound parameters.

        registrationBean.setOrder(GrailsFilters.FIRST.getOrder());

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 56.06061% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.6511%. Comparing base (fa1e147) to head (6fe63b8).
⚠️ Report is 6 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...plugins/web/controllers/GrailsMultipartFilter.java	66.6667%	7 Missing and 2 partials ⚠️
...servlet/mvc/GrailsMultipartHttpServletRequest.java	33.3333%	8 Missing ⚠️
...org/grails/web/filters/HiddenHttpMethodFilter.java	0.0000%	6 Missing ⚠️
...y/org/grails/web/servlet/mvc/GrailsWebRequest.java	25.0000%	1 Missing and 2 partials ⚠️
...grails/web/servlet/mvc/GrailsWebRequestFilter.java	33.3333%	1 Missing and 1 partial ⚠️
...ils/web/servlet/mvc/GrailsDispatcherServlet.groovy	85.7143%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16146        +/-   ##
==================================================
+ Coverage     52.5503%   52.6511%   +0.1008%     
- Complexity      18391      18451        +60     
==================================================
  Files            2037       2039         +2     
  Lines           96498      96564        +66     
  Branches        16860      16869         +9     
==================================================
+ Hits            50710      50842       +132     
+ Misses          38353      38289        -64     
+ Partials         7435       7433         -2     

Files with missing lines	Coverage Δ
.../web/controllers/ControllersAutoConfiguration.java	95.5882% <100.0000%> (+0.5063%)	⬆️
...ils/web/servlet/mvc/GrailsDispatcherServlet.groovy	38.9830% <85.7143%> (+12.0600%)	⬆️
...grails/web/servlet/mvc/GrailsWebRequestFilter.java	52.7778% <33.3333%> (-1.7677%)	⬇️
...y/org/grails/web/servlet/mvc/GrailsWebRequest.java	62.9834% <25.0000%> (+1.7475%)	⬆️
...org/grails/web/filters/HiddenHttpMethodFilter.java	16.0000% <0.0000%> (-4.0000%)	⬇️
...servlet/mvc/GrailsMultipartHttpServletRequest.java	33.3333% <33.3333%> (ø)
...plugins/web/controllers/GrailsMultipartFilter.java	66.6667% <66.6667%> (ø)

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codeconsole]

I think I fixed this in one of my apps. I need to see how I solved it there.
As is, I think this is silently dropping _method

[codeconsole]

Finding	Verdict	Notes
Multipart _method regression	Valid — blocking	HiddenHttpMethodFilter now ignores _method for every multipart/* request. Grails upload forms using method="PUT", PATCH, or DELETE therefore remain POST.
Later request wrappers are discarded	Valid — blocking	getNativeRequest(..., MultipartHttpServletRequest) returns the inner multipart wrapper. Storing it as the current request bypasses later wrappers, including HttpMethodRequestWrapper and Spring Security's request wrapper.
spring.servlet.multipart.enabled=false is defeated	Valid — blocking	When Boot removes its multipart resolver, Spring's MultipartFilter falls back to its private StandardServletMultipartResolver, effectively re-enabling multipart processing.
Multipart parsing occurs before character encoding	Valid — blocking	Registering the multipart filter at FIRST causes eager parsing before CharacterEncodingFilter. Calling setCharacterEncoding afterward is too late for multipart fields.
Async multipart cleanup happens too early	Valid — blocking	MultipartFilter cleans multipart resources when the initial filter chain returns. For asynchronous MVC requests, temporary upload files may be deleted before the async worker consumes them.
GrailsWebRequestFilter assignment is ineffective	Valid — non-blocking	It is redundant when the request is directly a MultipartHttpServletRequest and does not handle multipart requests hidden beneath later wrappers.
Missing end-to-end regression coverage	Valid — non-blocking	The tests do not exercise an oversized upload reaching application error handling, a valid security-wrapped upload, multipart _method, encoding, disabled multipart support, or asynchronous file consumption.
Registration test is incomplete	Valid — non-blocking	The test compares the order with GrailsFilters.FIRST but does not verify relative ordering against character encoding, hidden-method, and security filters or test conditional backoff.
Fully qualified WebUtils call	Style only	An import alias would improve readability but has no behavioral effect.
Missing documentation	Advisory	Documentation would be useful, but this bug fix does not necessarily need to be blocked solely under the rule requiring documentation for new features.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 6fe63b8
▶️ Tests: 68219 executed
⚪️ Checks: 77/77 completed

---

Learn more about TestLens at testlens.app.

[codeconsole]

I haven't dug deep into this yet so I am just thinking out loud, but:

Do we still need all this 10+ year old infrastructure code? Is there an option to rewire more lean and push all the multipart handling onto Spring and use as little custom code as possible? Is there anything we can get rid of?

Can we get rid of GrailsMultipartFilter?

#16040

[codeconsole]

I have an alternative for the request plumbing: discover the multipart request from the wrapper chain rather than substituting it, so the security and method-override wrappers survive and the adapter isn't needed. It doesn't cover the oversize half — that still needs your filter.

Let me know what you think, we could merge mine first, then your filter + the HiddenHttpMethodFilter guard on top?
#16149

It eliminates the need for GrailsMultipartHttpServletRequest

[codeconsole]

I really don't like the idea of circumventing the Dispatcher servlet and parsing the multipart upstream all to get the HiddenHttpMethodFilter This is a lot of infrastructure code just for HiddenHttpMethodFilter.

It was a legacy workaround. Should we even be using it anymore??? We should at least explore disabling it by default like Spring.

The convention (originally Rails') is: the form POSTs with a hidden _method=PUT field, and a filter wraps the request so getMethod() returns PUT. That's what <g:form method="PUT"> and <g:uploadForm resource="book" action="update" emit, and it's how Grails' scaffolded RESTful forms work.

Spring has one — and Boot turns it off

org.springframework.web.filter.HiddenHttpMethodFilter exists in spring-web 7.0.8. But Boot's own configuration metadata says:

  name:     spring.mvc.hiddenmethod.filter.enabled
  default:  False

Spring Boot ships it disabled. It's been off by default since Boot 2.2 — the reasoning at the time being that it's a legacy workaround, browsers aren't the dominant client any more, and anything using fetch/XHR can just send the real method.

Grails, by contrast, registers its own version enabled by default, and Grails' version is broader: it accepts any method and also honours the X-HTTP-Method-Override header, where Spring's is narrower.

[codeconsole]

It seems like there is still 1 issue with oversized on this PR. I came up with a proposal that doesn't require any extra infrastructure. It just ignores the multipart and kicks it down the line to where it is normally handled in the Spring Dispatcher Servlet.

Unfortunately, the PR had to be stacked on top of my other performance PR so you can't see a clean diff, but if you compare the changes between the two it will be more obvious. Feel free to take whatever you want from It, expand on it, or bring any of the ideas over here.

I think we should consider exploring default disabling the Hidden Method Filter like Spring Boot does. What are your thoughts?

Test Results

What an application actually receives when a multipart upload exceeds the configured limit
(200 KB payload against the default 128000-byte maxRequestSize).

• No fix — 8.0.x @ a83f87480e
• Fix 16145: Oversized multipart uploads cannot be handled by Grails application code #16146 — fix/issue-16145 @ 6fe63b8486 (early MultipartFilter)
• Let oversized uploads reach the application's error handling #16152 — fix/multipart-lean-8.0.x (tolerate the parse failure, defer to checkMultipart)

Every row is same-app, same-config, same probe — only the framework code differs.

Container	Case	No fix	#16146	#16152
Tomcat 11	Oversized, no security, "413" mapping	500 raw Tomcat HTML	500 raw Tomcat HTML	413 {"error":"Content Too Large","handledBy":"errors.tooLarge"}
Tomcat 11	Oversized, Spring Security on the chain	413 raw Tomcat HTML	413 raw Tomcat HTML	413 application's handler
Tomcat 11	Normal upload	200	200	200
Tomcat 11	<g:uploadForm method="PUT">	200, method PUT	200, method PUT	200, method PUT
Jetty 12	Oversized	400 "Bad Request" — Boot's generic error, indistinguishable from a malformed request	not measured	413 application's handler
Undertow 2.4	Oversized	413, empty body	not measured	413, empty body — unchanged

#16146 was run on Tomcat only. Its Jetty and Undertow cells are blank because they were not measured,
not because they are known to be unchanged.

Undertow applies the limit while reading the request entity, so no filter and no servlet run. Nothing the
framework does can influence it; the limitation is documented and pinned by a spec.

[codeconsole]

@matrei. I am a little concerned with the performance implications of not using the DispatcherServlet (My alternate fix also has this issue). These have always been present (even before this PR).

GrailsMultipartFilter still eagerly resolves every multipart request in the filter chain, ahead of DispatcherServlet — for every request, regardless of whether the app needs _method support. That's the same shape of thing Spring Boot moved away from in 2019.

From the Boot issue that led to HiddenHttpMethodFilter being disabled by default
(spring-projects/spring-boot#16953):

The endpoints that were affected by this were accepting MultipartFile request
parameter - we didn't look that deep in since disabling the filter basically fixed
the problem and improved performance significantly but I assume this was related to
size of the request as HiddenHttpMethodFilter does ServletRequest#getParameter.