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 437e6a0e1689..9e3699e8a89d 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 @@ -15,6 +15,7 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.file.InvalidPathException; import java.time.Duration; import java.util.ArrayList; @@ -552,9 +553,18 @@ protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse // otherwise we can use the core response directly. boolean writingOrStreaming = servletContextResponse.isWritingOrStreaming(); boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || writingOrStreaming; - Response coreResponse = useServletResponse + Response r = useServletResponse ? new ServletCoreResponse(coreRequest, httpServletResponse, included) : servletChannel.getResponse(); + // Ignore last writes done via the Core API as the ServletChannel will perform the last write upon completion. + Response coreResponse = new Response.Wrapper(coreRequest, r) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + super.write(false, byteBuffer, callback); + } + }; // If the core response is already committed then do nothing more if (coreResponse.isCommitted()) 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 127bb505ce10..253e86da83ef 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 @@ -37,6 +37,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -71,10 +72,13 @@ import org.eclipse.jetty.io.content.ByteBufferContentSource; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.AllowedResourceAliasChecker; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.toolchain.test.FS; @@ -82,6 +86,7 @@ import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.resource.Resource; @@ -3757,6 +3762,48 @@ public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exceptio assertThat(filterCalled.get(), is(true)); } + @Test + public void testSingleLastWrite() throws Exception + { + server.stop(); + var rootHandler = new Handler.Wrapper(context) + { + final AtomicInteger lastWriteCounter = new AtomicInteger(); + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response = new Response.Wrapper(request, response) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (last) + lastWriteCounter.incrementAndGet(); + super.write(last, byteBuffer, callback); + } + }; + return super.handle(request, response, callback); + } + }; + server.setHandler(rootHandler); + server.start(); + + context.addServlet(DefaultServlet.class, "/"); + + Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8); + + String rawResponse = connector.getResponse(""" + GET /context/file.txt 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.toString(), response.getContent(), is("How now brown cow")); + assertThat(rootHandler.lastWriteCounter.get(), is(1)); + } + @Test public void testServeResourceAsyncNoTimeout() throws Exception { diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AsyncIOServletTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AsyncIOServletTest.java index ec11b1b7a776..1bdf78f5f757 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AsyncIOServletTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AsyncIOServletTest.java @@ -19,6 +19,8 @@ import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Deque; import java.util.Queue; import java.util.concurrent.CompletableFuture; @@ -54,6 +56,7 @@ import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.client.transport.internal.HttpConnectionOverHTTP; import org.eclipse.jetty.ee10.servlet.HttpOutput; +import org.eclipse.jetty.ee10.servlet.ResourceServlet; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; @@ -64,9 +67,11 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.logging.StacklessLogging; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.internal.HttpChannelState; +import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; @@ -77,6 +82,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -1533,6 +1539,49 @@ public void onError(Throwable x) assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); } + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testResourceServletLastWrite(TransportType transportType) throws Exception + { + prepareServer(transportType, new ResourceServlet()); + Path docRoot = workDir.getPathFile("docroot"); + FS.ensureDirExists(docRoot); + Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8); + servletContextHandler.setBaseResourceAsPath(docRoot); + + AtomicInteger lastWriteCounter = new AtomicInteger(); + server.setHandler(new Handler.Wrapper(servletContextHandler) + { + @Override + public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception + { + response = new org.eclipse.jetty.server.Response.Wrapper(request, response) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (last) + lastWriteCounter.incrementAndGet(); + super.write(last, byteBuffer, callback); + } + }; + return super.handle(request, response, callback); + } + }); + server.start(); + startClient(transportType); + + var request = client.newRequest(newURI(transportType)) + .path("/file.txt") + .method(HttpMethod.GET) + .timeout(15, TimeUnit.SECONDS); + CompletableFuture completable = new CompletableResponseListener(request).send(); + ContentResponse response = completable.get(5, TimeUnit.SECONDS); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals("How now brown cow", response.getContentAsString()); + assertEquals(1, lastWriteCounter.get()); + } + private static class Listener implements ReadListener, WriteListener { private final Executor executor = Executors.newFixedThreadPool(32); diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java index 566d81a470c7..c0540053a371 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java @@ -15,6 +15,7 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.file.InvalidPathException; import java.time.Duration; import java.util.ArrayList; @@ -564,9 +565,18 @@ protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse // otherwise we can use the core response directly. boolean writingOrStreaming = servletContextResponse.isWritingOrStreaming(); boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || writingOrStreaming; - Response coreResponse = useServletResponse + Response r = useServletResponse ? new ServletCoreResponse(coreRequest, httpServletResponse, included) : servletChannel.getResponse(); + // Ignore last writes done via the Core API as the ServletChannel will perform the last write upon completion. + Response coreResponse = new Response.Wrapper(coreRequest, r) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + super.write(false, byteBuffer, callback); + } + }; // If the core response is already committed then do nothing more if (coreResponse.isCommitted()) diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResourceServletTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResourceServletTest.java index e91d6fb64bac..3a83e9bc3fcd 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResourceServletTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResourceServletTest.java @@ -37,6 +37,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -71,10 +72,13 @@ import org.eclipse.jetty.io.content.ByteBufferContentSource; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.AllowedResourceAliasChecker; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.ResourceService; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker; import org.eclipse.jetty.toolchain.test.FS; @@ -82,6 +86,7 @@ import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; @@ -3784,6 +3789,48 @@ public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exceptio assertThat(filterCalled.get(), is(true)); } + @Test + public void testSingleLastWrite() throws Exception + { + server.stop(); + var rootHandler = new Handler.Wrapper(context) + { + final AtomicInteger lastWriteCounter = new AtomicInteger(); + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + response = new Response.Wrapper(request, response) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (last) + lastWriteCounter.incrementAndGet(); + super.write(last, byteBuffer, callback); + } + }; + return super.handle(request, response, callback); + } + }; + server.setHandler(rootHandler); + server.start(); + + context.addServlet(DefaultServlet.class, "/"); + + Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8); + + String rawResponse = connector.getResponse(""" + GET /context/file.txt 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.toString(), response.getContent(), is("How now brown cow")); + assertThat(rootHandler.lastWriteCounter.get(), is(1)); + } + @Test public void testServeResourceAsyncNoTimeout() throws Exception { diff --git a/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-client-transports/src/test/java/org/eclipse/jetty/ee11/test/client/transport/AsyncIOServletTest.java b/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-client-transports/src/test/java/org/eclipse/jetty/ee11/test/client/transport/AsyncIOServletTest.java index c075ab6f6b3f..7e314e6741c7 100644 --- a/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-client-transports/src/test/java/org/eclipse/jetty/ee11/test/client/transport/AsyncIOServletTest.java +++ b/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-client-transports/src/test/java/org/eclipse/jetty/ee11/test/client/transport/AsyncIOServletTest.java @@ -19,6 +19,8 @@ import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Deque; import java.util.Queue; import java.util.concurrent.CompletableFuture; @@ -54,6 +56,7 @@ import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.client.transport.internal.HttpConnectionOverHTTP; import org.eclipse.jetty.ee11.servlet.HttpOutput; +import org.eclipse.jetty.ee11.servlet.ResourceServlet; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; @@ -64,9 +67,11 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.logging.StacklessLogging; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.internal.HttpChannelState; +import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; @@ -77,6 +82,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -1546,6 +1552,49 @@ public void onError(Throwable x) assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); } + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testResourceServletLastWrite(TransportType transportType) throws Exception + { + prepareServer(transportType, new ResourceServlet()); + Path docRoot = workDir.getPathFile("docroot"); + FS.ensureDirExists(docRoot); + Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8); + servletContextHandler.setBaseResourceAsPath(docRoot); + + AtomicInteger lastWriteCounter = new AtomicInteger(); + server.setHandler(new Handler.Wrapper(servletContextHandler) + { + @Override + public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception + { + response = new org.eclipse.jetty.server.Response.Wrapper(request, response) + { + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (last) + lastWriteCounter.incrementAndGet(); + super.write(last, byteBuffer, callback); + } + }; + return super.handle(request, response, callback); + } + }); + server.start(); + startClient(transportType); + + var request = client.newRequest(newURI(transportType)) + .path("/file.txt") + .method(HttpMethod.GET) + .timeout(15, TimeUnit.SECONDS); + CompletableFuture completable = new CompletableResponseListener(request).send(); + ContentResponse response = completable.get(5, TimeUnit.SECONDS); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals("How now brown cow", response.getContentAsString()); + assertEquals(1, lastWriteCounter.get()); + } + private static class Listener implements ReadListener, WriteListener { private final Executor executor = Executors.newFixedThreadPool(32);