Issue #4382 - Support HTTP/1 upgrade to HTTP/2 in HttpClient.

Updated after first review.
Added more tests, fixed the upgrade over TLS case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2020-03-17 18:49:57 +01:00
parent 466517a0e8
commit 2bc8f361a6
7 changed files with 165 additions and 5 deletions

View File

@ -89,6 +89,8 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C
public boolean accept(Connection connection)
{
boolean accepted = super.accept(connection);
if (LOG.isDebugEnabled())
LOG.debug("Accepted {} {}", accepted, connection);
if (accepted)
{
synchronized (this)

View File

@ -35,6 +35,8 @@ import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
/**
* <p>A HttpUpgrader that upgrades to a given protocol.</p>
@ -45,6 +47,8 @@ import org.eclipse.jetty.util.Promise;
*/
public class ProtocolHttpUpgrader implements HttpUpgrader
{
private static final Logger LOG = Log.getLogger(ProtocolHttpUpgrader.class);
private final HttpDestination destination;
private final String protocol;
@ -79,6 +83,9 @@ public class ProtocolHttpUpgrader implements HttpUpgrader
context.put(HttpResponse.class.getName(), response);
context.put(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY, Promise.from(y -> callback.succeeded(), callback::failed));
if (LOG.isDebugEnabled())
LOG.debug("Upgrading {} on {}", response.getRequest(), endPoint);
dynamicTransport.upgrade(endPoint, context);
}
else

View File

@ -557,6 +557,16 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
}
}
/**
* <p>Creates a new stream allocating a stream id if the given HEADERS frame does not have one.</p>
* <p>The new HEADERS frame with the newly allocated stream id is returned as the first element
* of the array parameter.</p>
*
* @param frameIn the HEADERS frame that triggered the stream creation
* @param frameOut an array of size 1 to return the HEADERS frame with the newly
* allocated stream id, or null if not interested in the modified headers frame
* @return a new stream
*/
public IStream newStream(HeadersFrame frameIn, HeadersFrame[] frameOut)
{
HeadersFrame frame = frameIn;

View File

@ -30,6 +30,8 @@ import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.HTTP2ClientConnectionFactory;
import org.eclipse.jetty.io.ClientConnectionFactory;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
@ -80,9 +82,12 @@ public class ClientConnectionFactoryOverHTTP2 extends ContainerLifeCycle impleme
@Override
public void succeeded(HttpConnectionOverHTTP2 connection)
{
// This code is run when the client receives the server preface reply.
// Upgrade the connection to setup HTTP/2 frame listeners that will
// handle the HTTP/2 response to the upgrade request.
promise.succeeded(connection);
destination.accept(connection);
connection.upgrade(context);
destination.accept(connection);
}
@Override
@ -98,7 +103,13 @@ public class ClientConnectionFactoryOverHTTP2 extends ContainerLifeCycle impleme
{
try
{
endPoint.upgrade(factory.newConnection(endPoint, context));
// Avoid double TLS wrapping. We want to keep the existing
// SslConnection that has already performed the TLS handshake,
// and just upgrade the nested connection.
if (factory instanceof SslClientConnectionFactory && endPoint instanceof SslConnection.DecryptedEndPoint)
factory = ((SslClientConnectionFactory)factory).getClientConnectionFactory();
var newConnection = factory.newConnection(endPoint, context);
endPoint.upgrade(newConnection);
}
catch (IOException x)
{

View File

@ -106,6 +106,7 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S
// (with upgrade) for a resource, and the response is HTTP/2.
// Create the implicit stream#1 so that it can receive the HTTP/2 response.
MetaData.Request metaData = new MetaData.Request(request.getMethod(), new HttpURI(request.getURI()), HttpVersion.HTTP_2, request.getHeaders());
// We do not support upgrade requests with content, so endStream=true.
HeadersFrame frame = new HeadersFrame(metaData, null, true);
IStream stream = ((HTTP2Session)session).newStream(frame, null);
stream.updateClose(frame.isEndStream(), CloseState.Event.AFTER_SEND);
@ -119,6 +120,9 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S
http2Channel.setStream(stream);
newExchange.requestComplete(null);
newExchange.terminateRequest();
if (LOG.isDebugEnabled())
LOG.debug("Upgrade completed for {}", this);
}
@Override

View File

@ -56,6 +56,11 @@ public class SslClientConnectionFactory implements ClientConnectionFactory
this.connectionFactory = connectionFactory;
}
public ClientConnectionFactory getClientConnectionFactory()
{
return connectionFactory;
}
public void setDirectBuffersForEncryption(boolean useDirectBuffers)
{
this._directBuffersForEncryption = useDirectBuffers;

View File

@ -20,6 +20,7 @@ package org.eclipse.jetty.http.client;
import java.io.IOException;
import java.util.List;
import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ThreadLocalRandom;
@ -32,13 +33,18 @@ import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
import org.eclipse.jetty.client.AbstractConnectionPool;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpDestination;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.HttpRequest;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Destination;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.dynamic.HttpClientTransportDynamic;
import org.eclipse.jetty.client.http.HttpClientConnectionFactory;
import org.eclipse.jetty.client.util.BufferingResponseListener;
import org.eclipse.jetty.client.util.BytesContentProvider;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
@ -57,12 +63,14 @@ import org.eclipse.jetty.server.ProxyConnectionFactory;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -559,11 +567,124 @@ public class HttpClientTransportDynamicTest
assertEquals(content, response.getContentAsString());
// We still have 2 destinations.
assertEquals(2, client.getDestinations().size());
}
// TODO: make also a test over TLS!
@Test
public void testHTTP11UpgradeToH2CWithForwardProxy() throws Exception
{
String content = "upgrade";
startServer(this::h1H2C, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
response.setContentType("text/plain; charset=UTF-8");
response.getOutputStream().print(content);
}
});
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
ClientConnectionFactory.Info h2c = new ClientConnectionFactoryOverHTTP2.H2C(http2Client);
startClient(clientConnector, HttpClientConnectionFactory.HTTP11, h2c);
// TODO: add a test with a forward proxy!
int proxyPort = connector.getLocalPort();
// The proxy speaks both http/1.1 and h2c.
Origin.Protocol proxyProtocol = new Origin.Protocol(List.of("http/1.1", "h2c"), false);
client.getProxyConfiguration().getProxies().add(new HttpProxy(new Origin.Address("localhost", proxyPort), false, proxyProtocol));
// Make an upgrade request from HTTP/1.1 to H2C.
int serverPort = proxyPort + 1; // Any port will do.
ContentResponse response = client.newRequest("localhost", serverPort)
.header(HttpHeader.UPGRADE, "h2c")
.header(HttpHeader.HTTP2_SETTINGS, "")
.header(HttpHeader.CONNECTION, "Upgrade, HTTP2-Settings")
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
assertEquals(content, response.getContentAsString());
// Verify that we upgraded.
assertEquals(2, client.getDestinations().size());
}
@Test
public void testHTTP11UpgradeToH2COverTLS() throws Exception
{
String content = "upgrade";
startServer(this::sslH1H2C, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
response.setContentType("text/plain; charset=UTF-8");
response.getOutputStream().print(content);
}
});
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
ClientConnectionFactory.Info h2c = new ClientConnectionFactoryOverHTTP2.H2C(http2Client);
startClient(clientConnector, HttpClientConnectionFactory.HTTP11, h2c);
// Make an upgrade request from HTTP/1.1 to H2C.
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
.scheme(HttpScheme.HTTPS.asString())
.header(HttpHeader.UPGRADE, "h2c")
.header(HttpHeader.HTTP2_SETTINGS, "")
.header(HttpHeader.CONNECTION, "Upgrade, HTTP2-Settings")
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
assertEquals(content, response.getContentAsString());
// Verify that we upgraded.
assertEquals(2, client.getDestinations().size());
}
@Test
public void testHTTP11UpgradeToH2CWithRequestContentDoesNotUpgrade() throws Exception
{
startServer(this::h1H2C, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
IO.copy(request.getInputStream(), response.getOutputStream());
}
});
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
ClientConnectionFactory.Info h2c = new ClientConnectionFactoryOverHTTP2.H2C(http2Client);
startClient(clientConnector, HttpClientConnectionFactory.HTTP11, h2c);
// Make a POST upgrade request from HTTP/1.1 to H2C.
// We don't support upgrades with request content because
// the application would need to read the request content in
// HTTP/1.1 format but write the response in HTTP/2 format.
byte[] bytes = new byte[1024 * 1024];
new Random().nextBytes(bytes);
CountDownLatch latch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort())
.method(HttpMethod.POST)
.header(HttpHeader.UPGRADE, "h2c")
.header(HttpHeader.HTTP2_SETTINGS, "")
.header(HttpHeader.CONNECTION, "Upgrade, HTTP2-Settings")
.content(new BytesContentProvider(bytes))
.timeout(5, TimeUnit.SECONDS)
.send(new BufferingResponseListener(bytes.length)
{
@Override
public void onComplete(Result result)
{
assertTrue(result.isSucceeded());
assertArrayEquals(bytes, getContent());
latch.countDown();
}
});
assertTrue(latch.await(15, TimeUnit.SECONDS));
// Check that we did not upgrade.
assertEquals(1, client.getDestinations().size());
}
@Test
@ -591,7 +712,7 @@ public class HttpClientTransportDynamicTest
startServer(this::h1H2C, new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
jettyRequest.getHttpChannel().getEndPoint().getConnection().close();
}