hk2166 commented on code in PR #16786: URL: https://github.com/apache/grails-core/pull/16786#discussion_r4409173408
########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,907 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.plugins.web.async + +import java.nio.charset.Charset +import java.nio.charset.StandardCharsets +import java.security.Principal +import java.text.ParseException +import java.text.SimpleDateFormat +import java.util.concurrent.ConcurrentHashMap + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncContext +import jakarta.servlet.DispatcherType +import jakarta.servlet.ReadListener +import jakarta.servlet.RequestDispatcher +import jakarta.servlet.ServletConnection +import jakarta.servlet.ServletContext +import jakarta.servlet.ServletInputStream +import jakarta.servlet.ServletRequest +import jakarta.servlet.ServletRequestWrapper +import jakarta.servlet.ServletResponse +import jakarta.servlet.http.Cookie +import jakarta.servlet.http.HttpServletMapping +import jakarta.servlet.http.HttpServletRequest +import jakarta.servlet.http.HttpServletResponse +import jakarta.servlet.http.HttpSession +import jakarta.servlet.http.HttpUpgradeHandler +import jakarta.servlet.http.MappingMatch +import jakarta.servlet.http.Part + +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpMethod +import org.springframework.util.LinkedCaseInsensitiveMap +import org.springframework.util.LinkedMultiValueMap +import org.springframework.util.MultiValueMap +import org.springframework.web.context.request.RequestAttributes +import org.springframework.web.multipart.MultipartFile +import org.springframework.web.multipart.MultipartHttpServletRequest + +/** + * A copy of a request, taken as the request completed, which a task that outlives the request reads in its place. + * + * <p>The container recycles a request once it completes, so a task that read the request itself from then on + * would fail. One copy is taken as the request completes, and each task reads it through a {@link #newView view} + * of its own, so each has the part of the body nothing had read, served through one stream or reader whose + * position persists, as the live request serves it. The copy holds the attributes, parameters, headers, locales + * and cookies of the request, its session and what identifies it. Attributes a task sets stay with the copy, + * shared by every view, as they are shared on the live request. The request, its response and what holds + * either, such as the web request of the request thread, are not carried over, so the copy holds nothing the + * container recycles. What only the live request can do, such as dispatching, processing asynchronously, creating + * a session or checking a role, is not available.</p> + * + * <p>The uploads of a multipart request are deleted as the request ends, so the copy answers as a multipart + * request without uploads: through the request of a task when the request was a multipart request, and in place of + * a multipart request an attribute held, such as the one the dispatcher resolved. Reading params then finds the + * copied parameters and no uploads.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements MultipartHttpServletRequest { + + private static final byte[] NO_BODY = new byte[0] + private static final List<String> DATE_HEADER_FORMS = List.of( + 'EEE, dd MMM yyyy HH:mm:ss zzz', + 'EEEE, dd-MMM-yy HH:mm:ss zzz', + 'EEE MMM dd HH:mm:ss yyyy') + + private final Map<String, Object> attributes + private final byte[] body + private final Map<String, String[]> parameters + private final Map<String, List<String>> headers + private final List<Locale> locales + private final Cookie[] cookies + private final HttpSession session + private final ServletContext servletContext + private final Principal userPrincipal + private final DispatcherType dispatcherType + private final HttpServletMapping httpServletMapping + private final ServletConnection servletConnection + private final String authType + private final String contentType + private final long contentLength + private final String method + private final String requestURI + private final String requestURL + private final String queryString + private final String contextPath + private final String servletPath + private final String pathInfo + private final String pathTranslated + private final String protocol + private final String scheme + private final String serverName + private final int serverPort + private final boolean secure + private final String remoteAddr + private final String remoteHost + private final int remotePort + private final String localName + private final String localAddr + private final int localPort + private final String remoteUser + private final String requestedSessionId + private final boolean requestedSessionIdValid + private final boolean requestedSessionIdFromCookie + private final boolean requestedSessionIdFromURL + private final String requestId + private final String protocolRequestId + private volatile String characterEncoding + private ServletInputStream inputStream + private BufferedReader reader + + private CompletedHttpServletRequest(HttpServletRequest request) { + this.attributes = new ConcurrentHashMap<>() + List<String> multipartRequests = new ArrayList<>() + for (String name : Collections.list(request.attributeNames)) { + Object value = request.getAttribute(name) + if (value instanceof MultipartHttpServletRequest) { + multipartRequests.add(name) + } + else if (value != null && !recycledWithRequest(value)) { + attributes.put(name, value) + } + } + this.parameters = Collections.unmodifiableMap(copyParameters(request)) Review Comment: The copy now reads the parameters as `params` does. Before the copy takes the place of a multipart request an attribute holds, that request's parameters are added to the copy's, keeping the request's own value for a name both have, as `GrailsParameterMap` merges them with `putIfAbsent`. A multipart request that can no longer parse its body adds nothing, as it adds nothing to `params`. `the copy keeps the text fields only the multipart request the dispatcher resolved has, with the request value of a field both have` puts one field only on the published multipart request and another on both, and checks `params` before and after completion. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/TaskRequest.groovy: ########## @@ -0,0 +1,267 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.plugins.web.async + +import java.lang.reflect.InvocationHandler +import java.lang.reflect.InvocationTargetException +import java.lang.reflect.Method +import java.lang.reflect.Modifier +import java.lang.reflect.Proxy + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncEvent +import jakarta.servlet.AsyncListener +import jakarta.servlet.ServletRequest +import jakarta.servlet.ServletRequestWrapper +import jakarta.servlet.http.HttpServletRequest + +import org.springframework.web.context.request.RequestAttributes + +import org.grails.web.servlet.mvc.GrailsWebRequest + +/** + * The request a task started during a web request sees. + * + * <p>Until the request completes, it is that request, with every public view the request and the requests it + * wraps offer, such as a multipart wrapper's, so a task the response waits for, such as one started with + * {@code WebPromises}, renders through it. The container recycles the request once it completes, including any + * asynchronous processing, so from then on it is a {@link CompletedHttpServletRequest copy} of the request taken + * as it completed, and the recycled request is released. One copy is taken for the request, however many tasks + * it has, and each task reads it through a view of its own, so each has the body to read. A multipart request + * stays one, without the uploads, which go with the request.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class TaskRequest implements InvocationHandler { + + private static final String STATE = TaskRequest.name + '.STATE' + private static final String COMPLETION_CALLBACK = TaskRequest.name + '.COMPLETION' + + private final RequestState state + private CompletedHttpServletRequest view + + private TaskRequest(RequestState state) { + this.state = state + } + + /** + * The request a task started during a web request sees. It has to be called while the web request is the + * current one, when the task is submitted. + * + * @param webRequest the web request the task is started during + * @return the request for the task + */ + static HttpServletRequest of(GrailsWebRequest webRequest) { + HttpServletRequest current = webRequest.currentRequest + if (Proxy.isProxyClass(current.getClass())) { + InvocationHandler handler = Proxy.getInvocationHandler(current) + if (handler instanceof TaskRequest) { + // A task started from a task reads the same request through a view of its own + return (HttpServletRequest) Proxy.newProxyInstance(current.getClass().classLoader, + current.getClass().interfaces, new TaskRequest(((TaskRequest) handler).state)) + } + } + ClassLoader loader = current.getClass().classLoader ?: TaskRequest.classLoader + return (HttpServletRequest) Proxy.newProxyInstance(loader, interfacesOf(current, loader), + new TaskRequest(stateOf(webRequest, current))) + } + + // One state for the request, however many tasks it has, so completing it takes the one copy they all read + private static RequestState stateOf(GrailsWebRequest webRequest, HttpServletRequest current) { + RequestState existing = boundTo(current) Review Comment: Each task now keeps the request it was given. The state on the request holds one entry for each request its tasks were given, so a task given a wrapper of the request, such as the request of a forward or a filter's wrapper, reads that wrapper while the request is live, and a task given the request itself still reads the request. The completion stays shared. When the request completes, one copy is taken of each of those requests, and the copies share the attributes and the body, as the requests do. After completion each task still sees what its own request showed, including the wrapper's form field. A request that can no longer be read as the request completes leaves only its own tasks without a copy. `each task sees the request it was given, though an earlier task of the request was given the request it wraps` starts one task on the request and one on a wrapper that adds a field. It checks both tasks before and after completion, including that an attribute one task sets after completion reaches the other. Testing this turned up a related problem. The task's request took every public interface of the wrapper chain, including `GroovyObject` when a wrapper is written in Groovy, as the requests of Grails unit tests are. Groovy then dispatched calls through the wrapper's meta class against the stand-in. A servlet method the wrapper overrides, such as `getParameter`, failed with `IllegalArgumentException: object is not an instance of declaring class`, and a method only that class has failed with a `ClassCastException`. The task's request now leaves `GroovyObject` out, so Groovy calls it through the servlet interfaces, and a method only the wrapper has is missing, as on any other request. `Groovy code calls the request of a task through its interfaces, though the request is written in Groovy` covers it. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,907 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.plugins.web.async + +import java.nio.charset.Charset +import java.nio.charset.StandardCharsets +import java.security.Principal +import java.text.ParseException +import java.text.SimpleDateFormat +import java.util.concurrent.ConcurrentHashMap + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncContext +import jakarta.servlet.DispatcherType +import jakarta.servlet.ReadListener +import jakarta.servlet.RequestDispatcher +import jakarta.servlet.ServletConnection +import jakarta.servlet.ServletContext +import jakarta.servlet.ServletInputStream +import jakarta.servlet.ServletRequest +import jakarta.servlet.ServletRequestWrapper +import jakarta.servlet.ServletResponse +import jakarta.servlet.http.Cookie +import jakarta.servlet.http.HttpServletMapping +import jakarta.servlet.http.HttpServletRequest +import jakarta.servlet.http.HttpServletResponse +import jakarta.servlet.http.HttpSession +import jakarta.servlet.http.HttpUpgradeHandler +import jakarta.servlet.http.MappingMatch +import jakarta.servlet.http.Part + +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpMethod +import org.springframework.util.LinkedCaseInsensitiveMap +import org.springframework.util.LinkedMultiValueMap +import org.springframework.util.MultiValueMap +import org.springframework.web.context.request.RequestAttributes +import org.springframework.web.multipart.MultipartFile +import org.springframework.web.multipart.MultipartHttpServletRequest + +/** + * A copy of a request, taken as the request completed, which a task that outlives the request reads in its place. + * + * <p>The container recycles a request once it completes, so a task that read the request itself from then on + * would fail. One copy is taken as the request completes, and each task reads it through a {@link #newView view} + * of its own, so each has the part of the body nothing had read, served through one stream or reader whose + * position persists, as the live request serves it. The copy holds the attributes, parameters, headers, locales + * and cookies of the request, its session and what identifies it. Attributes a task sets stay with the copy, + * shared by every view, as they are shared on the live request. The request, its response and what holds + * either, such as the web request of the request thread, are not carried over, so the copy holds nothing the + * container recycles. What only the live request can do, such as dispatching, processing asynchronously, creating + * a session or checking a role, is not available.</p> + * + * <p>The uploads of a multipart request are deleted as the request ends, so the copy answers as a multipart + * request without uploads: through the request of a task when the request was a multipart request, and in place of + * a multipart request an attribute held, such as the one the dispatcher resolved. Reading params then finds the + * copied parameters and no uploads.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements MultipartHttpServletRequest { + + private static final byte[] NO_BODY = new byte[0] + private static final List<String> DATE_HEADER_FORMS = List.of( + 'EEE, dd MMM yyyy HH:mm:ss zzz', + 'EEEE, dd-MMM-yy HH:mm:ss zzz', + 'EEE MMM dd HH:mm:ss yyyy') + + private final Map<String, Object> attributes + private final byte[] body + private final Map<String, String[]> parameters + private final Map<String, List<String>> headers + private final List<Locale> locales + private final Cookie[] cookies + private final HttpSession session + private final ServletContext servletContext + private final Principal userPrincipal + private final DispatcherType dispatcherType + private final HttpServletMapping httpServletMapping + private final ServletConnection servletConnection + private final String authType + private final String contentType + private final long contentLength + private final String method + private final String requestURI + private final String requestURL + private final String queryString + private final String contextPath + private final String servletPath + private final String pathInfo + private final String pathTranslated + private final String protocol + private final String scheme + private final String serverName + private final int serverPort + private final boolean secure + private final String remoteAddr + private final String remoteHost + private final int remotePort + private final String localName + private final String localAddr + private final int localPort + private final String remoteUser + private final String requestedSessionId + private final boolean requestedSessionIdValid + private final boolean requestedSessionIdFromCookie + private final boolean requestedSessionIdFromURL + private final String requestId + private final String protocolRequestId + private volatile String characterEncoding + private ServletInputStream inputStream + private BufferedReader reader + + private CompletedHttpServletRequest(HttpServletRequest request) { + this.attributes = new ConcurrentHashMap<>() + List<String> multipartRequests = new ArrayList<>() + for (String name : Collections.list(request.attributeNames)) { + Object value = request.getAttribute(name) + if (value instanceof MultipartHttpServletRequest) { + multipartRequests.add(name) + } + else if (value != null && !recycledWithRequest(value)) { + attributes.put(name, value) + } + } + this.parameters = Collections.unmodifiableMap(copyParameters(request)) + // After the parameters: parsing them consumes a form body, on the live request too + this.body = copyBody(request) + Map<String, List<String>> headers = new LinkedCaseInsensitiveMap<>(Locale.ENGLISH) + for (String name : Collections.list(request.headerNames)) { + headers.put(name, Collections.unmodifiableList(Collections.list(request.getHeaders(name)))) + } + this.headers = Collections.unmodifiableMap(headers) + this.locales = Collections.unmodifiableList(Collections.list(request.locales)) + this.cookies = copyCookies(request.cookies) + this.session = request.getSession(false) + this.servletContext = request.servletContext + this.userPrincipal = request.userPrincipal + this.dispatcherType = request.dispatcherType + // Values only: the container's own objects can reference the request it recycles + this.httpServletMapping = copyMapping(request.httpServletMapping) + this.servletConnection = copyConnection(request.servletConnection) + this.authType = request.authType + this.characterEncoding = request.characterEncoding + this.contentType = request.contentType + this.contentLength = request.contentLengthLong + this.method = request.method + this.requestURI = request.requestURI + this.requestURL = request.requestURL?.toString() + this.queryString = request.queryString + this.contextPath = request.contextPath + this.servletPath = request.servletPath + this.pathInfo = request.pathInfo + this.pathTranslated = request.pathTranslated + this.protocol = request.protocol + this.scheme = request.scheme + this.serverName = request.serverName + this.serverPort = request.serverPort + this.secure = request.secure + this.remoteAddr = request.remoteAddr + this.remoteHost = request.remoteHost + this.remotePort = request.remotePort + this.localName = request.localName + this.localAddr = request.localAddr + this.localPort = request.localPort + this.remoteUser = request.remoteUser + this.requestedSessionId = request.requestedSessionId + this.requestedSessionIdValid = request.requestedSessionIdValid + this.requestedSessionIdFromCookie = request.requestedSessionIdFromCookie + this.requestedSessionIdFromURL = request.requestedSessionIdFromURL + this.requestId = request.requestId + this.protocolRequestId = request.protocolRequestId + // A multipart request, such as the one the dispatcher resolved, wraps the request, so the copy, which has + // no uploads either, takes its place + for (String name : multipartRequests) { + attributes.put(name, this) + } + } + + // Field by field: the getters serve tasks, so some, such as getSession and getLocales, do not return the field + private CompletedHttpServletRequest(CompletedHttpServletRequest copy) { + this.attributes = copy.@attributes + this.body = copy.@body + this.parameters = copy.@parameters + this.headers = copy.@headers + this.locales = copy.@locales + this.cookies = copy.@cookies + this.session = copy.@session + this.servletContext = copy.@servletContext + this.userPrincipal = copy.@userPrincipal + this.dispatcherType = copy.@dispatcherType + this.httpServletMapping = copy.@httpServletMapping + this.servletConnection = copy.@servletConnection + this.authType = copy.@authType + this.characterEncoding = copy.@characterEncoding + this.contentType = copy.@contentType + this.contentLength = copy.@contentLength + this.method = copy.@method + this.requestURI = copy.@requestURI + this.requestURL = copy.@requestURL + this.queryString = copy.@queryString + this.contextPath = copy.@contextPath + this.servletPath = copy.@servletPath + this.pathInfo = copy.@pathInfo + this.pathTranslated = copy.@pathTranslated + this.protocol = copy.@protocol + this.scheme = copy.@scheme + this.serverName = copy.@serverName + this.serverPort = copy.@serverPort + this.secure = copy.@secure + this.remoteAddr = copy.@remoteAddr + this.remoteHost = copy.@remoteHost + this.remotePort = copy.@remotePort + this.localName = copy.@localName + this.localAddr = copy.@localAddr + this.localPort = copy.@localPort + this.remoteUser = copy.@remoteUser + this.requestedSessionId = copy.@requestedSessionId + this.requestedSessionIdValid = copy.@requestedSessionIdValid + this.requestedSessionIdFromCookie = copy.@requestedSessionIdFromCookie + this.requestedSessionIdFromURL = copy.@requestedSessionIdFromURL + this.requestId = copy.@requestId + this.protocolRequestId = copy.@protocolRequestId + } + + /** + * Copies a request that is completing. It has to be called before the container recycles the request. + * + * @param request the request + * @return the copy + */ + static CompletedHttpServletRequest copyOf(HttpServletRequest request) { + return new CompletedHttpServletRequest(request) + } + + /** + * A view of the copy for one task. Every view shares the copied request, including the attributes, and reads + * the one copied body through a stream or reader of its own, so what one task reads is still there for the next. + * + * @return the view + */ + CompletedHttpServletRequest newView() { + return new CompletedHttpServletRequest(this) + } + + // The request, its response and what holds either, such as the web request, are recycled with the request + private static boolean recycledWithRequest(Object value) { + return value instanceof ServletRequest || value instanceof ServletResponse || value instanceof RequestAttributes + } + + private static String[] copyValues(String[] values) { + return values == null ? null : Arrays.copyOf(values, values.length) + } + + private static Map<String, String[]> copyParameters(HttpServletRequest request) { + ServletRequest source = request + while (true) { + try { + Map<String, String[]> parameters = new LinkedHashMap<>() + for (Map.Entry<String, String[]> parameter : source.parameterMap.entrySet()) { + parameters.put(parameter.key, copyValues(parameter.value)) + } + return parameters + } + catch (RuntimeException failure) { + if (source instanceof ServletRequestWrapper) { + // A multipart wrapper resolving lazily parses the body for its parameters, which a request + // that is completing no longer offers, so the parameters come from the request it wraps + source = ((ServletRequestWrapper) source).request + } Review Comment: The copy no longer depends on the parameters being readable. They are read the way `WebUtils.readParameterMap` reads them. When the parameters of a multipart request cannot be parsed, the copy has none, which is what `params` has on the live request. Any other failure is kept and thrown to a task that reads the parameters, as the live request throws it. Everything else, headers included, is copied either way. A lazily resolving multipart wrapper still falls back to the request it wraps before any of this applies. Covered by `a multipart request the container cannot parse is copied without parameters, as params reads it` and `a request whose parameters cannot be read is still copied, and reading them fails as on the live request`. Both read a header after completion, which used to throw. -- 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]
