Fixes #2454 - Avoid sending empty DATA frame in case of HTTP/2 trailers.
Updated the logic to avoid sending an empty data frame when only sending the trailers in HttpTransportOverHTTP2.send(). Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
parent
0e88849a68
commit
50c44f2297
|
@ -20,6 +20,9 @@ package org.eclipse.jetty.http2.client;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.nio.ByteBuffer;
|
import java.nio.ByteBuffer;
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.List;
|
||||||
import java.util.concurrent.CountDownLatch;
|
import java.util.concurrent.CountDownLatch;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
|
|
||||||
|
@ -37,11 +40,14 @@ import org.eclipse.jetty.http2.api.Session;
|
||||||
import org.eclipse.jetty.http2.api.Stream;
|
import org.eclipse.jetty.http2.api.Stream;
|
||||||
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
|
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
|
||||||
import org.eclipse.jetty.http2.frames.DataFrame;
|
import org.eclipse.jetty.http2.frames.DataFrame;
|
||||||
|
import org.eclipse.jetty.http2.frames.Frame;
|
||||||
import org.eclipse.jetty.http2.frames.HeadersFrame;
|
import org.eclipse.jetty.http2.frames.HeadersFrame;
|
||||||
import org.eclipse.jetty.server.Request;
|
import org.eclipse.jetty.server.Request;
|
||||||
|
import org.eclipse.jetty.server.Response;
|
||||||
import org.eclipse.jetty.util.Callback;
|
import org.eclipse.jetty.util.Callback;
|
||||||
import org.eclipse.jetty.util.FuturePromise;
|
import org.eclipse.jetty.util.FuturePromise;
|
||||||
import org.eclipse.jetty.util.Promise;
|
import org.eclipse.jetty.util.Promise;
|
||||||
|
import org.hamcrest.Matchers;
|
||||||
import org.junit.Assert;
|
import org.junit.Assert;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
|
@ -223,4 +229,62 @@ public class TrailersTest extends AbstractTest
|
||||||
|
|
||||||
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testTrailersSentByServerShouldNotSendEmptyDataFrame() throws Exception
|
||||||
|
{
|
||||||
|
String trailerName = "X-Trailer";
|
||||||
|
String trailerValue = "Zot!";
|
||||||
|
start(new EmptyHttpServlet()
|
||||||
|
{
|
||||||
|
@Override
|
||||||
|
protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
|
||||||
|
{
|
||||||
|
Request jettyRequest = (Request)request;
|
||||||
|
Response jettyResponse = jettyRequest.getResponse();
|
||||||
|
HttpFields trailers = new HttpFields();
|
||||||
|
jettyResponse.setTrailers(() -> trailers);
|
||||||
|
|
||||||
|
jettyResponse.getOutputStream().write("hello_trailers".getBytes(StandardCharsets.UTF_8));
|
||||||
|
jettyResponse.flushBuffer();
|
||||||
|
// Force the content to be sent above, and then only send the trailers below.
|
||||||
|
trailers.put(trailerName, trailerValue);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
Session session = newClient(new Session.Listener.Adapter());
|
||||||
|
MetaData.Request request = newRequest("GET", new HttpFields());
|
||||||
|
HeadersFrame requestFrame = new HeadersFrame(request, null, true);
|
||||||
|
CountDownLatch latch = new CountDownLatch(1);
|
||||||
|
List<Frame> frames = new ArrayList<>();
|
||||||
|
session.newStream(requestFrame, new Promise.Adapter<>(), new Stream.Listener.Adapter()
|
||||||
|
{
|
||||||
|
@Override
|
||||||
|
public void onHeaders(Stream stream, HeadersFrame frame)
|
||||||
|
{
|
||||||
|
frames.add(frame);
|
||||||
|
if (frame.isEndStream())
|
||||||
|
latch.countDown();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void onData(Stream stream, DataFrame frame, Callback callback)
|
||||||
|
{
|
||||||
|
frames.add(frame);
|
||||||
|
callback.succeeded();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
|
||||||
|
Assert.assertThat(frames.toString(), frames.size(), Matchers.is(3));
|
||||||
|
|
||||||
|
HeadersFrame headers = (HeadersFrame)frames.get(0);
|
||||||
|
DataFrame data = (DataFrame)frames.get(1);
|
||||||
|
HeadersFrame trailers = (HeadersFrame)frames.get(2);
|
||||||
|
|
||||||
|
Assert.assertFalse(headers.isEndStream());
|
||||||
|
Assert.assertFalse(data.isEndStream());
|
||||||
|
Assert.assertTrue(trailers.isEndStream());
|
||||||
|
Assert.assertThat(trailers.getMetaData().getFields().get(trailerName), Matchers.equalTo(trailerValue));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -161,10 +161,18 @@ public class HttpTransportOverHTTP2 implements HttpTransport
|
||||||
if (lastContent)
|
if (lastContent)
|
||||||
{
|
{
|
||||||
Supplier<HttpFields> trailers = metaData.getTrailerSupplier();
|
Supplier<HttpFields> trailers = metaData.getTrailerSupplier();
|
||||||
if (transportCallback.start(new SendTrailers(callback, trailers), false))
|
SendTrailers sendTrailers = new SendTrailers(callback, trailers);
|
||||||
|
if (hasContent || trailers == null)
|
||||||
|
{
|
||||||
|
if (transportCallback.start(sendTrailers, false))
|
||||||
sendDataFrame(content, true, trailers == null, transportCallback);
|
sendDataFrame(content, true, trailers == null, transportCallback);
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
|
{
|
||||||
|
sendTrailers.succeeded();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else
|
||||||
{
|
{
|
||||||
if (transportCallback.start(callback, false))
|
if (transportCallback.start(callback, false))
|
||||||
sendDataFrame(content, false, false, transportCallback);
|
sendDataFrame(content, false, false, transportCallback);
|
||||||
|
|
Loading…
Reference in New Issue