Skip to content

Fixes #11370 - IllegalStateException when last write fails. #11439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Fixes #11370 - IllegalStateException when last write fails. #11439

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae2a6d6
Fixes #11370 - IllegalStateException when last write fails.
sbordet Feb 22, 2024
9458b0b
Fixes #11370 - IllegalStateException when last write fails.
sbordet Feb 23, 2024
a653642
Fixes #11370 - IllegalStateException when last write fails.
sbordet Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/SizeLimitHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ public void write(boolean last, ByteBuffer content, Callback callback)
{
if (_responseLimit >= 0 && (_written + content.remaining()) > _responseLimit)
{
callback.failed(new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response body is too large: " +
_written + content.remaining() + ">" + _responseLimit));
String message = "Response body is too large: %d > %d".formatted(_written + content.remaining(), _responseLimit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You put spaces around the > sign. You need to fix the test to match

callback.failed(new HttpException.RuntimeException(HttpStatus.INTERNAL_SERVER_ERROR_500, message));
return;
}
_written += content.remaining();
Expand Down
13 changes: 1 addition & 12 deletions jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ public void complete(Callback callback)
}

if (content != null)
channelWrite(content, true, new CompleteWriteCompleteCB());
channelWrite(content, true, new WriteCompleteCB());
}

/**
Expand Down Expand Up @@ -1766,15 +1766,4 @@ public InvocationType getInvocationType()
return InvocationType.NON_BLOCKING;
}
}

private class CompleteWriteCompleteCB extends WriteCompleteCB
{
@Override
public void failed(Throwable x)
{
// TODO why is this needed for h2/h3?
HttpOutput.this._servletChannel.abort(x);
super.failed(x);
}
}
}
3 changes: 3 additions & 0 deletions .../jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,9 @@ protected void completed(Throwable failure)
if (_requestState != RequestState.COMPLETING)
throw new IllegalStateException(this.getStatusStringLocked());

if (failure != null)
abortResponse(failure);

if (_event == null)
{
_requestState = RequestState.COMPLETED;
Expand Down
96 changes: 84 additions & 12 deletions ...e10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.IOException;
import java.net.URI;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
Expand All @@ -40,7 +41,6 @@
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -49,6 +49,7 @@
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.lessThan;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class SizeLimitHandlerServletTest
{
Expand All @@ -57,8 +58,7 @@ public class SizeLimitHandlerServletTest
private ServerConnector _connector;
private HttpClient _client;

@BeforeEach
public void before() throws Exception
private void start(HttpServlet servlet) throws Exception
{
_server = new Server();
_connector = new ServerConnector(_server);
Expand All @@ -71,15 +71,7 @@ public void before() throws Exception
sizeLimitHandler.setHandler(gzipHandler);
contextHandler.insertHandler(sizeLimitHandler);

contextHandler.addServlet(new HttpServlet()
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
String requestContent = IO.toString(req.getInputStream());
resp.getWriter().print(requestContent);
}
}, "/");
contextHandler.addServlet(servlet, "/");

_server.setHandler(contextHandler);
_server.start();
Expand All @@ -99,6 +91,16 @@ public void after() throws Exception
@Test
public void testGzippedEcho() throws Exception
{
start(new HttpServlet()
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
String requestContent = IO.toString(req.getInputStream());
resp.getWriter().print(requestContent);
}
});

String content = "x".repeat(SIZE_LIMIT * 2);

URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
Expand All @@ -123,6 +125,16 @@ public void testGzippedEcho() throws Exception
@Test
public void testNormalEcho() throws Exception
{
start(new HttpServlet()
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
String requestContent = IO.toString(req.getInputStream());
resp.getWriter().print(requestContent);
}
});

String content = "x".repeat(SIZE_LIMIT * 2);

URI uri = URI.create("http://localhost:" + _connector.getLocalPort());
Expand All @@ -135,6 +147,65 @@ public void testNormalEcho() throws Exception
@Test
public void testGzipEchoNoAcceptEncoding() throws Exception
{
start(new HttpServlet()
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
String requestContent = IO.toString(req.getInputStream());
// The content will be buffered, and the implementation
// will try to flush it, failing because of SizeLimitHandler.
resp.getWriter().print(requestContent);
}
});

String content = "x".repeat(SIZE_LIMIT * 2);
URI uri = URI.create("http://localhost:" + _connector.getLocalPort());

StringBuilder contentReceived = new StringBuilder();
CompletableFuture<Result> resultFuture = new CompletableFuture<>();
_client.POST(uri)
.headers(httpFields -> httpFields.add(HttpHeader.CONTENT_ENCODING, "gzip"))
.body(gzipContent(content))
.onResponseContentAsync((response, chunk, demander) ->
{
contentReceived.append(BufferUtil.toString(chunk.getByteBuffer()));
demander.run();
})
.send(resultFuture::complete);

Result result = resultFuture.get(5, TimeUnit.SECONDS);
assertNotNull(result);
assertThat(result.getResponse().getStatus(), equalTo(HttpStatus.INTERNAL_SERVER_ERROR_500));
assertThat(contentReceived.toString(), containsString("Response body is too large"));
}

@Test
public void testGzipEchoNoAcceptEncodingFlush() throws Exception
{
CountDownLatch flushFailureLatch = new CountDownLatch(1);
start(new HttpServlet()
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
String requestContent = IO.toString(req.getInputStream());
// The content will be buffered.
resp.getWriter().print(requestContent);
try
{
// The flush will fail because exceeds
// the SizeLimitHandler configuration.
resp.flushBuffer();
}
catch (Throwable x)
{
flushFailureLatch.countDown();
throw x;
}
}
});

String content = "x".repeat(SIZE_LIMIT * 2);
URI uri = URI.create("http://localhost:" + _connector.getLocalPort());

Expand All @@ -150,6 +221,7 @@ public void testGzipEchoNoAcceptEncoding() throws Exception
})
.send(resultFuture::complete);

assertTrue(flushFailureLatch.await(5, TimeUnit.SECONDS));

Result result = resultFuture.get(5, TimeUnit.SECONDS);
assertNotNull(result);
Expand Down