Skip to content
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

Jump to bottom

Fix #6562 last written bytebuffer #6563

Merged
merged 3 commits into from Aug 3, 2021
Merged

Fix #6562 last written bytebuffer #6563

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
8539e1c
Fix #6562 last written bytebuffer
gregw Jul 30, 2021
6d2c57c
Fix #6562 last written bytebuffer
gregw Jul 30, 2021
0f6d3e2
Fix #6562 last written bytebuffer
gregw Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
View file
Open in desktop
Expand Up @@ -896,10 +896,6 @@ public void write(byte[] b, int off, int len) throws IOException
}
channelWrite(view, last);
}
else if (last)
{
channelWrite(BufferUtil.EMPTY_BUFFER, true);
gregw marked this conversation as resolved.
Show resolved Hide resolved
}

onWriteComplete(last, null);
}
Expand All @@ -923,7 +919,7 @@ public void write(ByteBuffer buffer) throws IOException
{
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)
Expand Down Expand Up @@ -973,8 +969,6 @@ public void write(ByteBuffer buffer) throws IOException
// write any remaining content in the buffer directly
if (len > 0)
channelWrite(buffer, last);
else if (last)
gregw marked this conversation as resolved.
Show resolved Hide resolved
channelWrite(BufferUtil.EMPTY_BUFFER, true);

onWriteComplete(last, null);
}
Expand Down
83 changes: 77 additions & 6 deletions jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@
import java.nio.channels.ReadableByteChannel;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.AsyncContext;
import javax.servlet.ServletException;
Expand All @@ -41,6 +42,7 @@
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;
Expand All @@ -50,6 +52,7 @@
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;

Expand Down Expand Up @@ -360,6 +363,7 @@ public void testWriteByteKnown() throws Exception
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
Expand All @@ -374,6 +378,7 @@ public void testWriteSmallKnown() throws Exception
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
Expand All @@ -388,6 +393,7 @@ public void testWriteMedKnown() throws Exception
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
Expand All @@ -402,6 +408,7 @@ public void testWriteLargeKnown() throws Exception
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
Expand All @@ -419,6 +426,7 @@ public void testWriteHugeKnown() throws Exception
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
Expand All @@ -433,6 +441,7 @@ public void testWriteBufferSmall() throws Exception
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
Expand All @@ -447,6 +456,7 @@ public void testWriteBufferMed() throws Exception
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
Expand All @@ -461,6 +471,52 @@ public void testWriteBufferLarge() throws Exception
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
Expand All @@ -476,6 +532,7 @@ public void testAsyncWriteByte() throws Exception
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
Expand All @@ -491,6 +548,7 @@ public void testAsyncWriteSmall() throws Exception
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
Expand All @@ -506,6 +564,7 @@ public void testAsyncWriteMed() throws Exception
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
Expand All @@ -521,12 +580,13 @@ public void testAsyncWriteLarge() throws Exception
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; )
Expand All @@ -538,7 +598,8 @@ public void testAsyncWriteHuge() throws Exception

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
Expand All @@ -554,6 +615,7 @@ public void testAsyncWriteBufferSmall() throws Exception
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
Expand All @@ -569,6 +631,7 @@ public void testAsyncWriteBufferMed() throws Exception
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
Expand All @@ -584,6 +647,7 @@ public void testAsyncWriteBufferLarge() throws Exception
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
Expand All @@ -600,6 +664,7 @@ public void testAsyncWriteBufferLargeDirect()
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
Expand Down Expand Up @@ -634,6 +699,7 @@ public void testAsyncWriteSimpleKnown() throws Exception
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
Expand Down Expand Up @@ -868,7 +934,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
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);
Expand Down Expand Up @@ -1048,6 +1114,7 @@ static class ContentHandler extends AbstractHandler
ReadableByteChannel _contentChannel;
ByteBuffer _content;
ChainedInterceptor _interceptor;
final FuturePromise<Boolean> _closedAfterWrite = new FuturePromise<>();

@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
Expand All @@ -1068,13 +1135,15 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
{
out.sendContent(_contentInputStream);
_contentInputStream = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}

if (_contentChannel != null)
{
out.sendContent(_contentChannel);
_contentChannel = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}

Expand All @@ -1101,6 +1170,7 @@ public void onWritePossible() throws IOException
len = _arrayBuffer.length;
if (len == 0)
{
_closedAfterWrite.succeeded(out.isClosed());
async.complete();
break;
}
Expand All @@ -1111,7 +1181,6 @@ public void onWritePossible() throws IOException
else
out.write(_arrayBuffer, 0, len);
}
// assertFalse(out.isReady());
}

@Override
Expand All @@ -1136,7 +1205,7 @@ public void onError(Throwable t)
else
out.write(_arrayBuffer, 0, len);
}

_closedAfterWrite.succeeded(out.isClosed());
return;
}

Expand All @@ -1160,6 +1229,7 @@ public void onWritePossible() throws IOException
assertTrue(out.isReady());
if (BufferUtil.isEmpty(_content))
{
_closedAfterWrite.succeeded(out.isClosed());
async.complete();
break;
}
Expand Down Expand Up @@ -1190,7 +1260,7 @@ public void onError(Throwable t)
BufferUtil.flipToFlush(_byteBuffer, 0);
out.write(_byteBuffer);
}

_closedAfterWrite.succeeded(out.isClosed());
return;
}

Expand All @@ -1201,6 +1271,7 @@ public void onError(Throwable t)
else
out.sendContent(_content);
_content = null;
_closedAfterWrite.succeeded(out.isClosed());
return;
}
}
Expand Down