diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 65a182b24fd..f8df5ea27a2 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -847,10 +847,12 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Blocking write try { + boolean complete = false; // flush any content from the aggregate if (BufferUtil.hasContent(_aggregate)) { - channelWrite(_aggregate, last && len == 0); + complete = last && len == 0; + channelWrite(_aggregate, complete); // should we fill aggregate again from the buffer? if (len > 0 && !last && len <= _commitSize && len <= maximizeAggregateSpace()) @@ -880,7 +882,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable } channelWrite(view, last); } - else if (last) + else if (last && !complete) { channelWrite(BufferUtil.EMPTY_BUFFER, true); } @@ -907,7 +909,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { checkWritable(); long written = _written + len; - last = _channel.getResponse().isAllContentWritten(_written); + last = _channel.getResponse().isAllContentWritten(written); flush = last || len > 0 || BufferUtil.hasContent(_aggregate); if (last && _state == State.OPEN) @@ -951,13 +953,17 @@ public class HttpOutput extends ServletOutputStream implements Runnable { // Blocking write // flush any content from the aggregate + boolean complete = false; if (BufferUtil.hasContent(_aggregate)) - channelWrite(_aggregate, last && len == 0); + { + complete = last && len == 0; + channelWrite(_aggregate, complete); + } // write any remaining content in the buffer directly if (len > 0) channelWrite(buffer, last); - else if (last) + else if (last && !complete) channelWrite(BufferUtil.EMPTY_BUFFER, true); onWriteComplete(last, null); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java index d9d9e5452ca..af67693b8e5 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java @@ -22,6 +22,8 @@ import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.AsyncContext; import javax.servlet.ServletException; @@ -36,6 +38,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.HotSwapHandler; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.resource.Resource; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; @@ -45,6 +48,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -355,6 +359,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length")); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -369,6 +374,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length")); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -383,6 +389,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length")); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -397,6 +404,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length")); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -414,6 +422,7 @@ public class HttpOutputTest String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length")); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -428,6 +437,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -442,6 +452,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -456,6 +467,52 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); + } + + @Test + public void testWriteBufferSmallKnown() throws Exception + { + final Resource big = Resource.newClassPathResource("simple/big.txt"); + _handler._writeLengthIfKnown = true; + _handler._content = BufferUtil.toBuffer(big, false); + _handler._byteBuffer = BufferUtil.allocate(8); + + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(response, containsString("Content-Length")); + assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); + } + + @Test + public void testWriteBufferMedKnown() throws Exception + { + final Resource big = Resource.newClassPathResource("simple/big.txt"); + _handler._writeLengthIfKnown = true; + _handler._content = BufferUtil.toBuffer(big, false); + _handler._byteBuffer = BufferUtil.allocate(4000); + + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(response, containsString("Content-Length")); + assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); + } + + @Test + public void testWriteBufferLargeKnown() throws Exception + { + final Resource big = Resource.newClassPathResource("simple/big.txt"); + _handler._writeLengthIfKnown = true; + _handler._content = BufferUtil.toBuffer(big, false); + _handler._byteBuffer = BufferUtil.allocate(8192); + + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(response, containsString("Content-Length")); + assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -471,6 +528,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -486,6 +544,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -501,6 +560,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -516,12 +576,13 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test public void testAsyncWriteHuge() throws Exception { - _handler._writeLengthIfKnown = true; + _handler._writeLengthIfKnown = false; _handler._content = BufferUtil.allocate(4 * 1024 * 1024); _handler._content.limit(_handler._content.capacity()); for (int i = _handler._content.capacity(); i-- > 0; ) @@ -533,7 +594,8 @@ public class HttpOutputTest String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); - assertThat(response, containsString("Content-Length")); + assertThat(response, Matchers.not(containsString("Content-Length"))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -549,6 +611,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -564,6 +627,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -579,6 +643,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -595,6 +660,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, Matchers.not(containsString("Content-Length"))); assertThat(response, endsWith(toUTF8String(big))); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(false)); } @Test @@ -629,6 +695,7 @@ public class HttpOutputTest assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("Content-Length: 11")); assertThat(response, containsString("simple text")); + assertThat(_handler._closedAfterWrite.get(10, TimeUnit.SECONDS), is(true)); } @Test @@ -686,6 +753,116 @@ public class HttpOutputTest assertThat(response, containsString("400\tTHIS IS A BIGGER FILE")); } + @Test + public void testEmptyArray() throws Exception + { + FuturePromise committed = new FuturePromise<>(); + AbstractHandler handler = new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setStatus(200); + try + { + response.getOutputStream().write(new byte[0]); + committed.succeeded(response.isCommitted()); + } + catch (Throwable t) + { + committed.failed(t); + } + } + }; + + _swap.setHandler(handler); + handler.start(); + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(committed.get(10, TimeUnit.SECONDS), is(false)); + } + + @Test + public void testEmptyArrayKnown() throws Exception + { + FuturePromise committed = new FuturePromise<>(); + AbstractHandler handler = new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setStatus(200); + response.setContentLength(0); + try + { + response.getOutputStream().write(new byte[0]); + committed.succeeded(response.isCommitted()); + } + catch (Throwable t) + { + committed.failed(t); + } + } + }; + + _swap.setHandler(handler); + handler.start(); + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(response, containsString("Content-Length: 0")); + assertThat(committed.get(10, TimeUnit.SECONDS), is(true)); + } + + @Test + public void testEmptyBuffer() throws Exception + { + AtomicBoolean committed = new AtomicBoolean(); + AbstractHandler handler = new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setStatus(200); + ((HttpOutput)response.getOutputStream()).write(ByteBuffer.wrap(new byte[0])); + committed.set(response.isCommitted()); + } + }; + + _swap.setHandler(handler); + handler.start(); + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(committed.get(), is(false)); + } + + @Test + public void testEmptyBufferKnown() throws Exception + { + AtomicBoolean committed = new AtomicBoolean(); + AbstractHandler handler = new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + response.setStatus(200); + response.setContentLength(0); + ((HttpOutput)response.getOutputStream()).write(ByteBuffer.wrap(new byte[0])); + committed.set(response.isCommitted()); + } + }; + + _swap.setHandler(handler); + handler.start(); + String response = _connector.getResponse("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response, containsString("HTTP/1.1 200 OK")); + assertThat(response, containsString("Content-Length: 0")); + assertThat(committed.get(), is(true)); + } + @Test public void testAggregation() throws Exception { @@ -851,7 +1028,7 @@ public class HttpOutputTest aggregated += data.length; } - // write data that will not be aggregated + // write data that will not be aggregated because it is too large data = new byte[bufferSize + 1]; Arrays.fill(data, (byte)(fill++)); expected.write(data); @@ -1025,6 +1202,7 @@ public class HttpOutputTest ReadableByteChannel _contentChannel; ByteBuffer _content; ChainedInterceptor _interceptor; + final FuturePromise _closedAfterWrite = new FuturePromise<>(); @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -1045,6 +1223,7 @@ public class HttpOutputTest { out.sendContent(_contentInputStream); _contentInputStream = null; + _closedAfterWrite.succeeded(out.isClosed()); return; } @@ -1052,6 +1231,7 @@ public class HttpOutputTest { out.sendContent(_contentChannel); _contentChannel = null; + _closedAfterWrite.succeeded(out.isClosed()); return; } @@ -1078,6 +1258,7 @@ public class HttpOutputTest len = _arrayBuffer.length; if (len == 0) { + _closedAfterWrite.succeeded(out.isClosed()); async.complete(); break; } @@ -1088,7 +1269,6 @@ public class HttpOutputTest else out.write(_arrayBuffer, 0, len); } - // assertFalse(out.isReady()); } @Override @@ -1113,7 +1293,7 @@ public class HttpOutputTest else out.write(_arrayBuffer, 0, len); } - + _closedAfterWrite.succeeded(out.isClosed()); return; } @@ -1137,6 +1317,7 @@ public class HttpOutputTest assertTrue(out.isReady()); if (BufferUtil.isEmpty(_content)) { + _closedAfterWrite.succeeded(out.isClosed()); async.complete(); break; } @@ -1167,7 +1348,7 @@ public class HttpOutputTest BufferUtil.flipToFlush(_byteBuffer, 0); out.write(_byteBuffer); } - + _closedAfterWrite.succeeded(out.isClosed()); return; } @@ -1178,6 +1359,7 @@ public class HttpOutputTest else out.sendContent(_content); _content = null; + _closedAfterWrite.succeeded(out.isClosed()); return; } }