diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ResponseTrailerTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ResponseTrailerTest.java new file mode 100644 index 00000000000..11727ee7425 --- /dev/null +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ResponseTrailerTest.java @@ -0,0 +1,127 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.client.http; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.api.Session; +import org.eclipse.jetty.http2.api.Stream; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.HeadersFrame; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.FuturePromise; +import org.eclipse.jetty.util.Promise; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class ResponseTrailerTest extends AbstractTest +{ + @Test + public void testEmptyTrailersWithoutContent() throws Exception + { + testEmptyTrailers(null); + } + + @Test + public void testEmptyTrailersWithContent() throws Exception + { + testEmptyTrailers("data"); + } + + public void testEmptyTrailers(String data) throws Exception + { + start(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + // Send empty response trailers. + Response jettyResponse = jettyRequest.getResponse(); + jettyResponse.setTrailers(HttpFields::new); + if (data != null) + response.getOutputStream().write(data.getBytes(StandardCharsets.US_ASCII)); + } + }); + + HTTP2Client http2Client = new HTTP2Client(); + http2Client.start(); + try + { + String host = "localhost"; + int port = connector.getLocalPort(); + InetSocketAddress address = new InetSocketAddress(host, port); + FuturePromise sessionPromise = new FuturePromise<>(); + http2Client.connect(address, new Session.Listener.Adapter(), sessionPromise); + Session session = sessionPromise.get(5, TimeUnit.SECONDS); + + HttpURI uri = new HttpURI("http://" + host + ":" + port + "/"); + MetaData.Request request = new MetaData.Request(HttpMethod.GET.asString(), uri, HttpVersion.HTTP_2, new HttpFields()); + HeadersFrame frame = new HeadersFrame(request, null, true); + BlockingQueue headers = new LinkedBlockingQueue<>(); + CountDownLatch latch = new CountDownLatch(1); + session.newStream(frame, new Promise.Adapter<>(), new Stream.Listener.Adapter() + { + @Override + public void onHeaders(Stream stream, HeadersFrame frame) + { + headers.offer(frame); + if (frame.isEndStream()) + latch.countDown(); + } + + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + super.onData(stream, frame, callback); + if (frame.isEndStream()) + latch.countDown(); + } + }); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + assertEquals(1, headers.size()); + frame = headers.poll(); + assertNotNull(frame); + assertTrue(frame.getMetaData().isResponse()); + } + finally + { + http2Client.stop(); + } + } +} diff --git a/jetty-http2/http2-http-client-transport/src/test/resources/jetty-logging.properties b/jetty-http2/http2-http-client-transport/src/test/resources/jetty-logging.properties index 287d28319e0..34929219c9d 100644 --- a/jetty-http2/http2-http-client-transport/src/test/resources/jetty-logging.properties +++ b/jetty-http2/http2-http-client-transport/src/test/resources/jetty-logging.properties @@ -1,4 +1,5 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog +#org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.client.LEVEL=DEBUG org.eclipse.jetty.http2.hpack.LEVEL=INFO #org.eclipse.jetty.http2.LEVEL=DEBUG diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index 7b86321ac83..b19123bb8f1 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -119,9 +119,17 @@ public class HttpTransportOverHTTP2 implements HttpTransport { if (lastContent) { - Supplier trailers = info.getTrailerSupplier(); - if (transportCallback.start(new SendTrailers(getCallback(), trailers), false)) - sendDataFrame(content, true, trailers == null, transportCallback); + HttpFields trailers = retrieveTrailers(); + if (trailers != null) + { + if (transportCallback.start(new SendTrailers(getCallback(), trailers), false)) + sendDataFrame(content, true, false, transportCallback); + } + else + { + if (transportCallback.start(getCallback(), false)) + sendDataFrame(content, true, true, transportCallback); + } } else { @@ -137,9 +145,17 @@ public class HttpTransportOverHTTP2 implements HttpTransport { if (lastContent) { - Supplier trailers = info.getTrailerSupplier(); - if (transportCallback.start(new SendTrailers(callback, trailers), true)) - sendHeadersFrame(info, trailers == null, transportCallback); + HttpFields trailers = retrieveTrailers(); + if (trailers != null) + { + if (transportCallback.start(new SendTrailers(callback, trailers), true)) + sendHeadersFrame(info, false, transportCallback); + } + else + { + if (transportCallback.start(callback, true)) + sendHeadersFrame(info, true, transportCallback); + } } else { @@ -160,16 +176,24 @@ public class HttpTransportOverHTTP2 implements HttpTransport { if (lastContent) { - Supplier trailers = metaData.getTrailerSupplier(); - SendTrailers sendTrailers = new SendTrailers(callback, trailers); - if (hasContent || trailers == null) + HttpFields trailers = retrieveTrailers(); + if (trailers != null) { - if (transportCallback.start(sendTrailers, false)) - sendDataFrame(content, true, trailers == null, transportCallback); + SendTrailers sendTrailers = new SendTrailers(callback, trailers); + if (hasContent) + { + if (transportCallback.start(sendTrailers, false)) + sendDataFrame(content, true, false, transportCallback); + } + else + { + sendTrailers.succeeded(); + } } else { - sendTrailers.succeeded(); + if (transportCallback.start(callback, false)) + sendDataFrame(content, true, true, transportCallback); } } else @@ -185,6 +209,17 @@ public class HttpTransportOverHTTP2 implements HttpTransport } } + private HttpFields retrieveTrailers() + { + Supplier supplier = metaData.getTrailerSupplier(); + if (supplier == null) + return null; + HttpFields trailers = supplier.get(); + if (trailers == null) + return null; + return trailers.size() == 0 ? null : trailers; + } + @Override public boolean isPushSupported() { @@ -420,9 +455,9 @@ public class HttpTransportOverHTTP2 implements HttpTransport private class SendTrailers extends Callback.Nested { - private final Supplier trailers; + private final HttpFields trailers; - private SendTrailers(Callback callback, Supplier trailers) + private SendTrailers(Callback callback, HttpFields trailers) { super(callback); this.trailers = trailers; @@ -431,15 +466,8 @@ public class HttpTransportOverHTTP2 implements HttpTransport @Override public void succeeded() { - if (trailers != null) - { - if (transportCallback.start(getCallback(), false)) - sendTrailersFrame(new MetaData(HttpVersion.HTTP_2, trailers.get()), transportCallback); - } - else - { - super.succeeded(); - } + if (transportCallback.start(getCallback(), false)) + sendTrailersFrame(new MetaData(HttpVersion.HTTP_2, trailers), transportCallback); } } }