jamesfredley commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4210585896
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements
HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does
not set renderView=false,
+ // so this path is always intentional view resolution — honour
it unconditionally.
def modelAndView =
request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ // All other render() variants (template, text, JSON, file,
closure, object) set
+ // webRequest.renderView = false. If that flag is clear the
response has already been
+ // handled; returning a ModelAndView here would cause
DispatcherServlet to attempt
+ // view resolution and throw "Could not resolve view". (#15819)
+ if (!webRequest.renderView) {
Review Comment:
`GrailsWebRequest.isRenderView()` is false when `renderView` was cleared,
and also when the status is 300 or higher, the response is committed, or a
redirect was called. An action that sets `response.status` to 404 or 500 and
returns a Map or `ModelAndView` used to render that model. This guard now
returns null, so the error page body is dropped.
Limit the guard to an explicit `render()` (the `renderView` field itself).
Leave the error-status, committed, and redirect cases on the previous path.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer <<
"Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets
renderView=false,
+ * writes content, returns null. The adapter must return null so
DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map —
the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when
result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
These fixtures assign `renderView` and `MODEL_AND_VIEW` directly, so they
never call `render()`. Please drive the text, template, and view cases through
the public `render(...)` methods, and add a case that sets an error status and
returns a model so this guard cannot swallow error views.
--
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]
