jamesfredley commented on code in PR #16786: URL: https://github.com/apache/grails-core/pull/16786#discussion_r4397676537
########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,563 @@ +/* + * 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.security.Principal +import java.time.ZonedDateTime +import java.time.format.DateTimeFormatter +import java.time.format.DateTimeParseException +import java.util.concurrent.ConcurrentHashMap + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncContext +import jakarta.servlet.DispatcherType +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.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, its session + * and what identifies it. Attributes a task sets stay with the copy. What only the live request can do, such as + * reading the body, dispatching, processing asynchronously, creating a session or checking a role, is not + * available.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements HttpServletRequest { + + private final Map<String, Object> attributes = new ConcurrentHashMap<>() + 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) + 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 + this.httpServletMapping = request.httpServletMapping + this.servletConnection = 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 IllegalStateException unavailable(String operation) { + return new IllegalStateException("The request has completed, so a task that outlives it cannot call $operation".toString()) + } + + @Override + Object getAttribute(String name) { + return attributes.get(name) + } + + @Override + Enumeration<String> getAttributeNames() { + return Collections.enumeration(new ArrayList<String>(attributes.keySet())) + } + + @Override + void setAttribute(String name, Object value) { + if (value == null) { + attributes.remove(name) + } + else { + attributes.put(name, value) + } + } + + @Override + void removeAttribute(String name) { + attributes.remove(name) + } + + @Override + String getCharacterEncoding() { + return characterEncoding + } + + @Override + void setCharacterEncoding(String encoding) { + this.characterEncoding = encoding + } + + @Override + int getContentLength() { + return contentLength > Integer.MAX_VALUE ? -1 : (int) contentLength + } + + @Override + long getContentLengthLong() { + return contentLength + } + + @Override + String getContentType() { + return contentType + } + + @Override + ServletInputStream getInputStream() { + throw unavailable('getInputStream()') + } + + @Override + BufferedReader getReader() { + throw unavailable('getReader()') + } + + @Override + String getParameter(String name) { + String[] values = parameters.get(name) + return values ? values[0] : null + } + + @Override + Enumeration<String> getParameterNames() { + return Collections.enumeration(parameters.keySet()) + } + + @Override + String[] getParameterValues(String name) { + return copyValues(parameters.get(name)) + } + + @Override + Map<String, String[]> getParameterMap() { + return parameters + } + + @Override + String getProtocol() { + return protocol + } + + @Override + String getScheme() { + return scheme + } + + @Override + String getServerName() { + return serverName + } + + @Override + int getServerPort() { + return serverPort + } + + @Override + String getRemoteAddr() { + return remoteAddr + } + + @Override + String getRemoteHost() { + return remoteHost + } + + @Override + int getRemotePort() { + return remotePort + } + + @Override + String getLocalName() { + return localName + } + + @Override + String getLocalAddr() { + return localAddr + } + + @Override + int getLocalPort() { + return localPort + } + + @Override + Locale getLocale() { + return locales ? locales[0] : Locale.getDefault() + } + + @Override + Enumeration<Locale> getLocales() { + return Collections.enumeration(locales ?: [Locale.getDefault()]) + } + + @Override + boolean isSecure() { + return secure + } + + @Override + RequestDispatcher getRequestDispatcher(String path) { + throw unavailable('getRequestDispatcher(String)') + } + + @Override + ServletContext getServletContext() { + return servletContext + } + + @Override + AsyncContext startAsync() { + throw unavailable('startAsync()') + } + + @Override + AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) { + throw unavailable('startAsync(ServletRequest, ServletResponse)') + } + + @Override + boolean isAsyncStarted() { + return false + } + + @Override + boolean isAsyncSupported() { + return false + } + + @Override + AsyncContext getAsyncContext() { + throw unavailable('getAsyncContext()') + } + + @Override + DispatcherType getDispatcherType() { + return dispatcherType + } + + @Override + String getRequestId() { + return requestId + } + + @Override + String getProtocolRequestId() { + return protocolRequestId + } + + @Override + ServletConnection getServletConnection() { + return servletConnection + } + + @Override + String getAuthType() { + return authType + } + + @Override + Cookie[] getCookies() { + return copyCookies(cookies) + } + + @Override + long getDateHeader(String name) { + String value = getHeader(name) + if (value == null) { + return -1L + } + try { + return ZonedDateTime.parse(value, DateTimeFormatter.RFC_1123_DATE_TIME).toInstant().toEpochMilli() Review Comment: Sunday, 06-Nov-94 08:49:37 GMT is a valid HTTP date. The live request accepts getDateHeader for it. This copy throws IllegalArgumentException because it parses only RFC 1123. Accept the same date forms the servlet request accepts. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/TaskRequest.groovy: ########## @@ -0,0 +1,142 @@ +/* + * 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.Proxy +import java.util.concurrent.atomic.AtomicLong + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncEvent +import jakarta.servlet.AsyncListener +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, 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.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class TaskRequest implements InvocationHandler, AsyncListener { + + private static final String COMPLETION_CALLBACK = TaskRequest.name + '.COMPLETION.' + private static final AtomicLong COMPLETION_CALLBACKS = new AtomicLong() + + private final HttpServletRequest request + private volatile HttpServletRequest target + + private TaskRequest(HttpServletRequest request) { + this.request = request + this.target = request + } + + /** + * 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()) && Proxy.getInvocationHandler(current) instanceof TaskRequest) { + // A task started from a task sees what that task sees + return current + } + TaskRequest taskRequest = new TaskRequest(current) + if (webRequest.isActive()) { + webRequest.registerDestructionCallback(COMPLETION_CALLBACK + COMPLETION_CALLBACKS.incrementAndGet(), + taskRequest::requestCompleted, RequestAttributes.SCOPE_REQUEST) + } + else { + taskRequest.requestCompleted() + } + return (HttpServletRequest) Proxy.newProxyInstance(TaskRequest.classLoader, [HttpServletRequest] as Class[], taskRequest) Review Comment: This proxy implements only HttpServletRequest, so a multipart request wrapped before Grails, such as by MultipartFilter, is no longer found. params and getFile lose the upload while the request is still active. Keep that wrapper visible. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/TaskRequest.groovy: ########## @@ -0,0 +1,142 @@ +/* + * 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.Proxy +import java.util.concurrent.atomic.AtomicLong + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncEvent +import jakarta.servlet.AsyncListener +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, 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.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class TaskRequest implements InvocationHandler, AsyncListener { + + private static final String COMPLETION_CALLBACK = TaskRequest.name + '.COMPLETION.' + private static final AtomicLong COMPLETION_CALLBACKS = new AtomicLong() + + private final HttpServletRequest request Review Comment: request stays for the life of the task. copy replaces target only, so after completion the listener still holds the container request. Clear that reference once the copy is published, and keep it while asynchronous processing is still active. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,563 @@ +/* + * 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.security.Principal +import java.time.ZonedDateTime +import java.time.format.DateTimeFormatter +import java.time.format.DateTimeParseException +import java.util.concurrent.ConcurrentHashMap + +import groovy.transform.CompileStatic + +import jakarta.servlet.AsyncContext +import jakarta.servlet.DispatcherType +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.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, its session + * and what identifies it. Attributes a task sets stay with the copy. What only the live request can do, such as + * reading the body, dispatching, processing asynchronously, creating a session or checking a role, is not + * available.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements HttpServletRequest { + + private final Map<String, Object> attributes = new ConcurrentHashMap<>() + 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) + 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 + this.httpServletMapping = request.httpServletMapping + this.servletConnection = 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 IllegalStateException unavailable(String operation) { + return new IllegalStateException("The request has completed, so a task that outlives it cannot call $operation".toString()) + } + + @Override + Object getAttribute(String name) { + return attributes.get(name) + } + + @Override + Enumeration<String> getAttributeNames() { + return Collections.enumeration(new ArrayList<String>(attributes.keySet())) + } + + @Override + void setAttribute(String name, Object value) { + if (value == null) { + attributes.remove(name) + } + else { + attributes.put(name, value) + } + } + + @Override + void removeAttribute(String name) { + attributes.remove(name) + } + + @Override + String getCharacterEncoding() { + return characterEncoding + } + + @Override + void setCharacterEncoding(String encoding) { + this.characterEncoding = encoding + } + + @Override + int getContentLength() { + return contentLength > Integer.MAX_VALUE ? -1 : (int) contentLength + } + + @Override + long getContentLengthLong() { + return contentLength + } + + @Override + String getContentType() { + return contentType + } + + @Override + ServletInputStream getInputStream() { Review Comment: getInputStream and getReader throw, and the copy never keeps the body. bindData of a POST {"title":"Grails"} after completion becomes a binding error instead of setting the title. Keep the body on the copy. -- 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]
