andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Do not use hard-coded handler names in HTTP/2

Browse Source 
Motivation:

Our HTTP/2 implementation sometimes uses hard-coded handler names when
adding/removing a handler to/from a pipeline. It's not really a good
idea because it can easily result in name clashes. Unless there is a
good reason, we need to use the reference to the handlers

Modifications:

- Allow null as a handler name for Http2Client/ServerUpgradeCodec
  - Use null as the default upgrade handler name
- Do not use handler name strings in some test cases and examples

Result:

Fixes #3815
This commit is contained in:
Trustin Lee 2015-06-09 14:55:39 +09:00
parent e72d04509f
commit 73d79a4b3b
6 changed files with 100 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java
View File

@ -14,16 +14,6 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.base64.Base64Dialect.URL_SAFE;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_SETTINGS_HEADER;
import static io.netty.handler.codec.http2.Http2CodecUtil.SETTING_ENTRY_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeUnsignedInt;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeUnsignedShort;
import static io.netty.util.CharsetUtil.UTF_8;
import static io.netty.util.ReferenceCountUtil.release;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.base64.Base64;
@ -36,6 +26,16 @@ import java.util.Collection;
import java.util.Collections;
import java.util.List;
import static io.netty.handler.codec.base64.Base64Dialect.URL_SAFE;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_SETTINGS_HEADER;
import static io.netty.handler.codec.http2.Http2CodecUtil.SETTING_ENTRY_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeUnsignedInt;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeUnsignedShort;
import static io.netty.util.CharsetUtil.UTF_8;
import static io.netty.util.ReferenceCountUtil.release;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
/**
* Client-side cleartext upgrade codec from HTTP to HTTP/2.
*/
@ -50,21 +50,21 @@ public class Http2ClientUpgradeCodec implements HttpClientUpgradeHandler.Upgrade
* Creates the codec using a default name for the connection handler when adding to the
* pipeline.
*
* @param connectionHandler the HTTP/2 connection handler.
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ClientUpgradeCodec(Http2ConnectionHandler connectionHandler) {
this("http2ConnectionHandler", connectionHandler);
this(null, connectionHandler);
}
/**
* Creates the codec providing an upgrade to the given handler for HTTP/2.
*
* @param handlerName the name of the HTTP/2 connection handler to be used in the pipeline.
* @param connectionHandler the HTTP/2 connection handler.
* @param handlerName the name of the HTTP/2 connection handler to be used in the pipeline,
* or {@code null} to auto-generate the name
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ClientUpgradeCodec(String handlerName,
Http2ConnectionHandler connectionHandler) {
this.handlerName = checkNotNull(handlerName, "handlerName");
public Http2ClientUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler) {
this.handlerName = handlerName;
this.connectionHandler = checkNotNull(connectionHandler, "connectionHandler");
}

28
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodec.java
View File

@ -14,14 +14,6 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.base64.Base64Dialect.URL_SAFE;
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
import static io.netty.handler.codec.http2.Http2CodecUtil.FRAME_HEADER_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_SETTINGS_HEADER;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeFrameHeader;
import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.channel.ChannelHandlerContext;
@ -36,6 +28,15 @@ import java.util.Collection;
import java.util.Collections;
import java.util.List;
import static io.netty.handler.codec.base64.Base64Dialect.URL_SAFE;
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
import static io.netty.handler.codec.http2.Http2CodecUtil.FRAME_HEADER_LENGTH;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME;
import static io.netty.handler.codec.http2.Http2CodecUtil.HTTP_UPGRADE_SETTINGS_HEADER;
import static io.netty.handler.codec.http2.Http2CodecUtil.writeFrameHeader;
import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
/**
* Server-side codec for performing a cleartext upgrade from HTTP/1.x to HTTP/2.
*/
@ -52,20 +53,21 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* Creates the codec using a default name for the connection handler when adding to the
* pipeline.
*
* @param connectionHandler the HTTP/2 connection handler.
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ServerUpgradeCodec(Http2ConnectionHandler connectionHandler) {
this("http2ConnectionHandler", connectionHandler);
this(null, connectionHandler);
}
/**
* Creates the codec providing an upgrade to the given handler for HTTP/2.
*
* @param handlerName the name of the HTTP/2 connection handler to be used in the pipeline.
* @param connectionHandler the HTTP/2 connection handler.
* @param handlerName the name of the HTTP/2 connection handler to be used in the pipeline,
* or {@code null} to auto-generate the name
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ServerUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler) {
this.handlerName = checkNotNull(handlerName, "handlerName");
this.handlerName = handlerName;
this.connectionHandler = checkNotNull(connectionHandler, "connectionHandler");
frameReader = new DefaultHttp2FrameReader();
}

44
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameRoundtripTest.java
View File

@ -15,19 +15,6 @@
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http2.Http2TestUtil.as;
import static io.netty.handler.codec.http2.Http2TestUtil.randomString;
import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel;
import static io.netty.util.CharsetUtil.UTF_8;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap;
import io.netty.buffer.ByteBuf;
@ -44,6 +31,13 @@ import io.netty.channel.socket.nio.NioSocketChannel;
import io.netty.handler.codec.http2.Http2TestUtil.Http2Runnable;
import io.netty.util.NetUtil;
import io.netty.util.concurrent.Future;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import java.net.InetSocketAddress;
import java.util.ArrayList;
@ -52,13 +46,19 @@ import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import static io.netty.handler.codec.http2.Http2TestUtil.as;
import static io.netty.handler.codec.http2.Http2TestUtil.randomString;
import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel;
import static io.netty.util.CharsetUtil.UTF_8;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
/**
* Tests encoding/decoding each HTTP2 frame type.
@ -362,7 +362,7 @@ public class Http2FrameRoundtripTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
serverAdapter = new Http2TestUtil.FrameAdapter(serverListener, requestLatch);
p.addLast("reader", serverAdapter);
p.addLast(serverAdapter);
}
});
@ -372,7 +372,7 @@ public class Http2FrameRoundtripTest {
@Override
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast("reader", new Http2TestUtil.FrameAdapter(null, null));
p.addLast(new Http2TestUtil.FrameAdapter(null, null));
}
});

62
codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
View File

@ -14,17 +14,6 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http2.Http2Exception.isStreamError;
import static io.netty.handler.codec.http2.Http2CodecUtil.getEmbeddedHttp2Exception;
import static io.netty.handler.codec.http2.Http2TestUtil.as;
import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap;
import io.netty.buffer.ByteBuf;
@ -57,11 +46,6 @@ import io.netty.util.AsciiString;
import io.netty.util.CharsetUtil;
import io.netty.util.NetUtil;
import io.netty.util.concurrent.Future;
import java.net.InetSocketAddress;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@ -69,6 +53,22 @@ import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import java.net.InetSocketAddress;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import static io.netty.handler.codec.http2.Http2CodecUtil.getEmbeddedHttp2Exception;
import static io.netty.handler.codec.http2.Http2Exception.isStreamError;
import static io.netty.handler.codec.http2.Http2TestUtil.as;
import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
/**
* Testing the {@link InboundHttp2ToHttpPriorityAdapter} and base class {@link InboundHttp2ToHttpAdapter} for HTTP/2
* frames into {@link HttpObject}s
@ -124,15 +124,16 @@ public class InboundHttp2ToHttpAdapterTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
Http2Connection connection = new DefaultHttp2Connection(true);
p.addLast(
"reader",
new HttpAdapterFrameAdapter(connection,
new InboundHttp2ToHttpPriorityAdapter.Builder(connection)
.maxContentLength(maxContentLength)
.validateHttpHeaders(true)
.propagateSettings(true)
.build(),
new CountDownLatch(10)));
p.addLast(new HttpAdapterFrameAdapter(
connection,
new InboundHttp2ToHttpPriorityAdapter.Builder(connection)
.maxContentLength(maxContentLength)
.validateHttpHeaders(true)
.propagateSettings(true)
.build(),
new CountDownLatch(10)));
serverDelegator = new HttpResponseDelegator(serverListener, serverLatch);
p.addLast(serverDelegator);
serverConnectedChannel = ch;
@ -160,13 +161,14 @@ public class InboundHttp2ToHttpAdapterTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
Http2Connection connection = new DefaultHttp2Connection(false);
p.addLast(
"reader",
new HttpAdapterFrameAdapter(connection,
new InboundHttp2ToHttpPriorityAdapter.Builder(connection)
p.addLast(new HttpAdapterFrameAdapter(
connection,
new InboundHttp2ToHttpPriorityAdapter.Builder(connection)
.maxContentLength(maxContentLength)
.build(),
new CountDownLatch(10)));
new CountDownLatch(10)));
clientDelegator = new HttpResponseDelegator(clientListener, clientLatch);
p.addLast(clientDelegator);
}

21
example/src/main/java/io/netty/example/http2/helloworld/client/Http2ClientInitializer.java
View File

@ -14,8 +14,6 @@
*/
package io.netty.example.http2.helloworld.client;
import static io.netty.handler.logging.LogLevel.INFO;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelInitializer;
@ -41,6 +39,8 @@ import io.netty.handler.codec.http2.HttpToHttp2ConnectionHandler;
import io.netty.handler.codec.http2.InboundHttp2ToHttpAdapter;
import io.netty.handler.ssl.SslContext;
import static io.netty.handler.logging.LogLevel.INFO;
/**
* Configures the client pipeline to support HTTP/2 frames.
*/
@ -88,8 +88,7 @@ public class Http2ClientInitializer extends ChannelInitializer<SocketChannel> {
}
protected void configureEndOfPipeline(ChannelPipeline pipeline) {
pipeline.addLast("Http2SettingsHandler", settingsHandler);
pipeline.addLast("HttpResponseHandler", responseHandler);
pipeline.addLast(settingsHandler, responseHandler);
}
/**
@ -97,8 +96,8 @@ public class Http2ClientInitializer extends ChannelInitializer<SocketChannel> {
*/
private void configureSsl(SocketChannel ch) {
ChannelPipeline pipeline = ch.pipeline();
pipeline.addLast("SslHandler", sslCtx.newHandler(ch.alloc()));
pipeline.addLast("Http2Handler", connectionHandler);
pipeline.addLast(sslCtx.newHandler(ch.alloc()),
connectionHandler);
configureEndOfPipeline(pipeline);
}
@ -110,10 +109,10 @@ public class Http2ClientInitializer extends ChannelInitializer<SocketChannel> {
Http2ClientUpgradeCodec upgradeCodec = new Http2ClientUpgradeCodec(connectionHandler);
HttpClientUpgradeHandler upgradeHandler = new HttpClientUpgradeHandler(sourceCodec, upgradeCodec, 65536);
ch.pipeline().addLast("Http2SourceCodec", sourceCodec);
ch.pipeline().addLast("Http2UpgradeHandler", upgradeHandler);
ch.pipeline().addLast("Http2UpgradeRequestHandler", new UpgradeRequestHandler());
ch.pipeline().addLast("Logger", new UserEventLogger());
ch.pipeline().addLast(sourceCodec,
upgradeHandler,
new UpgradeRequestHandler(),
new UserEventLogger());
}
/**
@ -131,7 +130,7 @@ public class Http2ClientInitializer extends ChannelInitializer<SocketChannel> {
// Done with this handler, remove it from the pipeline.
ctx.pipeline().remove(this);
Http2ClientInitializer.this.configureEndOfPipeline(ctx.pipeline());
configureEndOfPipeline(ctx.pipeline());
}
}

9
example/src/main/java/io/netty/example/http2/tiles/HttpServer.java
View File

@ -54,11 +54,10 @@ public final class HttpServer {
.childHandler(new ChannelInitializer<SocketChannel>() {
@Override
protected void initChannel(SocketChannel ch) throws Exception {
ChannelPipeline pipeline = ch.pipeline();
pipeline.addLast("httpRequestDecoder", new HttpRequestDecoder());
pipeline.addLast("httpResponseEncoder", new HttpResponseEncoder());
pipeline.addLast("httpChunkAggregator", new HttpObjectAggregator(MAX_CONTENT_LENGTH));
pipeline.addLast("httpRequestHandler", new Http1RequestHandler());
ch.pipeline().addLast(new HttpRequestDecoder(),
new HttpResponseEncoder(),
new HttpObjectAggregator(MAX_CONTENT_LENGTH),
new Http1RequestHandler());
}
});