From 45574ce9522abd232f8ec30146d6a2b40a35cfb2 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 10 Mar 2019 23:40:18 -0400 Subject: [PATCH] Remove connection pool for HTTP connections for now. The upstream feature this relies on will be gone in Netty 5.x and there is probably more benefit in using the connection's event loop to connect to Mojang's authentication servers. --- .../client/LoginSessionHandler.java | 7 +- .../proxy/network/http/NettyHttpClient.java | 88 +++++++------------ .../http/SimpleHttpResponseCollector.java | 6 +- 3 files changed, 36 insertions(+), 65 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java index 11b8b8826..d0707588c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java @@ -1,5 +1,6 @@ package com.velocitypowered.proxy.connection.client; +import static com.google.common.net.UrlEscapers.urlFormParameterEscaper; import static com.velocitypowered.api.network.ProtocolVersion.MINECRAFT_1_13; import static com.velocitypowered.proxy.VelocityServer.GSON; import static com.velocitypowered.proxy.connection.VelocityConstants.EMPTY_BYTE_ARRAY; @@ -120,10 +121,10 @@ public class LoginSessionHandler implements MinecraftSessionHandler { String playerIp = ((InetSocketAddress) mcConnection.getRemoteAddress()).getHostString(); String url = String.format(MOJANG_HASJOINED_URL, - UrlEscapers.urlFormParameterEscaper().escape(login.getUsername()), serverId, - UrlEscapers.urlFormParameterEscaper().escape(playerIp)); + urlFormParameterEscaper().escape(login.getUsername()), serverId, + urlFormParameterEscaper().escape(playerIp)); server.getHttpClient() - .get(new URL(url)) + .get(new URL(url), mcConnection.eventLoop()) .thenAcceptAsync(profileResponse -> { if (mcConnection.isClosed()) { // The player disconnected after we authenticated them. diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java b/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java index 9314f29f5..da69a0571 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/http/NettyHttpClient.java @@ -1,14 +1,10 @@ package com.velocitypowered.proxy.network.http; -import com.google.common.base.VerifyException; import com.velocitypowered.proxy.VelocityServer; -import io.netty.bootstrap.Bootstrap; import io.netty.channel.Channel; -import io.netty.channel.pool.AbstractChannelPoolMap; -import io.netty.channel.pool.ChannelPoolHandler; -import io.netty.channel.pool.ChannelPoolMap; -import io.netty.channel.pool.FixedChannelPool; -import io.netty.channel.pool.SimpleChannelPool; +import io.netty.channel.ChannelFutureListener; +import io.netty.channel.ChannelInitializer; +import io.netty.channel.EventLoop; import io.netty.handler.codec.http.DefaultFullHttpRequest; import io.netty.handler.codec.http.HttpClientCodec; import io.netty.handler.codec.http.HttpHeaderNames; @@ -25,8 +21,8 @@ import javax.net.ssl.SSLEngine; public class NettyHttpClient { - private final ChannelPoolMap poolMap; private final String userAgent; + private final VelocityServer server; /** * Initializes the HTTP client. @@ -35,48 +31,16 @@ public class NettyHttpClient { */ public NettyHttpClient(VelocityServer server) { this.userAgent = server.getVersion().getName() + "/" + server.getVersion().getVersion(); - Bootstrap bootstrap = server.initializeGenericBootstrap(); - this.poolMap = new AbstractChannelPoolMap() { - @Override - protected SimpleChannelPool newPool(HostAndSsl key) { - return new FixedChannelPool(bootstrap.remoteAddress(key.address), new ChannelPoolHandler() { - @Override - public void channelReleased(Channel channel) throws Exception { - channel.pipeline().remove("collector"); - } - - @Override - public void channelAcquired(Channel channel) throws Exception { - // We don't do anything special when acquiring channels. The channel handler cleans up - // after each connection is used. - } - - @Override - public void channelCreated(Channel channel) throws Exception { - if (key.ssl) { - SslContext context = SslContextBuilder.forClient().protocols("TLSv1.2").build(); - // Unbelievably, Java doesn't automatically check the CN to make sure we're talking - // to the right host! Therefore, we provide the intended host name and port, along - // with asking Java very nicely if it could check the hostname in the certificate - // for us. - SSLEngine engine = context.newEngine(channel.alloc(), key.address.getHostString(), - key.address.getPort()); - engine.getSSLParameters().setEndpointIdentificationAlgorithm("HTTPS"); - channel.pipeline().addLast("ssl", new SslHandler(engine)); - } - channel.pipeline().addLast("http", new HttpClientCodec()); - } - }, 8); - } - }; + this.server = server; } /** * Attempts an HTTP GET request to the specified URL. * @param url the URL to fetch + * @param loop the event loop to use * @return a future representing the response */ - public CompletableFuture get(URL url) { + public CompletableFuture get(URL url, EventLoop loop) { String host = url.getHost(); int port = url.getPort(); boolean ssl = url.getProtocol().equals("https"); @@ -84,27 +48,37 @@ public class NettyHttpClient { port = ssl ? 443 : 80; } - HostAndSsl key = new HostAndSsl(InetSocketAddress.createUnresolved(host, port), ssl); - + InetSocketAddress address = InetSocketAddress.createUnresolved(host, port); CompletableFuture reply = new CompletableFuture<>(); - poolMap.get(key) - .acquire() - .addListener(future -> { - if (future.isSuccess()) { - Channel channel = (Channel) future.getNow(); - if (channel == null) { - throw new VerifyException("Null channel retrieved from pool!"); + server.initializeGenericBootstrap(loop) + .handler(new ChannelInitializer() { + @Override + protected void initChannel(Channel ch) throws Exception { + if (ssl) { + SslContext context = SslContextBuilder.forClient().protocols("TLSv1.2").build(); + // Unbelievably, Java doesn't automatically check the CN to make sure we're talking + // to the right host! Therefore, we provide the intended host name and port, along + // with asking Java very nicely if it could check the hostname in the certificate + // for us. + SSLEngine engine = context.newEngine(ch.alloc(), address.getHostString(), + address.getPort()); + engine.getSSLParameters().setEndpointIdentificationAlgorithm("HTTPS"); + ch.pipeline().addLast("ssl", new SslHandler(engine)); } - channel.pipeline().addLast("collector", new SimpleHttpResponseCollector(reply)); + ch.pipeline().addLast("http", new HttpClientCodec()); + ch.pipeline().addLast("collector", new SimpleHttpResponseCollector(reply)); + } + }) + .connect(address) + .addListener((ChannelFutureListener) future -> { + if (future.isSuccess()) { + Channel channel = future.channel(); DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, url.getPath() + "?" + url.getQuery()); request.headers().add(HttpHeaderNames.HOST, url.getHost()); request.headers().add(HttpHeaderNames.USER_AGENT, userAgent); - channel.writeAndFlush(request); - - // Make sure to release this connection - reply.whenComplete((resp, err) -> poolMap.get(key).release(channel)); + channel.writeAndFlush(request, channel.voidPromise()); } else { reply.completeExceptionally(future.cause()); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/network/http/SimpleHttpResponseCollector.java b/proxy/src/main/java/com/velocitypowered/proxy/network/http/SimpleHttpResponseCollector.java index 5b911cf82..533e8170f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/network/http/SimpleHttpResponseCollector.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/network/http/SimpleHttpResponseCollector.java @@ -16,7 +16,6 @@ class SimpleHttpResponseCollector extends ChannelInboundHandlerAdapter { private final StringBuilder buffer = new StringBuilder(); private final CompletableFuture reply; private int httpCode; - private boolean canKeepAlive; SimpleHttpResponseCollector(CompletableFuture reply) { this.reply = reply; @@ -29,16 +28,13 @@ class SimpleHttpResponseCollector extends ChannelInboundHandlerAdapter { HttpResponse response = (HttpResponse) msg; HttpResponseStatus status = response.status(); this.httpCode = status.code(); - this.canKeepAlive = HttpUtil.isKeepAlive(response); } if (msg instanceof HttpContent) { buffer.append(((HttpContent) msg).content().toString(StandardCharsets.UTF_8)); if (msg instanceof LastHttpContent) { - if (!canKeepAlive) { - ctx.close(); - } + ctx.close(); reply.complete(new SimpleHttpResponse(httpCode, buffer.toString())); } }