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/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java

@@ -60,10 +60,15 @@ public class HttpServerUpgradeHandler extends HttpObjectAggregator {
         Collection<CharSequence> 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,
+        boolean prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest,
-                                    FullHttpResponse upgradeResponse);
+                                    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() {

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<CharSequence> 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,
+    public boolean prepareUpgradeResponse(ChannelHandlerContext ctx, FullHttpRequest upgradeRequest,
-            FullHttpResponse upgradeResponse) {
+            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.
+            // Everything looks good.
-        } catch (Throwable e) {
+            return true;
-            // Send a failed response back to the client.
+        } catch (Throwable cause) {
-            upgradeResponse.setStatus(BAD_REQUEST);
+            logger.info("Error during upgrade to HTTP/2", cause);
-            upgradeResponse.headers().clear();
+            return false;
         }
     }