hk2166 commented on code in PR #16786: URL: https://github.com/apache/grails-core/pull/16786#discussion_r4414908340
########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/TaskRequest.groovy: ########## @@ -0,0 +1,690 @@ +/* + * 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 groovy.transform.PackageScope + +import jakarta.servlet.AsyncEvent +import jakarta.servlet.AsyncListener +import jakarta.servlet.ReadListener +import jakarta.servlet.ServletInputStream +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 the request the task was given, with every public view that 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. Tasks of one request can be given different requests, such as the + * request and the request of a forward, and each task keeps its own. The container recycles the request once it + * completes, including any asynchronous processing, so from then on each task sees a + * {@link CompletedHttpServletRequest copy} of its request taken as the request completed, and the recycled request + * is released. The copies are taken once for the request, however many tasks it has, and each task reads its copy + * 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> + * + * <p>A task keeps how its request was dispatched when it started: its path, the forward and include attributes and, + * for a forward or an include, its parameters, though a container that gave the forward the very wrapper the + * request had, with its own wrapper inserted underneath, restores that wrapper as the forward returns. A task that + * is reading the body when the request completes reads on from its view of the copy, where it stopped.</p> + * + * <p>A request completes when the dispatch the container started for it ends, as {@link TaskRequestFilter} tells, + * after any forward or include within it, though each of those completes the web request it bound or reused as it + * returns. Without the filter, such as in a unit test, the request completes with the web request the first of its + * tasks was started during.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class TaskRequest implements InvocationHandler { + + private static final String STATE = TaskRequest.name + '.STATE' + private static final String PROCESSING = TaskRequest.name + '.PROCESSING' + private static final String COMPLETION_CALLBACK = TaskRequest.name + '.COMPLETION' + + private final GivenRequest given + private CompletedHttpServletRequest view + private ServletInputStream body + private BufferedReader bodyReader + + private TaskRequest(GivenRequest given) { + this.given = given + } + + /** + * 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 request that task was given through a view of its own + return (HttpServletRequest) Proxy.newProxyInstance(current.getClass().classLoader, + current.getClass().interfaces, new TaskRequest(((TaskRequest) handler).given)) + } + } + ClassLoader loader = current.getClass().classLoader ?: TaskRequest.classLoader + return (HttpServletRequest) Proxy.newProxyInstance(loader, interfacesOf(current, loader), + new TaskRequest(givenOf(webRequest, current, DispatchView.of(current)))) + } + + /** + * Marks the request while the dispatch the container started for it is processed, unless an enclosing dispatch + * already marked it. + * + * @param request the request of the dispatch + * @return whether the request was marked, so the dispatch has to call {@link #processed} as it ends + */ + @PackageScope + static boolean processing(ServletRequest request) { + ServletRequest underlying = underlying(request) + if (underlying.getAttribute(PROCESSING) != null) { + return false + } + underlying.setAttribute(PROCESSING, Boolean.TRUE) + return true + } + + /** + * Ends the processing {@link #processing} marked: the requests the tasks of the request were given are copied, + * or, while the request is processed asynchronously, as that processing completes. + * + * @param request the request of the dispatch + */ + @PackageScope + static void processed(ServletRequest request) { + ServletRequest underlying = underlying(request) + underlying.removeAttribute(PROCESSING) + Object state = underlying.getAttribute(STATE) + if (state instanceof RequestState) { + ((RequestState) state).requestCompleted() + } + } + + // One state for the request, however many tasks it has and whichever request each was given, so completing it + // copies every request its tasks were given at once + private static GivenRequest givenOf(GrailsWebRequest webRequest, HttpServletRequest current, DispatchView view) { + // The state lives on the request the container created, which every wrapper of it, such as the request a + // forward dispatched, shows, so the tasks of the request find it, unless the request has completed + ServletRequest underlying = underlying(current) + GivenRequest given = boundTo(underlying)?.give(current, view) + if (given != null) { + return given + } + RequestState state = new RequestState() + given = state.give(current, view) + if (marked(underlying)) { + // Copied as the dispatch the container started ends, though a forward or an include within it binds a + // web request of its own, or reuses this one, and completes it as it returns + underlying.setAttribute(STATE, state) + } + else if (webRequest.isActive()) { + underlying.setAttribute(STATE, state) + webRequest.registerDestructionCallback(COMPLETION_CALLBACK, state::requestCompleted, + RequestAttributes.SCOPE_REQUEST) + } + else { + state.requestCompleted() + } + return given + } + + private static RequestState boundTo(ServletRequest underlying) { + try { + Object state = underlying.getAttribute(STATE) + return state instanceof RequestState ? (RequestState) state : null + } + catch (IllegalStateException ignored) { + // The container has already recycled the request + return null + } + } + + private static boolean marked(ServletRequest underlying) { + try { + return underlying.getAttribute(PROCESSING) != null + } + catch (IllegalStateException ignored) { + return false + } + } + + /** + * The request the container created, which the given request is or wraps. + * + * @param request a request + * @return the innermost request of its wrappers + */ + @PackageScope + static ServletRequest underlying(ServletRequest request) { + ServletRequest underlying = request + for (ServletRequest wrapped = unwrap(request); wrapped != null; wrapped = unwrap(wrapped)) { + underlying = wrapped + } + return underlying + } + + // Every public view of the request and of the requests it wraps, such as a multipart wrapper's, so the task + // finds what a filter resolved however the request is wrapped, as unwrapping the request finds it + private static Class<?>[] interfacesOf(HttpServletRequest request, ClassLoader loader) { + Set<Class<?>> interfaces = new LinkedHashSet<>() + interfaces.add(HttpServletRequest) + for (ServletRequest wrapped = request; wrapped != null; wrapped = unwrap(wrapped)) { + for (Class<?> type = wrapped.getClass(); type != null; type = type.superclass) { + addInterfaces(type.interfaces, loader, interfaces) + } + } + return interfaces.toArray(new Class<?>[0]) + } + + private static void addInterfaces(Class<?>[] candidates, ClassLoader loader, Set<Class<?>> interfaces) { + for (Class<?> candidate : candidates) { + // A proxy class can only implement interfaces it can access, through the class loader that defines it. + // Nor is it a Groovy object, so Groovy calls it through these interfaces, not through the meta class of + // the request's class, whose methods it does not have + if (Modifier.isPublic(candidate.modifiers) && !GroovyObject.isAssignableFrom(candidate) && + visibleFrom(loader, candidate) && interfaces.add(candidate)) { + addInterfaces(candidate.interfaces, loader, interfaces) + } + } + } + + private static boolean visibleFrom(ClassLoader loader, Class<?> type) { + try { + return Class.forName(type.name, false, loader).is(type) + } + catch (ClassNotFoundException ignored) { + return false + } + } + + // The request, or the first request it wraps that offers the method, such as a multipart request in a wrapper + private static ServletRequest offering(ServletRequest request, Class<?> type) { + for (ServletRequest candidate = request; candidate != null; candidate = unwrap(candidate)) { + if (type.isInstance(candidate)) { + return candidate + } + } + return null + } + + private static ServletRequest unwrap(ServletRequest request) { + return request instanceof ServletRequestWrapper ? ((ServletRequestWrapper) request).request : null + } + + @Override + Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if (method.name == 'equals' && method.parameterCount == 1) { + return proxy.is(args[0]) + } + if (method.name == 'hashCode' && method.parameterCount == 0) { + return System.identityHashCode(proxy) + } + if (method.parameterCount == 0 && method.name == 'getInputStream') { + return inputStream() + } + if (method.parameterCount == 0 && method.name == 'getReader') { + return reader() + } + HttpServletRequest current = given.live + if (current == null) { + current = view() + } + else if (given.view != null) { + // How the request was dispatched when the task was given it, which the container restores after a + // forward or an include + Object answer = given.view.answer(method, args, current) + if (!answer.is(DispatchView.UNANSWERED)) { + return answer + } + } + ServletRequest target = offering(current, method.declaringClass) + if (target == null) { + // Only the live request offers the views the copy does not keep, such as a custom wrapper's + throw new IllegalStateException('The request has completed, so a task that outlives it cannot call ' + + "${method.declaringClass.simpleName}.${method.name}, which the copy of the request does not keep") + } + try { + return method.invoke(target, args) + } + catch (InvocationTargetException e) { + throw e.targetException + } + } + + @PackageScope + synchronized CompletedHttpServletRequest view() { + if (view == null) { + view = given.copyView() + } + return view + } + + // The body as the live request serves it until the request is copied, and then from the task's view of the + // copy, which starts where the live request stopped, so a task reading it when the request completes goes on + private synchronized ServletInputStream inputStream() { + if (body == null) { + if (bodyReader != null) { + throw new IllegalStateException('getReader() has already been called, so the body is not available as a stream') + } + synchronized (given.bodyLock) { + HttpServletRequest live = given.live + if (live == null) { + return view().inputStream + } + body = new TaskInputStream(given, this, live.inputStream) + } + } + return body + } + + private synchronized BufferedReader reader() { + if (bodyReader == null) { + if (body != null) { + throw new IllegalStateException('getInputStream() has already been called, so the body is not available as a reader') + } + synchronized (given.bodyLock) { + HttpServletRequest live = given.live + if (live == null) { + return view().reader + } + bodyReader = new TaskReader(given, this, live.reader) + } + } + return bodyReader + } + + // What the tasks of one request share: its completion, which copies every request they were given at once + private static final class RequestState implements AsyncListener { + + private final List<GivenRequest> given = new ArrayList<>() + // Held while a task reads the live body and while the requests are copied, so a read and the copy of the + // body do not interleave + private final Object body = new Object() + private boolean completed + + // The request a task was given, shared by the tasks given the same request dispatched the same way, or null + // once the request has completed + private synchronized GivenRequest give(HttpServletRequest request, DispatchView view) { + if (completed) { + return null + } + for (GivenRequest existing : given) { + if (existing.live.is(request) && DispatchView.same(existing.view, view)) { + return existing + } + } + GivenRequest added = new GivenRequest(request, view, body) + given.add(added) + return added + } + + private synchronized HttpServletRequest first() { + return completed ? null : given.get(0).live + } + + private void requestCompleted() { + HttpServletRequest current = first() + if (current == null) { + return + } + try { + if (current.isAsyncStarted()) { + // The response is still being produced, so a task it waits for may still be rendering it. The + // asynchronous processing completes on a thread outside the filters, so who made the request is + // taken now, while a security filter still tells + rememberIdentities() + current.asyncContext.addListener(this) + return + } + } + catch (IllegalStateException ignored) { + // Too late to be told when the asynchronous processing completes, so the request is copied now + } + copyRequests(false) + } + + private synchronized void rememberIdentities() { + for (GivenRequest request : given) { + try { + request.identity = new CompletedHttpServletRequest.Identity(request.live) + } + catch (RuntimeException ignored) { + // The copy reads who made the request from the request itself + } + } + } + + // Copied inside the dispatch that ends, or, once the asynchronous processing completes, outside it, with + // who made the request as the last dispatch told + private synchronized void copyRequests(boolean outsideDispatch) { + if (completed) { + return + } + completed = true + List<HttpServletRequest> requests = new ArrayList<>() + List<DispatchView> views = new ArrayList<>() + List<CompletedHttpServletRequest.Identity> identities = new ArrayList<>() + for (GivenRequest request : given) { + requests.add(request.live) + views.add(request.view) + identities.add(outsideDispatch ? request.identity : null) + } + // A task reading the live body finishes its read first, and goes on from the copy + synchronized (body) { + try { + underlying(requests.get(0)).removeAttribute(STATE) + List<CompletedHttpServletRequest> copies = CompletedHttpServletRequest.copyOf(requests, views, identities) + for (int i = 0; i < copies.size(); i++) { + given.get(i).copy = copies.get(i) + } + } + catch (RuntimeException ignored) { + // The container has already recycled the request, so there is nothing left to copy + } + finally { + // Released whether or not the copies were taken, so no task holds the recycled request + for (GivenRequest request : given) { + request.live = null + } + } + } + } + + @Override + void onComplete(AsyncEvent event) { + copyRequests(true) + } + + @Override + void onTimeout(AsyncEvent event) { + } + + @Override + void onError(AsyncEvent event) { + } + + @Override + void onStartAsync(AsyncEvent event) { + // Starting another asynchronous cycle removes the listeners of the one before + event.asyncContext.addListener(this) + } + } + + // A request tasks were given: the request itself until the request completes, then the copy of it + private static final class GivenRequest { + + @PackageScope + final DispatchView view + + @PackageScope + final Object bodyLock + + @PackageScope + volatile HttpServletRequest live + + @PackageScope + volatile CompletedHttpServletRequest copy + + @PackageScope + volatile CompletedHttpServletRequest.Identity identity + + @PackageScope + GivenRequest(HttpServletRequest live, DispatchView view, Object bodyLock) { + this.live = live + this.view = view + this.bodyLock = bodyLock + } + + @PackageScope + CompletedHttpServletRequest copyView() { + CompletedHttpServletRequest completed = copy + if (completed == null) { + throw new IllegalStateException( + 'The request has completed, and it was recycled before it could be copied, so a task that outlives it cannot read it') + } + return completed.newView() + } + } + + // The body stream of a task: the live request's until the request is copied, then the task's view of the copy, + // which holds what the live stream had not served yet + private static final class TaskInputStream extends ServletInputStream { + + private final GivenRequest given + private final TaskRequest task + private final ServletInputStream live + + private TaskInputStream(GivenRequest given, TaskRequest task, ServletInputStream live) { + this.given = given + this.task = task + this.live = live + } + + @Override + int read() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.read() + } + } + return copied().read() + } + + @Override + int read(byte[] destination, int offset, int length) throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.read(destination, offset, length) + } + } + return copied().read(destination, offset, length) + } + + @Override + long skip(long count) throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.skip(count) + } + } + return copied().skip(count) + } + + @Override + int available() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.available() + } + } + return copied().available() + } + + @Override + void close() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + live.close() + return + } + } + copied().close() + } + + @Override + boolean isFinished() { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.isFinished() + } + } + return task.view().inputStream.isFinished() + } + + @Override + boolean isReady() { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.isReady() + } + } + return task.view().inputStream.isReady() + } + + @Override + void setReadListener(ReadListener readListener) { + synchronized (given.bodyLock) { + if (given.live != null) { + live.setReadListener(readListener) + return + } + } + task.view().inputStream.setReadListener(readListener) + } + + private ServletInputStream copied() throws IOException { + try { + return task.view().inputStream + } + catch (IllegalStateException unavailable) { + throw new IOException(unavailable.message, unavailable) + } + } + } + + // The body reader of a task: the live request's until the request is copied, then the task's view of the copy, + // which holds what the live reader had not served yet + private static final class TaskReader extends BufferedReader { + + private final GivenRequest given + private final TaskRequest task + private final BufferedReader live + + private TaskReader(GivenRequest given, TaskRequest task, BufferedReader live) { + super(Reader.nullReader()) + this.given = given + this.task = task + this.live = live + } + + @Override + int read() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.read() + } + } + return copied().read() + } + + @Override + int read(char[] destination, int offset, int length) throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.read(destination, offset, length) + } + } + return copied().read(destination, offset, length) + } + + @Override + String readLine() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.readLine() + } + } + return copied().readLine() + } + + @Override + long skip(long count) throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.skip(count) + } + } + return copied().skip(count) + } + + @Override + boolean ready() throws IOException { + synchronized (given.bodyLock) { + if (given.live != null) { + return live.ready() + } + } + return copied().ready() + } + + @Override + boolean markSupported() { Review Comment: Fixed in 3847d6f006. The task's reader now keeps its own mark, as a request's reader supports one. What was read since the mark is kept and served again after `reset()`, while the request is live and across the copy, since the mark belongs to the task's reader, not to the live reader or the copy. The task's stream does the same when the live stream supports a mark (Tomcat's doesn't, and then the task's stream doesn't either). A line ended by `\r` is returned without waiting for the next character, and a `\n` right after it is dropped on the next read, as `BufferedReader` does; that state is restored by `reset()` too. `mark(Integer.MAX_VALUE)` works, and a closed reader or stream stays closed after the copy. Tests: - `a task marks and resets the body reader while the request is live, and across its completion` (your case, then a mark held across the completion). - `a task marks and resets the body stream when the live stream supports it, across the completion of the request`. - `the body reader of a task keeps a line ended by a carriage return and its line feed within a mark, as BufferedReader does` (`mark(5)` on `abcd\r\nxy`, and `mark(Integer.MAX_VALUE)`). - `a line ended by a carriage return is read without waiting for what follows, as BufferedReader reads it`. - `a body reader a task closed stays closed once the request has completed`. All fail on 48f61a443b. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/TaskRequest.groovy: ########## @@ -0,0 +1,690 @@ +/* + * 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 groovy.transform.PackageScope + +import jakarta.servlet.AsyncEvent +import jakarta.servlet.AsyncListener +import jakarta.servlet.ReadListener +import jakarta.servlet.ServletInputStream +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 the request the task was given, with every public view that 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. Tasks of one request can be given different requests, such as the + * request and the request of a forward, and each task keeps its own. The container recycles the request once it + * completes, including any asynchronous processing, so from then on each task sees a + * {@link CompletedHttpServletRequest copy} of its request taken as the request completed, and the recycled request + * is released. The copies are taken once for the request, however many tasks it has, and each task reads its copy + * 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> + * + * <p>A task keeps how its request was dispatched when it started: its path, the forward and include attributes and, + * for a forward or an include, its parameters, though a container that gave the forward the very wrapper the + * request had, with its own wrapper inserted underneath, restores that wrapper as the forward returns. A task that + * is reading the body when the request completes reads on from its view of the copy, where it stopped.</p> + * + * <p>A request completes when the dispatch the container started for it ends, as {@link TaskRequestFilter} tells, + * after any forward or include within it, though each of those completes the web request it bound or reused as it + * returns. Without the filter, such as in a unit test, the request completes with the web request the first of its + * tasks was started during.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class TaskRequest implements InvocationHandler { + + private static final String STATE = TaskRequest.name + '.STATE' + private static final String PROCESSING = TaskRequest.name + '.PROCESSING' + private static final String COMPLETION_CALLBACK = TaskRequest.name + '.COMPLETION' + + private final GivenRequest given + private CompletedHttpServletRequest view + private ServletInputStream body + private BufferedReader bodyReader + + private TaskRequest(GivenRequest given) { + this.given = given + } + + /** + * 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 request that task was given through a view of its own + return (HttpServletRequest) Proxy.newProxyInstance(current.getClass().classLoader, + current.getClass().interfaces, new TaskRequest(((TaskRequest) handler).given)) + } + } + ClassLoader loader = current.getClass().classLoader ?: TaskRequest.classLoader + return (HttpServletRequest) Proxy.newProxyInstance(loader, interfacesOf(current, loader), + new TaskRequest(givenOf(webRequest, current, DispatchView.of(current)))) + } + + /** + * Marks the request while the dispatch the container started for it is processed, unless an enclosing dispatch + * already marked it. + * + * @param request the request of the dispatch + * @return whether the request was marked, so the dispatch has to call {@link #processed} as it ends + */ + @PackageScope + static boolean processing(ServletRequest request) { + ServletRequest underlying = underlying(request) + if (underlying.getAttribute(PROCESSING) != null) { + return false + } + underlying.setAttribute(PROCESSING, Boolean.TRUE) + return true + } + + /** + * Ends the processing {@link #processing} marked: the requests the tasks of the request were given are copied, + * or, while the request is processed asynchronously, as that processing completes. + * + * @param request the request of the dispatch + */ + @PackageScope + static void processed(ServletRequest request) { + ServletRequest underlying = underlying(request) + underlying.removeAttribute(PROCESSING) + Object state = underlying.getAttribute(STATE) + if (state instanceof RequestState) { + ((RequestState) state).requestCompleted() + } + } + + // One state for the request, however many tasks it has and whichever request each was given, so completing it + // copies every request its tasks were given at once + private static GivenRequest givenOf(GrailsWebRequest webRequest, HttpServletRequest current, DispatchView view) { + // The state lives on the request the container created, which every wrapper of it, such as the request a + // forward dispatched, shows, so the tasks of the request find it, unless the request has completed + ServletRequest underlying = underlying(current) + GivenRequest given = boundTo(underlying)?.give(current, view) + if (given != null) { + return given + } + RequestState state = new RequestState() + given = state.give(current, view) + if (marked(underlying)) { + // Copied as the dispatch the container started ends, though a forward or an include within it binds a + // web request of its own, or reuses this one, and completes it as it returns + underlying.setAttribute(STATE, state) + } + else if (webRequest.isActive()) { + underlying.setAttribute(STATE, state) + webRequest.registerDestructionCallback(COMPLETION_CALLBACK, state::requestCompleted, + RequestAttributes.SCOPE_REQUEST) + } + else { + state.requestCompleted() + } + return given + } + + private static RequestState boundTo(ServletRequest underlying) { + try { + Object state = underlying.getAttribute(STATE) + return state instanceof RequestState ? (RequestState) state : null + } + catch (IllegalStateException ignored) { + // The container has already recycled the request + return null + } + } + + private static boolean marked(ServletRequest underlying) { + try { + return underlying.getAttribute(PROCESSING) != null + } + catch (IllegalStateException ignored) { + return false + } + } + + /** + * The request the container created, which the given request is or wraps. + * + * @param request a request + * @return the innermost request of its wrappers + */ + @PackageScope + static ServletRequest underlying(ServletRequest request) { + ServletRequest underlying = request + for (ServletRequest wrapped = unwrap(request); wrapped != null; wrapped = unwrap(wrapped)) { + underlying = wrapped + } + return underlying + } + + // Every public view of the request and of the requests it wraps, such as a multipart wrapper's, so the task + // finds what a filter resolved however the request is wrapped, as unwrapping the request finds it + private static Class<?>[] interfacesOf(HttpServletRequest request, ClassLoader loader) { + Set<Class<?>> interfaces = new LinkedHashSet<>() + interfaces.add(HttpServletRequest) + for (ServletRequest wrapped = request; wrapped != null; wrapped = unwrap(wrapped)) { + for (Class<?> type = wrapped.getClass(); type != null; type = type.superclass) { + addInterfaces(type.interfaces, loader, interfaces) + } + } + return interfaces.toArray(new Class<?>[0]) + } + + private static void addInterfaces(Class<?>[] candidates, ClassLoader loader, Set<Class<?>> interfaces) { + for (Class<?> candidate : candidates) { + // A proxy class can only implement interfaces it can access, through the class loader that defines it. + // Nor is it a Groovy object, so Groovy calls it through these interfaces, not through the meta class of + // the request's class, whose methods it does not have + if (Modifier.isPublic(candidate.modifiers) && !GroovyObject.isAssignableFrom(candidate) && + visibleFrom(loader, candidate) && interfaces.add(candidate)) { + addInterfaces(candidate.interfaces, loader, interfaces) + } + } + } + + private static boolean visibleFrom(ClassLoader loader, Class<?> type) { + try { + return Class.forName(type.name, false, loader).is(type) + } + catch (ClassNotFoundException ignored) { + return false + } + } + + // The request, or the first request it wraps that offers the method, such as a multipart request in a wrapper + private static ServletRequest offering(ServletRequest request, Class<?> type) { + for (ServletRequest candidate = request; candidate != null; candidate = unwrap(candidate)) { + if (type.isInstance(candidate)) { + return candidate + } + } + return null + } + + private static ServletRequest unwrap(ServletRequest request) { + return request instanceof ServletRequestWrapper ? ((ServletRequestWrapper) request).request : null + } + + @Override + Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + if (method.name == 'equals' && method.parameterCount == 1) { + return proxy.is(args[0]) + } + if (method.name == 'hashCode' && method.parameterCount == 0) { + return System.identityHashCode(proxy) + } + if (method.parameterCount == 0 && method.name == 'getInputStream') { + return inputStream() + } + if (method.parameterCount == 0 && method.name == 'getReader') { + return reader() + } + HttpServletRequest current = given.live Review Comment: Fixed in 3847d6f006. Every call a task makes on the live request now takes the read side of a `ReentrantReadWriteLock`, and the copy holds the write side from taking the attributes to switching the tasks to their copies. Tasks still call the live request at the same time. A task's `setAttribute` either finishes before the copy starts, so the copy has it, or waits for the copy and then goes to the copy. Calls the container can call back on, `setReadListener` and `close`, are made outside the lock, so a listener that completes the request on that thread can't hang. A task blocked reading a slow upload still holds up the copy until that read returns, as it already did. Test: `an attribute a task sets while the request is being copied is in the copy`. The copy is held inside the body read until the task setting the attribute is waiting on, or done with, the request. It fails on 48f61a443b, where the attribute went to the live request after the copy had taken the attributes. ########## grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/CompletedHttpServletRequest.groovy: ########## @@ -0,0 +1,1195 @@ +/* + * 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 groovy.transform.PackageScope + +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 + +import org.grails.web.util.WebUtils + +/** + * 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. As the request completes, a copy is taken of each request its tasks were given, such as the request + * and the request of a forward, and each task reads the copy of its request 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. A copy holds the attributes, parameters, headers, locales and cookies of + * its request, the session and what identifies the 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>A copy keeps how its request was dispatched when its task was given it, its path, forward and include + * attributes and, for a forward or an include, its parameters, though the container restores a request it wrapped + * for a forward as the forward returns. A copy taken as asynchronous processing completes, outside the filters, + * keeps who made the request as its last dispatch told, since a security filter clears that as the dispatch + * ends.</p> + * + * <p>The copies share the attributes of the request the container created and the body, as the requests share + * them, so attributes a task sets stay with the copies, shared by every task. What a request keeps itself, such as + * the forward attributes of the request a forward dispatched, stays with its own copy, as do the attributes it + * hides, and setting or removing one of them changes that copy only, as on the request.</p> + * + * <p>The parameters are those of the request. A multipart body the container could not parse leaves the copy + * without parameters, as params has none, and any other failure to read them is thrown to a task that reads them, + * as the live request throws it. Only a multipart request resolving lazily that can no longer parse the body as the + * request completes gives way to the request it wraps.</p> + * + * <p>The uploads of a multipart request are deleted as the request ends, so the copy answers as a multipart + * request without uploads when the request was a multipart request, and a copy of a multipart request an attribute + * held, such as the one the dispatcher resolved, takes its place, with its parameters, such as the text fields only + * it has. Reading params then finds the parameters it found on the live request and no uploads.</p> + * + * <p>The body is kept up to {@value #BODY_LIMIT} bytes. Reading a longer one from the copy throws, so the copy is + * not what holds an upload that was never read in memory. The body of a multipart request is not kept: its uploads + * go with the request, and its text fields are in the parameters.</p> + * + * @since 8.0.1 + */ +@CompileStatic +class CompletedHttpServletRequest implements MultipartHttpServletRequest { + + /** + * The longest body, in bytes, a copy keeps: what remains of a longer one is not read. + */ + static final int BODY_LIMIT = 2 * 1024 * 1024 + + private static final byte[] NO_BODY = new byte[0] + private static final int READ_CHUNK = 8192 + // An attribute the request of a copy hides, though the request the container created has it + private static final Object HIDDEN = new Object() + 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 Map<String, Object> ownAttributes + // Null when the body was longer than the copy keeps + private final byte[] body + private final Map<String, String[]> parameters + private final RuntimeException parameterFailure + 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, Map<String, Object> attributes, + Map<String, Object> ownAttributes, CopiedParameters parameters, byte[] body, DispatchView view, + Identity identity) { + this.attributes = attributes + this.ownAttributes = ownAttributes + this.parameters = parameters.values + this.parameterFailure = parameters.failure + this.body = body + 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 + // As the request was dispatched when the task was given it, though the container restores a request it + // wrapped for a forward as the forward returns + DispatchView dispatch = view ?: new DispatchView(request) + // Who made the request, as the filters processing it told, though a security filter clears it + Identity user = identity ?: new Identity(request) + this.userPrincipal = user.userPrincipal + this.dispatcherType = dispatch.dispatcherType + this.httpServletMapping = dispatch.httpServletMapping + this.servletConnection = copyConnection(request.servletConnection) + this.authType = user.authType + this.characterEncoding = request.characterEncoding + this.contentType = request.contentType + this.contentLength = request.contentLengthLong + this.method = request.method + this.requestURI = dispatch.requestURI + this.requestURL = dispatch.requestURL + this.queryString = dispatch.queryString + this.contextPath = dispatch.contextPath + this.servletPath = dispatch.servletPath + this.pathInfo = dispatch.pathInfo + this.pathTranslated = dispatch.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 = user.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 + } + + // 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.ownAttributes = copy.@ownAttributes + this.body = copy.@body + this.parameters = copy.@parameters + this.parameterFailure = copy.@parameterFailure + 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 the requests the tasks of a request that is completing were given: the request, and any other request + * a later task was given, such as the request of a forward. It has to be called before the container recycles + * the request. Each copy holds what its request shows, and the copies share the attributes of the request the + * container created and the body, as the requests share them. + * + * @param requests the requests, each the request the container created or a wrapper of it, the first being the + * one whose body the copies share + * @param views how each request was dispatched when a task was given it + * @param identities who made the request, as the filters processing it told, for each request, or {@code null} + * entries when the request itself still tells + * @return the copies, in the order of the requests, with {@code null} for a request after the first that could + * not be copied + */ + static List<CompletedHttpServletRequest> copyOf(List<HttpServletRequest> requests, List<DispatchView> views, + List<Identity> identities) { + Map<String, Object> underlying = attributesOf(TaskRequest.underlying(requests.get(0))) + // The requests to copy: those given, then each multipart request an attribute holds, such as the one the + // dispatcher resolved, which wraps the request, so its copy, which has no uploads either, takes its place + List<HttpServletRequest> copied = new ArrayList<>(requests) + Map<Object, Integer> multipartRequests = new IdentityHashMap<>() + for (int i = 0; i < requests.size(); i++) { + if (requests.get(i) instanceof MultipartHttpServletRequest) { + multipartRequests.putIfAbsent(requests.get(i), i) + } + } + addMultipartRequests(underlying, copied, multipartRequests) + List<Map<String, Object>> shownOtherwise = new ArrayList<>() + for (int i = 0; i < copied.size(); i++) { + Map<String, Object> shown = differences(attributesOf(copied.get(i)), underlying, viewOf(views, i)) + shownOtherwise.add(shown) + addMultipartRequests(shown, copied, multipartRequests) + } + // Every parameter map before the body: parsing the parameters consumes a form body, on the live request too + List<CopiedParameters> parameters = new ArrayList<>() + for (int i = 0; i < copied.size(); i++) { + parameters.add(viewOf(views, i)?.parameters ?: copyParameters(copied.get(i))) + } + byte[] body = copyBody(requests.get(0)) + Map<String, Object> attributes = new ConcurrentHashMap<>() + List<Map<String, Object>> ownAttributes = new ArrayList<>() + List<CompletedHttpServletRequest> copies = new ArrayList<>() + for (int i = 0; i < copied.size(); i++) { + Map<String, Object> own = new ConcurrentHashMap<>() + ownAttributes.add(own) + Identity identity = i < identities.size() ? identities.get(i) : null + copies.add(i == 0 ? + new CompletedHttpServletRequest(copied.get(0), attributes, own, parameters.get(0), body, + viewOf(views, 0), identity) : + copyAlongside(copied.get(i), attributes, own, parameters.get(i), body, viewOf(views, i), identity)) + } + putAttributes(underlying, attributes, multipartRequests, copies, false) + for (int i = 0; i < copied.size(); i++) { + putAttributes(shownOtherwise.get(i), ownAttributes.get(i), multipartRequests, copies, true) + } + return new ArrayList<>(copies.subList(0, requests.size())) + } + + // A multipart request an attribute holds was not given to a task, so it has no view + private static DispatchView viewOf(List<DispatchView> views, int index) { + return index < views.size() ? views.get(index) : null + } + + // A request that can no longer be read leaves its own tasks without a copy, not the tasks of the other requests + private static CompletedHttpServletRequest copyAlongside(HttpServletRequest request, Map<String, Object> attributes, + Map<String, Object> ownAttributes, CopiedParameters parameters, byte[] body, DispatchView view, + Identity identity) { + try { + return new CompletedHttpServletRequest(request, attributes, ownAttributes, parameters, body, view, identity) + } + catch (RuntimeException ignored) { + return null + } + } + + private static Map<String, Object> attributesOf(ServletRequest request) { + Map<String, Object> attributes = new LinkedHashMap<>() + for (String name : Collections.list(request.attributeNames)) { + Object value = request.getAttribute(name) + if (value != null) { + attributes.put(name, value) + } + } + return attributes + } + + // What a request shows otherwise than the request the container created, such as the forward attributes a + // forward's request keeps itself: its own values, and the names it hides. The forward and include attributes + // are those it showed when a task was given it, as the container restores a request it wrapped for a forward + private static Map<String, Object> differences(Map<String, Object> shown, Map<String, Object> underlying, + DispatchView view) { + Map<String, Object> differences = new LinkedHashMap<>() + Map<String, Object> dispatched = view == null ? shown : view.attributes + for (Map.Entry<String, Object> attribute : shown.entrySet()) { + if (!(view != null && DispatchView.dispatchAttribute(attribute.key)) && + !Objects.equals(attribute.value, underlying.get(attribute.key))) { + differences.put(attribute.key, attribute.value) + } + } + if (view != null) { + for (Map.Entry<String, Object> attribute : dispatched.entrySet()) { + if (!Objects.equals(attribute.value, underlying.get(attribute.key))) { + differences.put(attribute.key, attribute.value) + } + } + } + for (String name : underlying.keySet()) { + boolean shownByView = view != null && DispatchView.dispatchAttribute(name) + if (!(shownByView ? dispatched : shown).containsKey(name)) { + differences.put(name, HIDDEN) + } + } + return differences + } + + private static void addMultipartRequests(Map<String, Object> attributes, List<HttpServletRequest> copied, + Map<Object, Integer> multipartRequests) { + for (Object value : attributes.values()) { + if (value instanceof MultipartHttpServletRequest && !multipartRequests.containsKey(value)) { + multipartRequests.put(value, copied.size()) + copied.add((HttpServletRequest) value) + } + } + } + + // The attributes as the copies keep them: a multipart request is replaced by its copy, and what is recycled with + // the request is left out, so a request that showed it itself hides it + private static void putAttributes(Map<String, Object> attributes, Map<String, Object> kept, + Map<Object, Integer> multipartRequests, List<CompletedHttpServletRequest> copies, boolean own) { + for (Map.Entry<String, Object> attribute : attributes.entrySet()) { + Object value = attribute.value + if (value instanceof MultipartHttpServletRequest) { + value = copies.get(multipartRequests.get(value)) + } + else if (recycledWithRequest(value)) { + value = null + } + if (value != null) { + kept.put(attribute.key, value) + } + else if (own) { + kept.put(attribute.key, HIDDEN) + } + } + } + + /** + * 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) + } + + /** + * The parameters of a request, as WebUtils.readParameterMap reads them: a multipart body the container cannot + * parse leaves the request without parameters, and any other failure reaches the task that reads them, as on + * the request. + * + * @param request the request + * @return its parameters, or the failure reading them + */ + @PackageScope + static CopiedParameters copyParameters(HttpServletRequest request) { + try { + Map<String, String[]> parameters = new LinkedHashMap<>() + for (Map.Entry<String, String[]> parameter : ownParameters(request).entrySet()) { + parameters.put(parameter.key, copyValues(parameter.value)) + } + return new CopiedParameters(Collections.unmodifiableMap(parameters), null) + } + catch (RuntimeException unreadable) { + return new CopiedParameters(Collections.<String, String[]> emptyMap(), + WebUtils.isMultipartContentType(request) ? null : unreadable) + } + } + + // The parameters of the request or, when a multipart wrapper resolving lazily can no longer parse the body for + // them as the request completes, of the request it wraps. Any other wrapper that fails keeps its failure, as it + // fails on the live request + private static Map<String, String[]> ownParameters(HttpServletRequest request) { + RuntimeException failure = null + for (ServletRequest source = request; source != null; source = wrapped(source)) { + try { + return source.parameterMap + } + catch (RuntimeException unreadable) { + failure = failure ?: unreadable + if (!(source instanceof MultipartHttpServletRequest)) { + throw failure + } + } + } + throw failure + } + + private static ServletRequest wrapped(ServletRequest request) { + return request instanceof ServletRequestWrapper ? ((ServletRequestWrapper) request).request : null + } + + 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 + } + + // What remains of the body, or null when it is longer than the copy keeps + private static byte[] copyBody(HttpServletRequest request) { + if (WebUtils.isMultipartContentType(request)) { + // Its uploads go with the request and its text fields are in the parameters, so a multipart body the + // request did not read, such as one rejected for its size, is left unread + return NO_BODY + } + try { Review Comment: Fixed in 3847d6f006. The copy now reads the rest of the body from the stream or reader a task took, the one it is reading, since a reader can hold what it read ahead and a wrapper can hand out a new reader each time. Text is encoded as that reader decoded it, with the encoding of the request the task took it from. When no task took one and the request's stream has nothing left, what a reader of the request still holds is the rest, as with `ContentCachingRequestWrapper`. On Tomcat, `getReader()` after `getInputStream()` throws, and the copy then has no body, as before. A failure reading the rest of the body, such as an aborted upload or a wrapper rejecting a body too long for it, is thrown by a read past what was copied, as on the request, rather than ending the body early or dropping the copy. Tests: - `the copy keeps what a reader that read ahead holds`, with the first line read by a task and by the request, using `ContentCachingRequestWrapper`. - `the copy reads the rest of the body from the reader a task took, though the request hands out a new reader each time`. - `the copy keeps the rest of a body a task reads through a reader in the encoding that reader decodes`. - `a body whose rest cannot be read as the request completes fails a read from the copy, as on the request, not ending early`. - `a body a wrapper fails to read as the request completes fails a task reading it, and the rest of the request is copied`. All fail on 48f61a443b. -- 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]
