hk2166 commented on PR #16786:
URL: https://github.com/apache/grails-core/pull/16786#issuecomment-6494826726

   All three points are addressed in f8c3301c99.
   
   - **The nested dispatch**: the requests are copied when the dispatch the 
container started ends, as a new innermost `TaskRequestFilter` tells, not when 
a forward or include completes its web request. The copy is still taken inside 
the security filter chain (detail in the inline reply).
   - **The forward attributes**: the copies share the attributes of the request 
the container created, and each copy keeps what its own request shows 
otherwise, such as a forward's `jakarta.servlet.forward.*` (detail in the 
inline reply). The same check found that the multipart stand-in let a forward's 
parameters into `params` of the request's copy. A copy of the multipart request 
itself now stands in for it.
   - **The rejected upload**: the body of a multipart request is no longer 
copied, since its uploads go with the request and its text fields are in the 
parameters. Any other body is kept up to 2 MB, the amount Tomcat swallows of an 
unread body by default, and reading a longer one from the copy throws an 
`IllegalStateException`. The copy never reads more than that, through the 
stream or a reader the request was using. Tests: `the body of a multipart 
request is not copied, so an upload rejected for its size is not read` (nothing 
of a 16 MB upload is read), `a body longer than the copy keeps is not read 
whole, and reading it from the copy fails` (through the stream and through a 
reader), and `a body as long as the copy keeps is copied whole`.
   
   On d7f3923ce9, 10 of the 14 new unit test iterations fail, each on its own 
point, and so does the registration test I extended. I ran them there with a 
filter that only passes the request on, since there was no filter then. The 
rest pass on both: the re-entrancy and async tests of the filter, `params` with 
the request's task first, and the body at the limit. Without the filter's 
re-entrancy guard, only its test fails. Both tests of the new `TaskRequestSpec` 
in the pubsub demo fail there too, through Tomcat: the forward's copy answers 
`isForward` with false and `getForwardURI` with the forward target, and a task 
started during a forward misses the attribute set after it returned. With this 
commit, the module's 76 tests pass, and so do the pubsub demo's integration 
tests, including the new `TaskRequestSpec`. Checkstyle, CodeNarc, PMD and the 
repository conventions are clean, and the guide builds with the updated 
paragraph.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to