From 741ac12ee54941376249e4892e2d5d838ace79fd Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Tue, 3 Dec 2024 17:34:54 +0100 Subject: [PATCH] #12153 fix serving content async even when a filter already called startAsync Signed-off-by: Ludovic Orban --- .../jetty/ee10/servlet/ResourceServlet.java | 2 +- .../ee10/servlet/ResourceServletTest.java | 35 +++++++++++++++++++ .../jetty/ee9/nested/ResourceService.java | 2 +- .../jetty/ee9/servlet/DefaultServletTest.java | 35 +++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java index 7d07829c0e7d..2a6f04976d1c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java @@ -518,7 +518,7 @@ protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse (contentLength < 0 || contentLength > coreRequest.getConnectionMetaData().getHttpConfiguration().getOutputBufferSize())) { // send the content asynchronously - AsyncContext asyncContext = httpServletRequest.startAsync(); + AsyncContext asyncContext = httpServletRequest.isAsyncStarted() ? httpServletRequest.getAsyncContext() : httpServletRequest.startAsync(); Callback callback = new AsyncContextCallback(asyncContext, httpServletResponse); _resourceService.doGet(coreRequest, coreResponse, callback, content); } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResourceServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResourceServletTest.java index 9e3dffbb932f..7ebb934b31ae 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResourceServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResourceServletTest.java @@ -30,6 +30,7 @@ import java.util.Arrays; import java.util.EnumSet; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -38,6 +39,7 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; +import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; @@ -3773,6 +3775,39 @@ public void testNotAcceptRanges() throws Exception assertThat(response.getContent(), is("Test 2 to too two\n")); } + @Test + public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exception + { + // The OutputBufferSize must be smaller than the content length otherwise the request is not served async. + connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setOutputBufferSize(0); + + AtomicBoolean filterCalled = new AtomicBoolean(); + context.addFilter((request, response, chain) -> + { + filterCalled.set(true); + AsyncContext asyncContext = request.startAsync(); + chain.doFilter(request, response); + asyncContext.complete(); + }, "/*", EnumSet.of(DispatcherType.REQUEST)); + + ResourceServlet resourceServlet = new ResourceServlet(); + context.addServlet(resourceServlet, "/*"); + Resource memResource = ResourceFactory.of(context).newMemoryResource(getClass().getResource("/contextResources/test.txt")); + resourceServlet.getResourceService().setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain")); + + String rawResponse = connector.getResponse(""" + GET /context/ HTTP/1.1\r + Host: local\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("18")); + assertThat(response.getContent(), is("Test 2 to too two\n")); + assertThat(filterCalled.get(), is(true)); + } + public static class WriterFilter implements Filter { @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java index 2daf042dc137..eae424f2effd 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceService.java @@ -735,7 +735,7 @@ else if (written) // write the content asynchronously if supported if (request.isAsyncSupported()) { - final AsyncContext context = request.startAsync(); + AsyncContext context = request.isAsyncStarted() ? request.getAsyncContext() : request.startAsync(); context.setTimeout(0); ((HttpOutput)out).sendContent(content, new Callback() diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java index 1cce0c93b995..f27ab698b1c5 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.EnumSet; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -34,6 +35,7 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; +import jakarta.servlet.AsyncContext; import jakarta.servlet.DispatcherType; import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; @@ -2783,6 +2785,39 @@ public void testMemoryResourceRange() throws Exception assertThat(response.getContent(), is("too")); } + @Test + public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exception + { + AtomicBoolean filterCalled = new AtomicBoolean(); + startServer((context) -> + { + Resource memResource = ResourceFactory.of(context).newMemoryResource(getClass().getResource("/contextResources/test.txt")); + ResourceService resourceService = new ResourceService(); + resourceService.setHttpContentFactory(path -> new ResourceHttpContent(memResource, "text/plain")); + DefaultServlet defaultServlet = new DefaultServlet(resourceService); + context.addServlet(new ServletHolder(defaultServlet), "/*"); + context.addFilter(new FilterHolder((request, response, chain) -> + { + filterCalled.set(true); + AsyncContext asyncContext = request.startAsync(); + chain.doFilter(request, response); + asyncContext.complete(); + }), "/*", EnumSet.of(DispatcherType.REQUEST)); + }); + + String rawResponse = connector.getResponse(""" + GET /context/ HTTP/1.1\r + Host: local\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.get(HttpHeader.CONTENT_LENGTH), is("17")); + assertThat(response.getContent(), is("Test 2 to too two")); + assertThat(filterCalled.get(), is(true)); + } + @Test public void testMemoryResourceMultipleRanges() throws Exception {