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

Merge pull request #3865 from eclipse/jetty-9.4.x-3829-dont_send_empty_response_trailers 

Fixes #3829 - Avoid sending empty trailer frames for http/2 responses.
This commit is contained in:
Simone Bordet 2019-07-21 18:31:22 +03:00 committed by GitHub
parent 40fc4ef340 9f84c1cb8e
commit 1aaadea13d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 179 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

127
jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ResponseTrailerTest.java Normal file
View File

@ -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<Session> 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<HeadersFrame> 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();
}
}
}

1
jetty-http2/http2-http-client-transport/src/test/resources/jetty-logging.properties
View File

@ -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

74
jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
View File

@ -119,9 +119,17 @@ public class HttpTransportOverHTTP2 implements HttpTransport
{
if (lastContent)
{
Supplier<HttpFields> 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<HttpFields> 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<HttpFields> 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<HttpFields> 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<HttpFields> trailers;
private final HttpFields trailers;
private SendTrailers(Callback callback, Supplier<HttpFields> 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);
}
}
}