Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

HttpOtput: fix async write for heap ByteBuffer (#811) 

* Remove unused imports

Signed-off-by: arhimondr <andriyrosa@gmail.com>

* Improve HttpOtputTest

HttpOutputTest gives false positive results for couple of tests
(testAsyncWriteBufferLargeHEAD, testAsyncWriteBufferLarge). This happens
because only the tail of the response is being checked, and that fact that
the beginning of the response in not being sent is ignored.

For regular encoding responses it is easy to check that the entire content is
received by checking it with `response.endWith(expectedContent)`. However
for chunked responses extra parsing is required to check the content in such way.
For the sake of code simplicity only start and end of the response is being checked
for the chunked encoded responses.

Signed-off-by: arhimondr <andriyrosa@gmail.com>

* Add testAsyncWriteBufferLargeDirect test to HttpOutputTest

Just to verify that everything is right when sending native buffers
asynchronously instead of heap ones.

Signed-off-by: arhimondr <andriyrosa@gmail.com>

* HttpOtput: fix async write for heap ByteBuffer

Fix bug when asynchronous writes of Heap ByteBuffer were ignored.

ByteBuffer position was moved to the end in the constructor,
then in the `process` method `if (_buffer.hasRemaining())` condition
was always evaluated to `false` and the actual write was not performed.

Add assertion in `HttpOutputTest` to verify that after the asynchronous
write position of the buffer is always at the end.

Signed-off-by: arhimondr <andriyrosa@gmail.com>
This commit is contained in:
Andriy Rosa 2016-08-05 00:51:10 +02:00 committed by Greg Wilkins
parent 12804a16f5
commit 1d968ef53a
2 changed files with 54 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
View File

@ -1075,7 +1075,6 @@ public class HttpOutput extends ServletOutputStream implements Runnable
else
{
_slice=_buffer.duplicate();
_buffer.position(_buffer.limit());
}
}

79
jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java
View File

@ -19,6 +19,7 @@
package org.eclipse.jetty.server;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
@ -28,8 +29,6 @@ import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.channels.ReadableByteChannel;
import java.util.Arrays;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.AsyncContext;
@ -39,7 +38,6 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.HttpOutput.Interceptor;
import org.eclipse.jetty.server.HttpOutputTest.ChainedInterceptor;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.server.handler.HotSwapHandler;
import org.eclipse.jetty.util.BufferUtil;
@ -142,6 +140,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -165,6 +164,7 @@ public class HttpOutputTest
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Transfer-Encoding: chunked"));
assertThat(response, containsString("1\tThis is a big file"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response,containsString("\r\n0\r\n"));
}
@ -187,7 +187,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -198,7 +198,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -209,7 +209,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@ -257,6 +257,8 @@ public class HttpOutputTest
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Transfer-Encoding: chunked"));
assertThat(response, containsString("1\tThis is a big file"));
assertThat(response, containsString("400\tThis is a big file"));
assertThat(response,containsString("\r\n0\r\n"));
}
@ -271,7 +273,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -285,7 +287,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -299,7 +301,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -313,7 +315,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -327,7 +329,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -341,7 +343,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -355,7 +357,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -369,7 +371,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("Content-Length"));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@ -400,7 +402,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -414,7 +416,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -428,7 +430,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@ -444,7 +446,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -459,7 +461,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -474,7 +476,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -489,7 +491,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@ -523,7 +525,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -538,7 +540,7 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -553,7 +555,23 @@ public class HttpOutputTest
String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tThis is a big file"));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
public void testAsyncWriteBufferLargeDirect()
throws Exception
{
final Resource big = Resource.newClassPathResource("simple/big.txt");
_handler._writeLengthIfKnown = false;
_handler._content = BufferUtil.toBuffer(big, true);
_handler._byteBuffer = BufferUtil.allocateDirect(8192);
_handler._async = true;
String response = _connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n");
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, Matchers.not(containsString("Content-Length")));
assertThat(response, endsWith(toUTF8String(big)));
}
@Test
@ -570,7 +588,8 @@ public class HttpOutputTest
assertThat(_handler._owp.get()-start,Matchers.greaterThan(0));
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,Matchers.not(containsString("400\tThis is a big file")));
assertThat(response, Matchers.not(containsString("1\tThis is a big file")));
assertThat(response, Matchers.not(containsString("400\tThis is a big file")));
}
@Test
@ -649,6 +668,12 @@ public class HttpOutputTest
assertThat(response,Matchers.not(containsString("Content-Length")));
assertThat(response,containsString("400\tTHIS IS A BIGGER FILE"));
}
private static String toUTF8String(Resource resource)
throws IOException
{
return BufferUtil.toUTF8String(BufferUtil.toBuffer(resource, false));
}
interface ChainedInterceptor extends HttpOutput.Interceptor
{
@ -767,6 +792,8 @@ public class HttpOutputTest
final AsyncContext async = request.startAsync();
out.setWriteListener(new WriteListener()
{
private boolean isFirstWrite = true;
@Override
public void onWritePossible() throws IOException
{
@ -774,6 +801,7 @@ public class HttpOutputTest
while (out.isReady())
{
Assert.assertTrue(isFirstWrite || !_byteBuffer.hasRemaining());
Assert.assertTrue(out.isReady());
if(BufferUtil.isEmpty(_content))
{
@ -785,6 +813,7 @@ public class HttpOutputTest
BufferUtil.put(_content,_byteBuffer);
BufferUtil.flipToFlush(_byteBuffer,0);
out.write(_byteBuffer);
isFirstWrite = false;
}
}