From 8ec594c6eb32f34f154979bf2bd35368055912a1 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 14 Mar 2016 13:55:52 +0100 Subject: [PATCH] Change HttpServerUpgradeHandler.UpgradeCodec to allow aborting upgrade Motivation: HttpServerUpgradeHandler.UpgradeCodec.prepareUpgradeResponse should allow to abort the upgrade and so just continue with using HTTP. Beside this we should only pass in the response HttpHeaders as this is inline with the docs. Modifications: - UpgradeCodec.prepareUpgradeResponse now allows to return a boolean and so allows to specifiy if the upgrade should take place. - Change the param from FullHttpResponse to HttpHeaders to be inline with the javadocs. Result: More flexible and correct handling of upgrades. --- .../codec/http/HttpServerUpgradeHandler.java | 23 ++++++++++++------- .../codec/http2/Http2ServerUpgradeCodec.java | 19 ++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java index 1c9f8cda85..0d513ef9bc 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java @@ -60,10 +60,15 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator { Collection requiredUpgradeHeaders(); /** - * Adds any headers to the 101 Switching protocols response that are appropriate for this protocol. + * Prepares the {@code upgradeHeaders} for a protocol update based upon the contents of {@code upgradeRequest}. + * This method returns a boolean value to proceed or abort the upgrade in progress. If {@code false} is + * returned, the upgrade is aborted and the {@code upgradeRequest} will be passed through the inbound pipeline + * as if no upgrade was performed. If {@code true} is returned, the upgrade will proceed to the next + * step which invokes {@link #upgradeTo}. When returning {@code true}, you can add headers to + * the {@code upgradeHeaders} so that they are added to the 101 Switching protocols response. */ - void prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest, - FullHttpResponse upgradeResponse); + boolean prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest, + HttpHeaders upgradeHeaders); /** * Performs an HTTP protocol upgrade from the source codec. This method is responsible for @@ -98,7 +103,7 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator { private final CharSequence protocol; private final FullHttpRequest upgradeRequest; - private UpgradeEvent(CharSequence protocol, FullHttpRequest upgradeRequest) { + UpgradeEvent(CharSequence protocol, FullHttpRequest upgradeRequest) { this.protocol = protocol; this.upgradeRequest = upgradeRequest; } @@ -299,13 +304,15 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator { } } - // Create the user event to be fired once the upgrade completes. - final UpgradeEvent event = new UpgradeEvent(upgradeProtocol, request); - // Prepare and send the upgrade response. Wait for this write to complete before upgrading, // since we need the old codec in-place to properly encode the response. final FullHttpResponse upgradeResponse = createUpgradeResponse(upgradeProtocol); - upgradeCodec.prepareUpgradeResponse(ctx, request, upgradeResponse); + if (!upgradeCodec.prepareUpgradeResponse(ctx, request, upgradeResponse.headers())) { + return false; + } + + // Create the user event to be fired once the upgrade completes. + final UpgradeEvent event = new UpgradeEvent(upgradeProtocol, request); final UpgradeCodec finalUpgradeCodec = upgradeCodec; ctx.writeAndFlush(upgradeResponse).addListener(new ChannelFutureListener() { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodec.java index 3382fc2938..5affc301e5 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerUpgradeCodec.java @@ -20,8 +20,11 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.base64.Base64; import io.netty.handler.codec.http.FullHttpRequest; import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpServerUpgradeHandler; import io.netty.util.CharsetUtil; +import io.netty.util.internal.logging.InternalLogger; +import io.netty.util.internal.logging.InternalLoggerFactory; import java.nio.CharBuffer; import java.util.Collection; @@ -29,7 +32,6 @@ 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_SETTINGS_HEADER; import static io.netty.handler.codec.http2.Http2CodecUtil.writeFrameHeader; @@ -41,6 +43,7 @@ import static io.netty.util.internal.ObjectUtil.checkNotNull; */ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.UpgradeCodec { + private static final InternalLogger logger = InternalLoggerFactory.getInstance(Http2ServerUpgradeCodec.class); private static final List REQUIRED_UPGRADE_HEADERS = Collections.singletonList(HTTP_UPGRADE_SETTINGS_HEADER); @@ -77,8 +80,8 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade } @Override - public void prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest, - FullHttpResponse upgradeResponse) { + public boolean prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest, + HttpHeaders headers) { try { // Decode the HTTP2-Settings header and set the settings on the handler to make // sure everything is fine with the request. @@ -89,11 +92,11 @@ public class Http2ServerUpgradeCodec implements HttpServerUpgradeHandler.Upgrade } Http2Settings settings = decodeSettingsHeader(ctx, upgradeHeaders.get(0)); connectionHandler.onHttpServerUpgrade(settings); - // Everything looks good, no need to modify the response. - } catch (Throwable e) { - // Send a failed response back to the client. - upgradeResponse.setStatus(BAD_REQUEST); - upgradeResponse.headers().clear(); + // Everything looks good. + return true; + } catch (Throwable cause) { + logger.info("Error during upgrade to HTTP/2", cause); + return false; } }