479277 - HttpClient with HTTP/2 transport does not work for "https" URLs.

Fixed by reworking how ClientConnectionFactories are handled by both
HTTP2Client and by HttpClientTransportOverHTTP2, to avoid that the
latter wraps the nested factories with SslConnection twice.
This commit is contained in:
Simone Bordet 2015-10-07 21:39:02 +02:00
parent b2f8192c7b
commit 0ca40b59c6
12 changed files with 219 additions and 55 deletions

View File

@ -137,7 +137,18 @@ public class HTTP2Client extends ContainerLifeCycle
setByteBufferPool(new MappedByteBufferPool());
if (connectionFactory == null)
setClientConnectionFactory(new HTTP2ClientConnectionFactory());
{
HTTP2ClientConnectionFactory h2 = new HTTP2ClientConnectionFactory();
ALPNClientConnectionFactory alpn = new ALPNClientConnectionFactory(getExecutor(), h2, getProtocols());
setClientConnectionFactory((endPoint, context) ->
{
ClientConnectionFactory factory = h2;
SslContextFactory sslContextFactory = (SslContextFactory)context.get(SslClientConnectionFactory.SSL_CONTEXT_FACTORY_CONTEXT_KEY);
if (sslContextFactory != null)
factory = new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), alpn);
return factory.newConnection(endPoint, context);
});
}
if (sessions == null)
{
@ -356,17 +367,7 @@ public class HTTP2Client extends ContainerLifeCycle
context.put(HTTP2ClientConnectionFactory.BYTE_BUFFER_POOL_CONTEXT_KEY, getByteBufferPool());
context.put(HTTP2ClientConnectionFactory.EXECUTOR_CONTEXT_KEY, getExecutor());
context.put(HTTP2ClientConnectionFactory.SCHEDULER_CONTEXT_KEY, getScheduler());
ClientConnectionFactory factory = getClientConnectionFactory();
SslContextFactory sslContextFactory = (SslContextFactory)context.get(SslClientConnectionFactory.SSL_CONTEXT_FACTORY_CONTEXT_KEY);
if (sslContextFactory != null)
{
ALPNClientConnectionFactory alpn = new ALPNClientConnectionFactory(getExecutor(), factory, getProtocols());
factory = new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), alpn);
}
return factory.newConnection(endpoint, context);
return getClientConnectionFactory().newConnection(endpoint, context);
}
@Override

View File

@ -15,6 +15,38 @@
</properties>
<build>
<plugins>
<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<id>copy</id>
<phase>generate-resources</phase>
<goals>
<goal>copy</goal>
</goals>
<configuration>
<artifactItems>
<artifactItem>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.version}</version>
<type>jar</type>
<overWrite>false</overWrite>
<outputDirectory>${project.build.directory}/alpn</outputDirectory>
</artifactItem>
</artifactItems>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>-Xbootclasspath/p:${project.build.directory}/alpn/alpn-boot-${alpn.version}.jar</argLine>
</configuration>
</plugin>
</plugins>
</build>
<dependencies>

View File

@ -22,19 +22,24 @@ import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.Map;
import org.eclipse.jetty.alpn.client.ALPNClientConnectionFactory;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.HttpDestination;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http2.api.Session;
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.util.Promise;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.ssl.SslContextFactory;
@ManagedObject("The HTTP/2 client transport")
public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements HttpClientTransport
@ -68,7 +73,7 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements
addBean(client);
super.doStart();
this.connectionFactory = client.getClientConnectionFactory();
this.connectionFactory = new HTTP2ClientConnectionFactory();
client.setClientConnectionFactory((endPoint, context) ->
{
HttpDestination destination = (HttpDestination)context.get(HTTP_DESTINATION_CONTEXT_KEY);
@ -128,13 +133,21 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements
}
};
client.connect(httpClient.getSslContextFactory(), address, listener, promise, context);
SslContextFactory sslContextFactory = null;
if (HttpScheme.HTTPS.is(destination.getScheme()))
sslContextFactory = httpClient.getSslContextFactory();
client.connect(sslContextFactory, address, listener, promise, context);
}
@Override
public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context) throws IOException
{
return connectionFactory.newConnection(endPoint, context);
ClientConnectionFactory factory = connectionFactory;
SslContextFactory sslContextFactory = (SslContextFactory)context.get(SslClientConnectionFactory.SSL_CONTEXT_FACTORY_CONTEXT_KEY);
if (sslContextFactory != null)
factory = new ALPNClientConnectionFactory(client.getExecutor(), factory, client.getProtocols());
return factory.newConnection(endPoint, context);
}
protected HttpConnectionOverHTTP2 newHttpConnection(HttpDestination destination, Session session)

View File

@ -21,9 +21,13 @@ package org.eclipse.jetty.http2.client.http;
import java.util.concurrent.Executor;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
public class HttpClientTransportOverHTTP2Test
@ -51,4 +55,24 @@ public class HttpClientTransportOverHTTP2Test
Assert.assertTrue(http2Client.isStopped());
}
@Ignore
@Test
public void testExternalServer() throws Exception
{
HTTP2Client http2Client = new HTTP2Client();
SslContextFactory sslContextFactory = new SslContextFactory();
HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client), sslContextFactory);
Executor executor = new QueuedThreadPool();
httpClient.setExecutor(executor);
httpClient.start();
// ContentResponse response = httpClient.GET("https://http2.akamai.com/");
ContentResponse response = httpClient.GET("https://webtide.com/");
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
httpClient.stop();
}
}

View File

@ -0,0 +1,5 @@
org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
#org.eclipse.jetty.client.LEVEL=DEBUG
org.eclipse.jetty.http2.hpack.LEVEL=INFO
#org.eclipse.jetty.http2.LEVEL=DEBUG
#org.eclipse.jetty.io.ssl.LEVEL=DEBUG

View File

@ -17,6 +17,36 @@
<build>
<plugins>
<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<id>copy</id>
<phase>generate-resources</phase>
<goals>
<goal>copy</goal>
</goals>
<configuration>
<artifactItems>
<artifactItem>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.version}</version>
<type>jar</type>
<overWrite>false</overWrite>
<outputDirectory>${project.build.directory}/alpn</outputDirectory>
</artifactItem>
</artifactItems>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>-Xbootclasspath/p:${project.build.directory}/alpn/alpn-boot-${alpn.version}.jar</argLine>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
@ -47,6 +77,12 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-alpn-server</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.http2</groupId>
<artifactId>http2-http-client-transport</artifactId>

View File

@ -18,22 +18,28 @@
package org.eclipse.jetty.http.client;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.List;
import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
import org.eclipse.jetty.http2.HTTP2Cipher;
import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory;
import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
import org.eclipse.jetty.server.ConnectionFactory;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.toolchain.test.TestTracker;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.junit.After;
import org.junit.Rule;
@ -44,15 +50,16 @@ import org.junit.runners.Parameterized;
public abstract class AbstractTest
{
@Parameterized.Parameters(name = "transport: {0}")
public static List<Object[]> parameters() throws Exception
public static Object[] parameters() throws Exception
{
return Arrays.asList(new Object[]{Transport.HTTP}, new Object[]{Transport.HTTP2});
return new Object[]{Transport.HTTP, Transport.HTTPS, Transport.H2C, Transport.H2};
}
@Rule
public final TestTracker tracker = new TestTracker();
protected final Transport transport;
protected SslContextFactory sslContextFactory;
protected Server server;
protected ServerConnector connector;
protected HttpClient client;
@ -64,11 +71,18 @@ public abstract class AbstractTest
public void start(Handler handler) throws Exception
{
sslContextFactory = new SslContextFactory();
sslContextFactory.setKeyStorePath("src/test/resources/keystore.jks");
sslContextFactory.setKeyStorePassword("storepwd");
sslContextFactory.setTrustStorePath("src/test/resources/truststore.jks");
sslContextFactory.setTrustStorePassword("storepwd");
sslContextFactory.setUseCipherSuitesOrder(true);
sslContextFactory.setCipherComparator(HTTP2Cipher.COMPARATOR);
startServer(handler);
startClient();
}
protected void startServer(Handler handler) throws Exception
private void startServer(Handler handler) throws Exception
{
QueuedThreadPool serverThreads = new QueuedThreadPool();
serverThreads.setName("server");
@ -79,26 +93,58 @@ public abstract class AbstractTest
server.start();
}
protected void startClient() throws Exception
private void startClient() throws Exception
{
QueuedThreadPool clientThreads = new QueuedThreadPool();
clientThreads.setName("client");
client = new HttpClient(provideClientTransport(transport), null);
client = new HttpClient(provideClientTransport(transport), sslContextFactory);
client.setExecutor(clientThreads);
client.start();
}
private ConnectionFactory provideServerConnectionFactory(Transport transport)
private ConnectionFactory[] provideServerConnectionFactory(Transport transport)
{
List<ConnectionFactory> result = new ArrayList<>();
switch (transport)
{
case HTTP:
return new HttpConnectionFactory(new HttpConfiguration());
case HTTP2:
return new HTTP2ServerConnectionFactory(new HttpConfiguration());
{
result.add(new HttpConnectionFactory(new HttpConfiguration()));
break;
}
case HTTPS:
{
HttpConfiguration configuration = new HttpConfiguration();
configuration.addCustomizer(new SecureRequestCustomizer());
HttpConnectionFactory http = new HttpConnectionFactory(configuration);
SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, http.getProtocol());
result.add(ssl);
result.add(http);
break;
}
case H2C:
{
result.add(new HTTP2CServerConnectionFactory(new HttpConfiguration()));
break;
}
case H2:
{
HttpConfiguration configuration = new HttpConfiguration();
configuration.addCustomizer(new SecureRequestCustomizer());
HTTP2ServerConnectionFactory h2 = new HTTP2ServerConnectionFactory(configuration);
ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory("h2");
SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, alpn.getProtocol());
result.add(ssl);
result.add(alpn);
result.add(h2);
break;
}
default:
{
throw new IllegalArgumentException();
}
}
return result.toArray(new ConnectionFactory[result.size()]);
}
private HttpClientTransport provideClientTransport(Transport transport)
@ -106,10 +152,12 @@ public abstract class AbstractTest
switch (transport)
{
case HTTP:
case HTTPS:
{
return new HttpClientTransportOverHTTP(1);
}
case HTTP2:
case H2C:
case H2:
{
HTTP2Client http2Client = new HTTP2Client();
http2Client.setSelectors(1);
@ -122,6 +170,21 @@ public abstract class AbstractTest
}
}
protected String newURI()
{
switch (transport)
{
case HTTP:
case H2C:
return "http://localhost:" + connector.getLocalPort();
case HTTPS:
case H2:
return "https://localhost:" + connector.getLocalPort();
default:
throw new IllegalArgumentException();
}
}
@After
public void stop() throws Exception
{
@ -133,6 +196,6 @@ public abstract class AbstractTest
protected enum Transport
{
HTTP, HTTP2
HTTP, HTTPS, H2C, H2
}
}

View File

@ -53,7 +53,7 @@ public class AsyncRequestContentTest extends AbstractTest
DeferredContentProvider contentProvider = new DeferredContentProvider();
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{
@ -73,7 +73,7 @@ public class AsyncRequestContentTest extends AbstractTest
DeferredContentProvider contentProvider = new DeferredContentProvider();
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{
@ -95,7 +95,7 @@ public class AsyncRequestContentTest extends AbstractTest
InputStreamContentProvider contentProvider =
new InputStreamContentProvider(new ByteArrayInputStream(new byte[0]));
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{
@ -116,7 +116,7 @@ public class AsyncRequestContentTest extends AbstractTest
InputStreamContentProvider contentProvider =
new InputStreamContentProvider(new ByteArrayInputStream(new byte[1]));
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{
@ -136,7 +136,7 @@ public class AsyncRequestContentTest extends AbstractTest
OutputStreamContentProvider contentProvider = new OutputStreamContentProvider();
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{
@ -156,7 +156,7 @@ public class AsyncRequestContentTest extends AbstractTest
OutputStreamContentProvider contentProvider = new OutputStreamContentProvider();
CountDownLatch latch = new CountDownLatch(1);
client.POST("http://localhost:" + connector.getLocalPort())
client.POST(newURI())
.content(contentProvider)
.send(result ->
{

View File

@ -27,8 +27,6 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.Assert;
@ -61,14 +59,10 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
client.start();
final CountDownLatch latch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort()).send(new Response.CompleteListener()
client.newRequest(newURI()).send(result ->
{
@Override
public void onComplete(Result result)
{
if (result.isFailed())
latch.countDown();
}
if (result.isFailed())
latch.countDown();
});
Assert.assertTrue(latch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
@ -89,16 +83,12 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
});
final CountDownLatch latch = new CountDownLatch(1);
client.newRequest("localhost", connector.getLocalPort())
client.newRequest(newURI())
.idleTimeout(idleTimeout, TimeUnit.MILLISECONDS)
.send(new Response.CompleteListener()
.send(result ->
{
@Override
public void onComplete(Result result)
{
if (result.isFailed())
latch.countDown();
}
if (result.isFailed())
latch.countDown();
});
Assert.assertTrue(latch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));

View File

@ -61,7 +61,7 @@ public class HttpClientTest extends AbstractTest
}
});
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
ContentResponse response = client.newRequest(newURI())
.timeout(5, TimeUnit.SECONDS)
.send();
@ -95,7 +95,7 @@ public class HttpClientTest extends AbstractTest
}
});
org.eclipse.jetty.client.api.Request request = client.newRequest("localhost", connector.getLocalPort());
org.eclipse.jetty.client.api.Request request = client.newRequest(newURI());
FutureResponseListener listener = new FutureResponseListener(request, length);
request.timeout(10, TimeUnit.SECONDS).send(listener);
ContentResponse response = listener.get();
@ -139,7 +139,7 @@ public class HttpClientTest extends AbstractTest
}
});
org.eclipse.jetty.client.api.Request request = client.newRequest("localhost", connector.getLocalPort());
org.eclipse.jetty.client.api.Request request = client.newRequest(newURI());
FutureResponseListener listener = new FutureResponseListener(request, 2 * length);
request.timeout(10, TimeUnit.SECONDS).send(listener);
ContentResponse response = listener.get();
@ -183,7 +183,7 @@ public class HttpClientTest extends AbstractTest
}
});
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
ContentResponse response = client.newRequest(newURI())
.method(HttpMethod.POST)
.content(new BytesContentProvider(bytes))
.timeout(15, TimeUnit.SECONDS)
@ -228,7 +228,7 @@ public class HttpClientTest extends AbstractTest
int chunkSize = 16;
byte[][] bytes = IntStream.range(0, chunks).mapToObj(x -> new byte[chunkSize]).toArray(byte[][]::new);
BytesContentProvider contentProvider = new BytesContentProvider("application/octet-stream", bytes);
ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
ContentResponse response = client.newRequest(newURI())
.method(HttpMethod.POST)
.content(contentProvider)
.timeout(15, TimeUnit.SECONDS)