Skip to content

Commit 63ddea9

Browse files
lorbanlorban
authored
#13849 ignore last writes done via the Core API as the ServletChannel will perform the last write upon completion
Signed-off-by: Ludovic Orban <[email protected]>
1 parent 8ecdbb4 commit 63ddea9

File tree

6 files changed

+214
-2
lines changed

6 files changed

+214
-2
lines changed

‎jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import java.io.FileNotFoundException;
1717
import java.io.IOException;
18+
import java.nio.ByteBuffer;
1819
import java.nio.file.InvalidPathException;
1920
import java.time.Duration;
2021
import java.util.ArrayList;
@@ -552,9 +553,18 @@ protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse
552553
// otherwise we can use the core response directly.
553554
boolean writingOrStreaming = servletContextResponse.isWritingOrStreaming();
554555
boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || writingOrStreaming;
555-
Response coreResponse = useServletResponse
556+
Response r = useServletResponse
556557
? new ServletCoreResponse(coreRequest, httpServletResponse, included)
557558
: servletChannel.getResponse();
559+
// Ignore last writes done via the Core API as the ServletChannel will perform the last write upon completion.
560+
Response coreResponse = new Response.Wrapper(coreRequest, r)
561+
{
562+
@Override
563+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
564+
{
565+
super.write(false, byteBuffer, callback);
566+
}
567+
};
558568

559569
// If the core response is already committed then do nothing more
560570
if (coreResponse.isCommitted())

‎jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResourceServletTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.Timer;
3838
import java.util.TimerTask;
3939
import java.util.concurrent.atomic.AtomicBoolean;
40+
import java.util.concurrent.atomic.AtomicInteger;
4041
import java.util.concurrent.atomic.AtomicReference;
4142
import java.util.function.Consumer;
4243
import java.util.regex.Matcher;
@@ -71,17 +72,21 @@
7172
import org.eclipse.jetty.io.content.ByteBufferContentSource;
7273
import org.eclipse.jetty.logging.StacklessLogging;
7374
import org.eclipse.jetty.server.AllowedResourceAliasChecker;
75+
import org.eclipse.jetty.server.Handler;
7476
import org.eclipse.jetty.server.HttpConfiguration;
7577
import org.eclipse.jetty.server.HttpConnectionFactory;
7678
import org.eclipse.jetty.server.LocalConnector;
79+
import org.eclipse.jetty.server.Request;
7780
import org.eclipse.jetty.server.ResourceService;
81+
import org.eclipse.jetty.server.Response;
7882
import org.eclipse.jetty.server.Server;
7983
import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker;
8084
import org.eclipse.jetty.toolchain.test.FS;
8185
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
8286
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
8387
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
8488
import org.eclipse.jetty.util.BufferUtil;
89+
import org.eclipse.jetty.util.Callback;
8590
import org.eclipse.jetty.util.IO;
8691
import org.eclipse.jetty.util.StringUtil;
8792
import org.eclipse.jetty.util.resource.Resource;
@@ -3757,6 +3762,48 @@ public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exceptio
37573762
assertThat(filterCalled.get(), is(true));
37583763
}
37593764

3765+
@Test
3766+
public void testSingleLastWrite() throws Exception
3767+
{
3768+
server.stop();
3769+
var rootHandler = new Handler.Wrapper(context)
3770+
{
3771+
final AtomicInteger lastWriteCounter = new AtomicInteger();
3772+
@Override
3773+
public boolean handle(Request request, Response response, Callback callback) throws Exception
3774+
{
3775+
response = new Response.Wrapper(request, response)
3776+
{
3777+
@Override
3778+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
3779+
{
3780+
if (last)
3781+
lastWriteCounter.incrementAndGet();
3782+
super.write(last, byteBuffer, callback);
3783+
}
3784+
};
3785+
return super.handle(request, response, callback);
3786+
}
3787+
};
3788+
server.setHandler(rootHandler);
3789+
server.start();
3790+
3791+
context.addServlet(DefaultServlet.class, "/");
3792+
3793+
Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8);
3794+
3795+
String rawResponse = connector.getResponse("""
3796+
GET /context/file.txt HTTP/1.1\r
3797+
Host: local\r
3798+
Connection: close\r
3799+
\r
3800+
""");
3801+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
3802+
assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
3803+
assertThat(response.toString(), response.getContent(), is("How now brown cow"));
3804+
assertThat(rootHandler.lastWriteCounter.get(), is(1));
3805+
}
3806+
37603807
@Test
37613808
public void testServeResourceAsyncNoTimeout() throws Exception
37623809
{

‎jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AsyncIOServletTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.io.UncheckedIOException;
2020
import java.nio.ByteBuffer;
2121
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
2224
import java.util.Deque;
2325
import java.util.Queue;
2426
import java.util.concurrent.CompletableFuture;
@@ -54,6 +56,7 @@
5456
import org.eclipse.jetty.client.StringRequestContent;
5557
import org.eclipse.jetty.client.transport.internal.HttpConnectionOverHTTP;
5658
import org.eclipse.jetty.ee10.servlet.HttpOutput;
59+
import org.eclipse.jetty.ee10.servlet.ResourceServlet;
5760
import org.eclipse.jetty.http.HttpHeader;
5861
import org.eclipse.jetty.http.HttpHeaderValue;
5962
import org.eclipse.jetty.http.HttpMethod;
@@ -64,9 +67,11 @@
6467
import org.eclipse.jetty.io.Connection;
6568
import org.eclipse.jetty.io.EofException;
6669
import org.eclipse.jetty.logging.StacklessLogging;
70+
import org.eclipse.jetty.server.Handler;
6771
import org.eclipse.jetty.server.Request;
6872
import org.eclipse.jetty.server.handler.ContextHandler;
6973
import org.eclipse.jetty.server.internal.HttpChannelState;
74+
import org.eclipse.jetty.toolchain.test.FS;
7075
import org.eclipse.jetty.util.BufferUtil;
7176
import org.eclipse.jetty.util.Callback;
7277
import org.eclipse.jetty.util.FuturePromise;
@@ -77,6 +82,7 @@
7782
import org.junit.jupiter.params.ParameterizedTest;
7883
import org.junit.jupiter.params.provider.MethodSource;
7984

85+
import static java.nio.charset.StandardCharsets.UTF_8;
8086
import static org.awaitility.Awaitility.await;
8187
import static org.hamcrest.MatcherAssert.assertThat;
8288
import static org.hamcrest.Matchers.containsString;
@@ -1533,6 +1539,49 @@ public void onError(Throwable x)
15331539
assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus());
15341540
}
15351541

1542+
@ParameterizedTest
1543+
@MethodSource("transportsNoFCGI")
1544+
public void testResourceServletLastWrite(TransportType transportType) throws Exception
1545+
{
1546+
prepareServer(transportType, new ResourceServlet());
1547+
Path docRoot = workDir.getPathFile("docroot");
1548+
FS.ensureDirExists(docRoot);
1549+
Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8);
1550+
servletContextHandler.setBaseResourceAsPath(docRoot);
1551+
1552+
AtomicInteger lastWriteCounter = new AtomicInteger();
1553+
server.setHandler(new Handler.Wrapper(servletContextHandler)
1554+
{
1555+
@Override
1556+
public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception
1557+
{
1558+
response = new org.eclipse.jetty.server.Response.Wrapper(request, response)
1559+
{
1560+
@Override
1561+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
1562+
{
1563+
if (last)
1564+
lastWriteCounter.incrementAndGet();
1565+
super.write(last, byteBuffer, callback);
1566+
}
1567+
};
1568+
return super.handle(request, response, callback);
1569+
}
1570+
});
1571+
server.start();
1572+
startClient(transportType);
1573+
1574+
var request = client.newRequest(newURI(transportType))
1575+
.path("/file.txt")
1576+
.method(HttpMethod.GET)
1577+
.timeout(15, TimeUnit.SECONDS);
1578+
CompletableFuture<ContentResponse> completable = new CompletableResponseListener(request).send();
1579+
ContentResponse response = completable.get(5, TimeUnit.SECONDS);
1580+
assertEquals(HttpStatus.OK_200, response.getStatus());
1581+
assertEquals("How now brown cow", response.getContentAsString());
1582+
assertEquals(1, lastWriteCounter.get());
1583+
}
1584+
15361585
private static class Listener implements ReadListener, WriteListener
15371586
{
15381587
private final Executor executor = Executors.newFixedThreadPool(32);

‎jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ResourceServlet.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import java.io.FileNotFoundException;
1717
import java.io.IOException;
18+
import java.nio.ByteBuffer;
1819
import java.nio.file.InvalidPathException;
1920
import java.time.Duration;
2021
import java.util.ArrayList;
@@ -564,9 +565,18 @@ protected void doGet(HttpServletRequest httpServletRequest, HttpServletResponse
564565
// otherwise we can use the core response directly.
565566
boolean writingOrStreaming = servletContextResponse.isWritingOrStreaming();
566567
boolean useServletResponse = !(httpServletResponse instanceof ServletApiResponse) || writingOrStreaming;
567-
Response coreResponse = useServletResponse
568+
Response r = useServletResponse
568569
? new ServletCoreResponse(coreRequest, httpServletResponse, included)
569570
: servletChannel.getResponse();
571+
// Ignore last writes done via the Core API as the ServletChannel will perform the last write upon completion.
572+
Response coreResponse = new Response.Wrapper(coreRequest, r)
573+
{
574+
@Override
575+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
576+
{
577+
super.write(false, byteBuffer, callback);
578+
}
579+
};
570580

571581
// If the core response is already committed then do nothing more
572582
if (coreResponse.isCommitted())

‎jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/ResourceServletTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.Timer;
3838
import java.util.TimerTask;
3939
import java.util.concurrent.atomic.AtomicBoolean;
40+
import java.util.concurrent.atomic.AtomicInteger;
4041
import java.util.concurrent.atomic.AtomicReference;
4142
import java.util.function.Consumer;
4243
import java.util.regex.Matcher;
@@ -71,17 +72,21 @@
7172
import org.eclipse.jetty.io.content.ByteBufferContentSource;
7273
import org.eclipse.jetty.logging.StacklessLogging;
7374
import org.eclipse.jetty.server.AllowedResourceAliasChecker;
75+
import org.eclipse.jetty.server.Handler;
7476
import org.eclipse.jetty.server.HttpConfiguration;
7577
import org.eclipse.jetty.server.HttpConnectionFactory;
7678
import org.eclipse.jetty.server.LocalConnector;
79+
import org.eclipse.jetty.server.Request;
7780
import org.eclipse.jetty.server.ResourceService;
81+
import org.eclipse.jetty.server.Response;
7882
import org.eclipse.jetty.server.Server;
7983
import org.eclipse.jetty.server.SymlinkAllowedResourceAliasChecker;
8084
import org.eclipse.jetty.toolchain.test.FS;
8185
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
8286
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
8387
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
8488
import org.eclipse.jetty.util.BufferUtil;
89+
import org.eclipse.jetty.util.Callback;
8590
import org.eclipse.jetty.util.IO;
8691
import org.eclipse.jetty.util.StringUtil;
8792
import org.eclipse.jetty.util.URIUtil;
@@ -3784,6 +3789,48 @@ public void testServeResourceAsyncWhileStartAsyncAlreadyCalled() throws Exceptio
37843789
assertThat(filterCalled.get(), is(true));
37853790
}
37863791

3792+
@Test
3793+
public void testSingleLastWrite() throws Exception
3794+
{
3795+
server.stop();
3796+
var rootHandler = new Handler.Wrapper(context)
3797+
{
3798+
final AtomicInteger lastWriteCounter = new AtomicInteger();
3799+
@Override
3800+
public boolean handle(Request request, Response response, Callback callback) throws Exception
3801+
{
3802+
response = new Response.Wrapper(request, response)
3803+
{
3804+
@Override
3805+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
3806+
{
3807+
if (last)
3808+
lastWriteCounter.incrementAndGet();
3809+
super.write(last, byteBuffer, callback);
3810+
}
3811+
};
3812+
return super.handle(request, response, callback);
3813+
}
3814+
};
3815+
server.setHandler(rootHandler);
3816+
server.start();
3817+
3818+
context.addServlet(DefaultServlet.class, "/");
3819+
3820+
Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8);
3821+
3822+
String rawResponse = connector.getResponse("""
3823+
GET /context/file.txt HTTP/1.1\r
3824+
Host: local\r
3825+
Connection: close\r
3826+
\r
3827+
""");
3828+
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
3829+
assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200));
3830+
assertThat(response.toString(), response.getContent(), is("How now brown cow"));
3831+
assertThat(rootHandler.lastWriteCounter.get(), is(1));
3832+
}
3833+
37873834
@Test
37883835
public void testServeResourceAsyncNoTimeout() throws Exception
37893836
{

‎jetty-ee11/jetty-ee11-tests/jetty-ee11-test-client-transports/src/test/java/org/eclipse/jetty/ee11/test/client/transport/AsyncIOServletTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.io.UncheckedIOException;
2020
import java.nio.ByteBuffer;
2121
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
2224
import java.util.Deque;
2325
import java.util.Queue;
2426
import java.util.concurrent.CompletableFuture;
@@ -54,6 +56,7 @@
5456
import org.eclipse.jetty.client.StringRequestContent;
5557
import org.eclipse.jetty.client.transport.internal.HttpConnectionOverHTTP;
5658
import org.eclipse.jetty.ee11.servlet.HttpOutput;
59+
import org.eclipse.jetty.ee11.servlet.ResourceServlet;
5760
import org.eclipse.jetty.http.HttpHeader;
5861
import org.eclipse.jetty.http.HttpHeaderValue;
5962
import org.eclipse.jetty.http.HttpMethod;
@@ -64,9 +67,11 @@
6467
import org.eclipse.jetty.io.Connection;
6568
import org.eclipse.jetty.io.EofException;
6669
import org.eclipse.jetty.logging.StacklessLogging;
70+
import org.eclipse.jetty.server.Handler;
6771
import org.eclipse.jetty.server.Request;
6872
import org.eclipse.jetty.server.handler.ContextHandler;
6973
import org.eclipse.jetty.server.internal.HttpChannelState;
74+
import org.eclipse.jetty.toolchain.test.FS;
7075
import org.eclipse.jetty.util.BufferUtil;
7176
import org.eclipse.jetty.util.Callback;
7277
import org.eclipse.jetty.util.FuturePromise;
@@ -77,6 +82,7 @@
7782
import org.junit.jupiter.params.ParameterizedTest;
7883
import org.junit.jupiter.params.provider.MethodSource;
7984

85+
import static java.nio.charset.StandardCharsets.UTF_8;
8086
import static org.awaitility.Awaitility.await;
8187
import static org.hamcrest.MatcherAssert.assertThat;
8288
import static org.hamcrest.Matchers.containsString;
@@ -1546,6 +1552,49 @@ public void onError(Throwable x)
15461552
assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus());
15471553
}
15481554

1555+
@ParameterizedTest
1556+
@MethodSource("transportsNoFCGI")
1557+
public void testResourceServletLastWrite(TransportType transportType) throws Exception
1558+
{
1559+
prepareServer(transportType, new ResourceServlet());
1560+
Path docRoot = workDir.getPathFile("docroot");
1561+
FS.ensureDirExists(docRoot);
1562+
Files.writeString(docRoot.resolve("file.txt"), "How now brown cow", UTF_8);
1563+
servletContextHandler.setBaseResourceAsPath(docRoot);
1564+
1565+
AtomicInteger lastWriteCounter = new AtomicInteger();
1566+
server.setHandler(new Handler.Wrapper(servletContextHandler)
1567+
{
1568+
@Override
1569+
public boolean handle(Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception
1570+
{
1571+
response = new org.eclipse.jetty.server.Response.Wrapper(request, response)
1572+
{
1573+
@Override
1574+
public void write(boolean last, ByteBuffer byteBuffer, Callback callback)
1575+
{
1576+
if (last)
1577+
lastWriteCounter.incrementAndGet();
1578+
super.write(last, byteBuffer, callback);
1579+
}
1580+
};
1581+
return super.handle(request, response, callback);
1582+
}
1583+
});
1584+
server.start();
1585+
startClient(transportType);
1586+
1587+
var request = client.newRequest(newURI(transportType))
1588+
.path("/file.txt")
1589+
.method(HttpMethod.GET)
1590+
.timeout(15, TimeUnit.SECONDS);
1591+
CompletableFuture<ContentResponse> completable = new CompletableResponseListener(request).send();
1592+
ContentResponse response = completable.get(5, TimeUnit.SECONDS);
1593+
assertEquals(HttpStatus.OK_200, response.getStatus());
1594+
assertEquals("How now brown cow", response.getContentAsString());
1595+
assertEquals(1, lastWriteCounter.get());
1596+
}
1597+
15491598
private static class Listener implements ReadListener, WriteListener
15501599
{
15511600
private final Executor executor = Executors.newFixedThreadPool(32);

0 commit comments

Comments
 (0)