matrei commented on code in PR #16149:
URL: https://github.com/apache/grails-core/pull/16149#discussion_r3947898668
##########
grails-controllers/src/test/groovy/org/grails/plugins/web/controllers/ControllersAutoConfigurationSpec.groovy:
##########
@@ -200,6 +202,16 @@ class ControllersAutoConfigurationSpec extends
Specification {
}
}
+ private WebApplicationContextRunner hiddenMethodContextRunner() {
Review Comment:
Unused?
##########
grails-gsp/plugin/src/main/groovy/org/grails/plugins/web/taglib/FormTagLib.groovy:
##########
@@ -1575,6 +1580,7 @@ class FormTagLib implements ApplicationContextAware,
InitializingBean, TagLibrar
}
@Override
+ @GrailsCompileStatic
Review Comment:
Why `@GrailsCompileStatic` and not `@CompileStatic`?
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -3034,7 +3150,254 @@ job is disabled with `jobEnabled = false`, or the class
is not a job artefact of
An application that caught `NullPointerException` around these calls has to
catch those instead. See
<<quartzDynamicScheduling,Dynamic Job Scheduling>>.
-==== 52. Coordinate Changes
+==== 53. `request` Is No Longer a `MultipartHttpServletRequest`
+
+The `request` object Grails exposes to controllers, tag libraries and GSPs is
now always the outermost
+servlet request. Previously, for a file upload, Grails replaced it with the
resolved
+`MultipartHttpServletRequest`, which discarded the request wrappers
contributed by other filters — the
+hidden HTTP method filter, Spring Security, and any application filter.
+
+All file upload methods work exactly as before:
+
+[source,groovy]
+----
+def upload() {
+ def file = request.getFile('myFile')
+ if (file.empty) {
+ flash.message = 'file cannot be empty'
+ render(view: 'uploadForm')
+ return
+ }
+ file.transferTo(new File('/some/local/dir/myfile.txt'))
+}
+----
+
+`request.getFiles(name)`, `request.getFileNames()`, `request.getFileMap()`,
`request.getMultiFileMap()`
+and `request.getMultipartContentType(name)` are likewise unchanged, as is
binding a file through
+`params`:
+
+[source,groovy]
+----
+def img = new Image(params)
+----
+
+What no longer works is treating `request` as a `MultipartHttpServletRequest`
by type:
+
+[source,groovy]
+----
+// Before
+if (request instanceof MultipartHttpServletRequest) {
+ def file = ((MultipartHttpServletRequest) request).getFile('myFile')
+}
+
+// After
+def file = request.getFile('myFile')
+----
+
+Calling one of the file methods on a request that is not a file upload throws
`IllegalStateException`
+with a diagnostic message, the same way it previously failed with
`MissingMethodException`.
+
+===== 53.1 Oversized uploads now reach the application's error handling
+
+An upload larger than `grails.controllers.upload.maxFileSize` or
+`grails.controllers.upload.maxRequestSize` (both 128000 bytes by default)
fails when the servlet
+container parses the request parts, and every parameter read on that request
fails with it from then
+on. Because Grails reads request parameters on paths that run before,
alongside and after the handler
+— to resolve the `_method` override, whenever a filter such as Spring Security
builds `params`, in the
+locale-change interceptor, and in the exception resolver's request log — that
failure used to abort
+the request inside the filter chain, where no exception handler could see it.
The application was left
+with the container's own error page.
+
+Those reads now tolerate a multipart request the container refuses to parse,
so the failure surfaces
+where Spring raises it, as a
+`org.springframework.web.multipart.MultipartException`/`MaxUploadSizeExceededException`
during
+dispatch. The exception is available to `HandlerExceptionResolver` beans, and
the response is a `413`
+rendered through the application's error dispatch instead of the container's
own error page.
+
+A `"413"` response code URL mapping handles it, the same way a `"404"` or
`"500"` mapping handles those
+statuses:
+
+[source,groovy]
+----
+"413"(controller: 'errors', action: 'tooLarge')
+----
+
+The mapped action runs on the container's error dispatch, so
`request.dispatcherType` is `ERROR` and
+`params` is empty — the container never parsed the request.
`request.getFile(..)` is unavailable there
+for the same reason. The upload itself is gone by then; the action's job is to
render the response.
+
+The parameters of such a request are empty everywhere, not just in the error
handler. The request
+cannot reach the originally requested controller either way —
`DispatcherServlet` rejects it before
+handler resolution — so outside the error handler this is only observable in a
filter that inspects
+`params` ahead of the dispatch.
+
+====== Servlet container differences
+
+Tomcat and Jetty both surface the failure to the application as described
above. Undertow applies the
+limit while it reads the request entity and refuses the request at the HTTP
layer, so no application
+code runs at all: the response is a bare `413` with an empty body, and a
`"413"` mapping is not
+consulted. Raising `grails.controllers.upload.maxRequestSize` raises the point
at which Undertow
+refuses, but there is no way to render a body for a request the container
never dispatches.
+
+===== 53.2 An error handler that fails is no longer forwarded to from inside
itself
+
+A `"500"` (or exception-specific) URL mapping that names a controller is
reached by forwarding to it.
+That forward re-enters the `DispatcherServlet`, and the forwarded dispatch
resolves the same status
+code mapping again. An error handler that failed for a reason belonging to the
request rather than to
+the moment — an unparseable multipart body, a missing collaborator — therefore
failed again inside its
+own forward, and that failure forwarded to it once more, until the dispatch
stack overflowed.
+
+The error handler is no longer forwarded to while a forward to it is already
running. The repeat
+failure is logged and the exception is returned to the `DispatcherServlet`,
which reports it through
+the container, so the response is a single container-level error rather than a
dispatch that never
+terminates. Once the forward returns, a later error on the same request — from
an enclosing request
+whose include failed, for instance — is forwarded to the error handler as
before.
+
+==== 54. Request Processing Behaviour Changes
+
+Four changes fall out of Grails 8 delegating more of the request path to
Spring.
+
+===== 54.1 API versioning headers are now emitted for Grails-mapped requests
+
+`UrlMappingsHandlerMapping` previously assembled its own handler execution
chain rather than using
+Spring's. As a result, Grails-mapped requests skipped the interceptor Spring
adds for API versioning,
+so the `Deprecation`, `Sunset` and `Link` headers configured through
`spring.mvc.apiversion.*` were
+never sent. The chain is now assembled by Spring, and those headers are
emitted as configured.
+
+If an application relied on those headers being absent, unset the corresponding
+`spring.mvc.apiversion.*` configuration.
+
+===== 54.2 The `LocaleContext` is restored rather than cleared
+
+At the end of a request Grails previously called
`LocaleContextHolder.setLocale(null)`, which discarded
+any `LocaleContext` established earlier in the filter chain by non-Grails
code. Grails now saves and
+restores the previous `LocaleContext`, matching Spring's own
`RequestContextFilter`.
+
+===== 54.3 The application attributes object is shared per servlet context
+
+`GrailsApplicationAttributes` was constructed once per request. It is now
created once per servlet
+context and reused, so the beans it caches are resolved once rather than on
every request. It is
+rebuilt automatically if the `ApplicationContext` is replaced.
+
+Applications that implemented `GrailsApplicationAttributes` themselves must
ensure their
+implementation holds no request-scoped state and is safe to use from multiple
request threads.
+
+===== 54.4 `GrailsWebRequest.setMultipartRequest(..)` is deprecated
+
+`setMultipartRequest(HttpServletRequest)` installed the resolved multipart
request on the web request. With
+the substitution gone the resolver publishes it as a request attribute
instead, and Grails finds it there or
+by unwrapping. The method still works -- it publishes its argument and
discards the cached params -- but a
+caller that has one should move to letting the resolver publish it, or set the
attribute directly:
+
+[source,groovy]
+----
+// Before
+webRequest.setMultipartRequest(resolved)
+
+// After
+request.setAttribute(WebUtils.MULTIPART_HTTP_SERVLET_REQUEST_ATTRIBUTE,
resolved)
+webRequest.multipartRequestResolved()
+----
+
+===== 54.5 `GrailsWebRequest.getCurrentRequest()` is deprecated
+
+`getCurrentRequest()` existed to hand back the resolved multipart request in
place of the request Grails
+was bound to. That substitution is gone (see _52. `request` Is No Longer a
`MultipartHttpServletRequest`_),
+so the method now returns exactly what `getRequest()` returns. It is
deprecated; call `getRequest()`
+instead.
+
+[source,groovy]
+----
+// Before
+def uri = webRequest.currentRequest.requestURI
+
+// After
+def uri = webRequest.request.requestURI
+----
+
+Nothing about the value returned has changed, so this is a rename, not a
behaviour change. Code that
Review Comment:
I would not call this a rename. `getRequest()` was always there.
--
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]
