hk2166 commented on code in PR #16786: URL: https://github.com/apache/grails-core/pull/16786#discussion_r4399567761
########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,735 @@ +/* + * 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.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.util.LinkedCaseInsensitiveMap + +/** + * 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. The copy holds the attributes, parameters, headers, locales and cookies of the request, the part + * of its body nothing had read, its session and what identifies it. Attributes a task sets stay with the copy. + * What only the live request can do, such as dispatching, processing asynchronously, creating a session or + * checking a role, is not available.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements HttpServletRequest { + + 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 = new ConcurrentHashMap<>() + 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 CompletedHttpServletRequest(HttpServletRequest request) { + for (String name : Collections.list(request.attributeNames)) { + Object value = request.getAttribute(name) + if (value != null) { + attributes.put(name, value) + } + } + Map<String, String[]> parameters = new LinkedHashMap<>() + for (Map.Entry<String, String[]> parameter : request.parameterMap.entrySet()) { + parameters.put(parameter.key, copyValues(parameter.value)) + } + this.parameters = Collections.unmodifiableMap(parameters) + // 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 + } + + /** + * 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) + } + + private static String[] copyValues(String[] values) { + return values == null ? null : Arrays.copyOf(values, values.length) + } + + private static Cookie[] copyCookies(Cookie[] cookies) { + if (cookies == null) { + return null + } + Cookie[] copies = new Cookie[cookies.length] + for (int i = 0; i < cookies.length; i++) { + copies[i] = (Cookie) cookies[i].clone() + } + return copies + } + + private static byte[] copyBody(HttpServletRequest request) { + try { + InputStream remaining = request.inputStream + return remaining == null ? NO_BODY : remaining.readAllBytes() Review Comment: Fixed. Completion now takes one copy per request, however many tasks it has: the first `TaskRequest.of` call binds a shared completion state to the request (a request attribute, removed before the copy is taken) and registers the destruction callback once, so the body is read off the live stream exactly once. Each task reads that one copy through a view of its own (`CompletedHttpServletRequest.newView()`), which shares the copied data and serves the body bytes through a stream or reader of its own, so the second task reads the same `{"title":"Grails"}` as the first. Covered by `every task of the request reads the whole unread body from the one copy`, which submits two tasks and has one read the stream and the other the reader after the request completes. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,735 @@ +/* + * 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.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.util.LinkedCaseInsensitiveMap + +/** + * 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. The copy holds the attributes, parameters, headers, locales and cookies of the request, the part + * of its body nothing had read, its session and what identifies it. Attributes a task sets stay with the copy. + * What only the live request can do, such as dispatching, processing asynchronously, creating a session or + * checking a role, is not available.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements HttpServletRequest { + + 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 = new ConcurrentHashMap<>() + 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 CompletedHttpServletRequest(HttpServletRequest request) { + for (String name : Collections.list(request.attributeNames)) { + Object value = request.getAttribute(name) + if (value != null) { + attributes.put(name, value) Review Comment: Fixed. The copy now skips `org.grails.WEB_REQUEST` and any attribute whose value is the request itself, so it holds nothing the container recycles; the task does not need that attribute, because the decorator builds the task's own `GrailsWebRequest` from the values it captured at submission. The release test now binds the web request the way the filter does, through `WebUtils.storeGrailsWebRequest`, so the attribute is on the request when the copy is taken, and it also parks the request in an attribute of its own — the `WeakReference` is still collected, which it was not before this fix. The completion state also now releases the live request even when taking the copy fails, so no failure path keeps the recycled request either. -- 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]
