ivaylodimov-taulia commented on issue #15819:
URL: https://github.com/apache/grails-core/issues/15819#issuecomment-6123444365
@ruthst00 thanks for trying it. Your sample didn't reproduce it because the
bug only shows up under a specific filter configuration, and the root cause in
my original report was wrong. I went back and traced it properly. Corrected
analysis below.
## The trigger
The bug needs a Spring `RequestContextFilter` that runs **after**
`GrailsWebRequestFilter` (order `GrailsFilters.GRAILS_WEB_REQUEST_FILTER` =
−150).
A vanilla Grails app doesn't have one. Spring Boot's
`WebMvcAutoConfiguration` would normally register `OrderedRequestContextFilter`
at −105, but Grails adds `@EnableWebMvc` to the `Application` class
(`ApplicationClassInjector`), which disables `WebMvcAutoConfiguration` through
its `@ConditionalOnMissingBean(WebMvcConfigurationSupport)` condition. That's
why your sample works.
Ordinary configuration adds one back, though. In our app it comes from two
places:
- a `RequestContextFilter` added to the Spring Security
`SecurityFilterChain`, so that request-scoped beans are available to security
filters;
- an `OrderedRequestContextFilter` bean (order −105).
Either one alone is enough. Spring Boot's own `JerseyAutoConfiguration` also
registers a `RequestContextFilter` (order `spring.jersey.filter.order - 1`, so
−1 by default), so Grails plus `spring-boot-starter-jersey` should hit this as
well. I've inferred that from the Boot source but haven't run a vanilla Grails
+ Jersey app to confirm it.
## What actually happens
`DispatcherServlet` never checks `isCommitted()` before rendering. It
renders whenever the handler returns a non-null `ModelAndView`. With the
trigger above, **two `GrailsWebRequest` instances end up live in the same
request**, and `ResponseRenderer` and `UrlMappingsInfoHandlerAdapter` each read
a different one:
1. `GrailsWebRequestFilter` creates instance **A**, stores it as the
`WEB_REQUEST` request attribute and binds it to `RequestContextHolder`.
2. The later `RequestContextFilter` replaces the `RequestContextHolder`
binding with plain `ServletRequestAttributes`.
3. `GrailsDispatcherServlet.buildRequestAttributes` sees that the previous
attributes aren't a `GrailsWebRequest`, so it creates instance **B**. B is
bound to `RequestContextHolder` only, not stored as the request attribute.
4. `ResponseRenderer` uses
`RequestContextHolder.currentRequestAttributes()`, so `render(template:)` sets
`renderView = false` on **B**.
5. `UrlMappingsInfoHandlerAdapter.handle` uses
`GrailsWebRequest.lookup(request)`, which prefers the request attribute, so it
reads **A**. A's `renderView` is still `true`, so the adapter returns `new
ModelAndView(actionUriToViewName(action))`, and that fails with `Could not
resolve view with name '<action>'`.
`GrailsDispatcherServlet` does handle the "previous attributes aren't a
`GrailsWebRequest`" case explicitly, but it never publishes the new instance as
the request attribute, so `lookup(request)` and `RequestContextHolder` disagree
for the rest of the dispatch.
`response.flushBuffer()` only *appears* to fix it.
`GrailsWebRequest.isRenderView()` is `renderView && !response.isCommitted() &&
...`, so committing the response makes the stale instance A return `false` as
well. That's also why `render(text)` / `as JSON` often seem fine: their write
path tends to leave the response committed already, while `render(template:)`
doesn't.
Verified in our app (Grails 7.2.3) with our `flushBuffer()` workaround
removed, logging both instances' identity and `renderView` after `render(...)`:
| action | status | `renderView` (RequestContextHolder / request attr) |
committed |
|---|---|---|---|
| `render('hello')` | 200 | false / false | true |
| `render(template: 'frag')` | **500** | false / **true** | **false** |
| `render([a: 1] as JSON)` | 200 | false / false | true |
The two instance identities differed in every request.
## Reproducing on your sample
Register a `RequestContextFilter` that runs after `GrailsWebRequestFilter`
(any order > −150), then hit `/search/results`. I haven't run this against your
project. Something like this in `resources.groovy` (`FilterRegistrationBean`'s
package differs between Spring Boot 3 and 4):
```groovy
beans = {
lateRequestContextFilter(FilterRegistrationBean) {
filter = new org.springframework.web.filter.RequestContextFilter()
order = -100
}
}
```
## Suggested fix
Please disregard the `flushBuffer()` proposal in the original report: it
only masks the split. Better options:
- `GrailsDispatcherServlet.buildRequestAttributes` reuses the existing
instance (`GrailsWebRequest.lookup(request)`) when it's still active, instead
of creating a new one. Or, if it does create one, it also stores it as the
request attribute.
- Alternatively, `ResponseRenderer` and `UrlMappingsInfoHandlerAdapter`
should resolve the `GrailsWebRequest` the same way.
I'll update the issue title and description to match.
--
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]
