Issue #3758 - Avoid sending empty trailer frames for http/2 requests.

Added one more test case and comments about handling of
`content.isConsumed()` in HTTP/2.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2019-06-12 19:15:15 +02:00
parent 8f53d14e15
commit 82f7647629
4 changed files with 54 additions and 3 deletions

View File

@ -740,7 +740,7 @@ public abstract class HttpSender implements AsyncContentProvider.Listener
} }
else else
{ {
// Was any content sent while committing ? // Was any content sent while committing?
ByteBuffer contentBuffer = content.getContent(); ByteBuffer contentBuffer = content.getContent();
if (contentBuffer != null) if (contentBuffer != null)
{ {

View File

@ -94,8 +94,8 @@ public class HttpSenderOverHTTP extends HttpSender
} }
case NEED_CHUNK_TRAILER: case NEED_CHUNK_TRAILER:
{ {
callback.succeeded(); chunk = bufferPool.acquire(httpClient.getRequestBufferSize(), false);
return; break;
} }
case FLUSH: case FLUSH:
{ {

View File

@ -133,6 +133,9 @@ public class HttpSenderOverHTTP2 extends HttpSender
{ {
if (content.isConsumed()) if (content.isConsumed())
{ {
// The superclass calls sendContent() one more time after the last content.
// This is necessary for HTTP/1.1 to generate the terminal chunk (with trailers),
// but it's not necessary for HTTP/2 so we just succeed the callback.
callback.succeeded(); callback.succeeded();
} }
else else

View File

@ -139,4 +139,52 @@ public class RequestTrailersTest extends AbstractTest
assertTrue(latch.await(5, TimeUnit.SECONDS)); assertTrue(latch.await(5, TimeUnit.SECONDS));
} }
@Test
public void testEmptyTrailersWithEmptyDeferredContent() throws Exception
{
start(new ServerSessionListener.Adapter()
{
@Override
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
return new Stream.Listener.Adapter()
{
@Override
public void onData(Stream stream, DataFrame dataFrame, Callback callback)
{
callback.succeeded();
// We should not receive an empty HEADERS frame for the
// trailers, but instead a DATA frame with endStream=true.
if (dataFrame.isEndStream())
{
MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields());
HeadersFrame responseFrame = new HeadersFrame(stream.getId(), response, null, true);
stream.headers(responseFrame, Callback.NOOP);
}
}
};
}
});
HttpRequest request = (HttpRequest)client.newRequest("localhost", connector.getLocalPort());
HttpFields trailers = new HttpFields();
request.trailers(() -> trailers);
DeferredContentProvider content = new DeferredContentProvider();
request.content(content);
CountDownLatch latch = new CountDownLatch(1);
request.send(result ->
{
assertTrue(result.isSucceeded());
assertEquals(HttpStatus.OK_200, result.getResponse().getStatus());
latch.countDown();
});
// Send deferred content after a while.
Thread.sleep(1000);
content.close();
assertTrue(latch.await(5, TimeUnit.SECONDS));
}
} }