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

Fix NPE when using Http2ServerUpgradeCodec with Http2FrameCodec and Http2MultiplexCodec

Browse Source 
Motiviation:

At the moment an NPE is thrown if someone tries to use the Http2ServerUpgradeCodec with Http2FrameCodec and Http2MultiplexCodec.

Modifications:

- Ensure the handler was added to the pipeline before calling on*Upgrade(...) methods.
- Add tests
- Fix adding of handlers after upgrade.

Result:

Fixes [#7173].
This commit is contained in:
Norman Maurer 2017-09-07 08:22:12 +02:00
parent de9e666493
commit 15611dadbb
5 changed files with 168 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
View File

@ -156,8 +156,13 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
if (connection().isServer()) {
throw connectionError(PROTOCOL_ERROR, "Client-side HTTP upgrade requested for a server");
}
if (prefaceSent() || decoder.prefaceReceived()) {
throw connectionError(PROTOCOL_ERROR, "HTTP upgrade must occur before HTTP/2 preface is sent or received");
if (!prefaceSent()) {
// If the preface was not sent yet it most likely means the handler was not added to the pipeline before
// calling this method.
throw connectionError(INTERNAL_ERROR, "HTTP upgrade must occur after preface was sent");
}
if (decoder.prefaceReceived()) {
throw connectionError(PROTOCOL_ERROR, "HTTP upgrade must occur before HTTP/2 preface is received");
}
// Create a local stream used for the HTTP cleartext upgrade.
@ -172,8 +177,13 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
if (!connection().isServer()) {
throw connectionError(PROTOCOL_ERROR, "Server-side HTTP upgrade requested for a client");
}
if (prefaceSent() || decoder.prefaceReceived()) {
throw connectionError(PROTOCOL_ERROR, "HTTP upgrade must occur before HTTP/2 preface is sent or received");
if (!prefaceSent()) {
// If the preface was not sent yet it most likely means the handler was not added to the pipeline before
// calling this method.
throw connectionError(INTERNAL_ERROR, "HTTP upgrade must occur after preface was sent");
}
if (decoder.prefaceReceived()) {
throw connectionError(PROTOCOL_ERROR, "HTTP upgrade must occur before HTTP/2 preface is received");
}
// Apply the settings but no ACK is necessary.

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

@ -49,12 +49,15 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
private static final InternalLogger logger = InternalLoggerFactory.getInstance(Http2ServerUpgradeCodec.class);
private static final List<CharSequence> REQUIRED_UPGRADE_HEADERS =
Collections.singletonList(HTTP_UPGRADE_SETTINGS_HEADER);
private static final ChannelHandler[] EMPTY_HANDLERS = new ChannelHandler[0];
private final String handlerName;
private final Http2ConnectionHandler connectionHandler;
private final ChannelHandler upgradeToHandler;
private final ChannelHandler[] handlers;
private final Http2FrameReader frameReader;
private Http2Settings settings;
/**
* Creates the codec using a default name for the connection handler when adding to the
* pipeline.
@ -62,7 +65,7 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ServerUpgradeCodec(Http2ConnectionHandler connectionHandler) {
this(null, connectionHandler);
this(null, connectionHandler, EMPTY_HANDLERS);
}
/**
@ -72,7 +75,7 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* @param http2Codec the HTTP/2 multiplexing handler.
*/
public Http2ServerUpgradeCodec(Http2MultiplexCodec http2Codec) {
this(null, http2Codec);
this(null, http2Codec, EMPTY_HANDLERS);
}
/**
@ -83,7 +86,7 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* @param connectionHandler the HTTP/2 connection handler
*/
public Http2ServerUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler) {
this(handlerName, connectionHandler, connectionHandler);
this(handlerName, connectionHandler, EMPTY_HANDLERS);
}
/**
@ -93,7 +96,7 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* @param http2Codec the HTTP/2 multiplexing handler.
*/
public Http2ServerUpgradeCodec(String handlerName, Http2MultiplexCodec http2Codec) {
this(handlerName, http2Codec, http2Codec);
this(handlerName, http2Codec, EMPTY_HANDLERS);
}
/**
@ -103,23 +106,15 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
* @param http2Codec the HTTP/2 frame handler.
* @param handlers the handlers that will handle the {@link Http2Frame}s.
*/
public Http2ServerUpgradeCodec(final Http2FrameCodec http2Codec, final ChannelHandler... handlers) {
this(null, http2Codec, new ChannelHandlerAdapter() {
@Override
public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
ctx.pipeline().addLast(http2Codec);
ctx.pipeline().addLast(handlers);
ctx.pipeline().remove(this);
}
});
public Http2ServerUpgradeCodec(Http2FrameCodec http2Codec, ChannelHandler... handlers) {
this(null, http2Codec, handlers);
}
Http2ServerUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler,
ChannelHandler upgradeToHandler) {
private Http2ServerUpgradeCodec(String handlerName, Http2ConnectionHandler connectionHandler,
ChannelHandler... handlers) {
this.handlerName = handlerName;
this.connectionHandler = checkNotNull(connectionHandler, "connectionHandler");
this.upgradeToHandler = checkNotNull(upgradeToHandler, "upgradeToHandler");
this.connectionHandler = connectionHandler;
this.handlers = handlers;
frameReader = new DefaultHttp2FrameReader();
}
@ -139,8 +134,7 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
throw new IllegalArgumentException("There must be 1 and only 1 "
+ HTTP_UPGRADE_SETTINGS_HEADER + " header.");
}
Http2Settings settings = decodeSettingsHeader(ctx, upgradeHeaders.get(0));
connectionHandler.onHttpServerUpgrade(settings);
settings = decodeSettingsHeader(ctx, upgradeHeaders.get(0));
// Everything looks good.
return true;
} catch (Throwable cause) {
@ -151,8 +145,23 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade
@Override
public void upgradeTo(final ChannelHandlerContext ctx, FullHttpRequest upgradeRequest) {
// Add the HTTP/2 connection handler to the pipeline immediately following the current handler.
ctx.pipeline().addAfter(ctx.name(), handlerName, upgradeToHandler);
try {
// Add the HTTP/2 connection handler to the pipeline immediately following the current handler.
ctx.pipeline().addAfter(ctx.name(), handlerName, connectionHandler);
connectionHandler.onHttpServerUpgrade(settings);
} catch (Http2Exception e) {
ctx.fireExceptionCaught(e);
ctx.close();
return;
}
if (handlers != null) {
final String name = ctx.pipeline().context(connectionHandler).name();
for (int i = handlers.length - 1; i >= 0; i--) {
ctx.pipeline().addAfter(name, null, handlers[i]);
}
}
}
/**

18
codec-http2/src/test/java/io/netty/handler/codec/http2/CleartextHttp2ServerUpgradeHandlerTest.java
View File

@ -60,7 +60,8 @@ public class CleartextHttp2ServerUpgradeHandlerTest {
private void setUpServerChannel() {
frameListener = mock(Http2FrameListener.class);
http2ConnectionHandler = new Http2ConnectionHandlerBuilder().frameListener(frameListener).build();
http2ConnectionHandler = new Http2ConnectionHandlerBuilder()
.frameListener(frameListener).build();
UpgradeCodecFactory upgradeCodecFactory = new UpgradeCodecFactory() {
@Override
@ -138,6 +139,21 @@ public class CleartextHttp2ServerUpgradeHandlerTest {
Http2Stream stream = http2ConnectionHandler.connection().stream(1);
assertEquals(State.HALF_CLOSED_REMOTE, stream.state());
assertFalse(stream.isHeadersSent());
String expectedHttpResponse = "HTTP/1.1 101 Switching Protocols\r\n" +
"connection: upgrade\r\n" +
"upgrade: h2c\r\n" +
"content-length: 0\r\n\r\n";
ByteBuf responseBuffer = channel.readOutbound();
assertEquals(expectedHttpResponse, responseBuffer.toString(CharsetUtil.UTF_8));
responseBuffer.release();
// Check that the preface was send (a.k.a the settings frame)
ByteBuf settingsBuffer = channel.readOutbound();
assertNotNull(settingsBuffer);
settingsBuffer.release();
assertNull(channel.readOutbound());
}
@Test

23
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
View File

@ -57,6 +57,7 @@ import static io.netty.util.CharsetUtil.UTF_8;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyInt;
@ -217,6 +218,28 @@ public class Http2ConnectionHandlerTest {
}
}
@Test
public void onHttpServerUpgradeWithoutHandlerAdded() throws Exception {
handler = new Http2ConnectionHandlerBuilder().frameListener(new Http2FrameAdapter()).server(true).build();
try {
handler.onHttpServerUpgrade(new Http2Settings());
fail();
} catch (Http2Exception e) {
assertEquals(Http2Error.INTERNAL_ERROR, e.error());
}
}
@Test
public void onHttpClientUpgradeWithoutHandlerAdded() throws Exception {
handler = new Http2ConnectionHandlerBuilder().frameListener(new Http2FrameAdapter()).server(false).build();
try {
handler.onHttpClientUpgrade();
fail();
} catch (Http2Exception e) {
assertEquals(Http2Error.INTERNAL_ERROR, e.error());
}
}
@Test
public void clientShouldSendClientPrefaceStringWhenActive() throws Exception {
when(connection.isServer()).thenReturn(false);

82
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodecTest.java Normal file
View File

@ -0,0 +1,82 @@
/*
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License, version 2.0 (the
* "License"); you may not use this file except in compliance with the License. You may obtain a
* copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultHttpHeaders;
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpVersion;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
public class Http2ServerUpgradeCodecTest {
@Test
public void testUpgradeToHttp2ConnectionHandler() {
testUpgrade(new Http2ConnectionHandlerBuilder().frameListener(new Http2FrameAdapter()).build());
}
@Test
public void testUpgradeToHttp2FrameCodec() {
testUpgrade(new Http2FrameCodecBuilder(true).build());
}
@Test
public void testUpgradeToHttp2MultiplexCodec() {
testUpgrade(new Http2MultiplexCodecBuilder(true, new HttpInboundHandler()).build());
}
private static void testUpgrade(Http2ConnectionHandler handler) {
FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.OPTIONS, "*");
request.headers().set(HttpHeaderNames.HOST, "netty.io");
request.headers().set(HttpHeaderNames.CONNECTION, "Upgrade, HTTP2-Settings");
request.headers().set(HttpHeaderNames.UPGRADE, "h2c");
request.headers().set("HTTP2-Settings", "AAMAAABkAAQAAP__");
EmbeddedChannel channel = new EmbeddedChannel(new ChannelInboundHandlerAdapter());
ChannelHandlerContext ctx = channel.pipeline().firstContext();
Http2ServerUpgradeCodec codec = new Http2ServerUpgradeCodec("connectionHandler", handler);
assertTrue(codec.prepareUpgradeResponse(ctx, request, new DefaultHttpHeaders()));
codec.upgradeTo(ctx, request);
// Flush the channel to ensure we write out all buffered data
channel.flush();
assertSame(handler, channel.pipeline().remove("connectionHandler"));
assertNull(channel.pipeline().get(handler.getClass()));
assertTrue(channel.finish());
// Check that the preface was send (a.k.a the settings frame)
ByteBuf settingsBuffer = channel.readOutbound();
assertNotNull(settingsBuffer);
settingsBuffer.release();
assertNull(channel.readOutbound());
}
@ChannelHandler.Sharable
private static final class HttpInboundHandler extends ChannelInboundHandlerAdapter { }
}