From 33de96f4487c594c243cfc138be4592a51651451 Mon Sep 17 00:00:00 2001 From: Andrey Mizurov Date: Sat, 24 Oct 2020 15:41:11 +0300 Subject: [PATCH] Provide new client and server websocket handshake exceptions (#10646) Motivation: At the moment we have only one base `WebSocketHandshakeException` for handling WebSocket upgrade issues. Unfortunately, this message contains only a string message about the cause of the failure, which is inconvenient in handling. Modification: Provide new `WebSocketClientHandshakeException` with `HttpResponse` field and `WebSocketServerHandshakeException` with `HttpRequest` field both of them without content for avoid reference counting problems. Result: More information for more flexible handling. Fixes #10277 #4528 #10639. --- .../WebSocketClientHandshakeException.java | 55 ++++++++++++++ .../websocketx/WebSocketClientHandshaker.java | 4 +- .../WebSocketClientHandshaker00.java | 22 +++--- .../WebSocketClientHandshaker07.java | 19 +++-- .../WebSocketClientHandshaker08.java | 19 +++-- .../WebSocketClientHandshaker13.java | 19 +++-- .../WebSocketClientHandshakerFactory.java | 4 +- .../WebSocketClientProtocolHandler.java | 5 ++ ...bSocketClientProtocolHandshakeHandler.java | 5 +- .../websocketx/WebSocketProtocolHandler.java | 10 ++- .../WebSocketServerHandshakeException.java | 55 ++++++++++++++ .../WebSocketServerHandshaker00.java | 5 +- .../WebSocketServerHandshaker07.java | 2 +- .../WebSocketServerHandshaker08.java | 2 +- .../WebSocketServerHandshaker13.java | 2 +- .../WebSocketServerProtocolHandler.java | 5 ++ ...bSocketServerProtocolHandshakeHandler.java | 2 +- .../WebSocketClientHandshakerTest.java | 23 ++++++ .../WebSocketHandshakeExceptionTest.java | 74 +++++++++++++++++++ .../WebSocketServerHandshakerTest.java | 20 +++++ 20 files changed, 296 insertions(+), 56 deletions(-) create mode 100644 codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakeException.java create mode 100644 codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakeException.java create mode 100644 codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeExceptionTest.java diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakeException.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakeException.java new file mode 100644 index 0000000000..17c9381cfb --- /dev/null +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakeException.java @@ -0,0 +1,55 @@ +/* + * Copyright 2020 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.http.websocketx; + +import io.netty.handler.codec.http.DefaultHttpResponse; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.util.ReferenceCounted; + +/** + * Client exception during handshaking process. + * + *

IMPORTANT: This exception does not contain any {@link ReferenceCounted} fields + * e.g. {@link FullHttpResponse}, so no special treatment is needed. + */ +public final class WebSocketClientHandshakeException extends WebSocketHandshakeException { + + private static final long serialVersionUID = 1L; + + private final HttpResponse response; + + public WebSocketClientHandshakeException(String message) { + this(message, null); + } + + public WebSocketClientHandshakeException(String message, HttpResponse httpResponse) { + super(message); + if (httpResponse != null) { + response = new DefaultHttpResponse(httpResponse.protocolVersion(), + httpResponse.status(), httpResponse.headers()); + } else { + response = null; + } + } + + /** + * Returns a {@link HttpResponse response} if exception occurs during response validation otherwise {@code null}. + */ + public HttpResponse response() { + return response; + } +} diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java index 0f3f414239..a90fb79fa9 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java @@ -324,9 +324,9 @@ public abstract class WebSocketClientHandshaker { } // else mixed cases - which are all errors if (!protocolValid) { - throw new WebSocketHandshakeException(String.format( + throw new WebSocketClientHandshakeException(String.format( "Invalid subprotocol. Actual: %s. Expected one of: %s", - receivedProtocol, expectedSubprotocol)); + receivedProtocol, expectedSubprotocol), response); } setHandshakeComplete(); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java index c4c64dbbdc..55b421bf07 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java @@ -26,7 +26,6 @@ import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; -import io.netty.util.AsciiString; import java.net.URI; import java.nio.ByteBuffer; @@ -43,8 +42,6 @@ import java.nio.ByteBuffer; */ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { - private static final AsciiString WEBSOCKET = AsciiString.cached("WebSocket"); - private ByteBuf expectedChallengeResponseBytes; /** @@ -186,7 +183,7 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { headers.add(customHeaders); } - headers.set(HttpHeaderNames.UPGRADE, WEBSOCKET) + headers.set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET) .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE) .set(HttpHeaderNames.HOST, websocketHostValue(wsURL)) .set(HttpHeaderNames.SEC_WEBSOCKET_KEY1, key1) @@ -229,26 +226,25 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { */ @Override protected void verify(FullHttpResponse response) { - if (!response.status().equals(HttpResponseStatus.SWITCHING_PROTOCOLS)) { - throw new WebSocketHandshakeException("Invalid handshake response getStatus: " + response.status()); + HttpResponseStatus status = response.status(); + if (!HttpResponseStatus.SWITCHING_PROTOCOLS.equals(status)) { + throw new WebSocketClientHandshakeException("Invalid handshake response getStatus: " + status, response); } HttpHeaders headers = response.headers(); - CharSequence upgrade = headers.get(HttpHeaderNames.UPGRADE); - if (!WEBSOCKET.contentEqualsIgnoreCase(upgrade)) { - throw new WebSocketHandshakeException("Invalid handshake response upgrade: " - + upgrade); + if (!HttpHeaderValues.WEBSOCKET.contentEqualsIgnoreCase(upgrade)) { + throw new WebSocketClientHandshakeException("Invalid handshake response upgrade: " + upgrade, response); } if (!headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true)) { - throw new WebSocketHandshakeException("Invalid handshake response connection: " - + headers.get(HttpHeaderNames.CONNECTION)); + throw new WebSocketClientHandshakeException("Invalid handshake response connection: " + + headers.get(HttpHeaderNames.CONNECTION), response); } ByteBuf challenge = response.content(); if (!challenge.equals(expectedChallengeResponseBytes)) { - throw new WebSocketHandshakeException("Invalid challenge"); + throw new WebSocketClientHandshakeException("Invalid challenge", response); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java index bfa439b1aa..924978a256 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java @@ -264,27 +264,26 @@ public class WebSocketClientHandshaker07 extends WebSocketClientHandshaker { */ @Override protected void verify(FullHttpResponse response) { - final HttpResponseStatus status = HttpResponseStatus.SWITCHING_PROTOCOLS; - final HttpHeaders headers = response.headers(); - - if (!response.status().equals(status)) { - throw new WebSocketHandshakeException("Invalid handshake response getStatus: " + response.status()); + HttpResponseStatus status = response.status(); + if (!HttpResponseStatus.SWITCHING_PROTOCOLS.equals(status)) { + throw new WebSocketClientHandshakeException("Invalid handshake response getStatus: " + status, response); } + HttpHeaders headers = response.headers(); CharSequence upgrade = headers.get(HttpHeaderNames.UPGRADE); if (!HttpHeaderValues.WEBSOCKET.contentEqualsIgnoreCase(upgrade)) { - throw new WebSocketHandshakeException("Invalid handshake response upgrade: " + upgrade); + throw new WebSocketClientHandshakeException("Invalid handshake response upgrade: " + upgrade, response); } if (!headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true)) { - throw new WebSocketHandshakeException("Invalid handshake response connection: " - + headers.get(HttpHeaderNames.CONNECTION)); + throw new WebSocketClientHandshakeException("Invalid handshake response connection: " + + headers.get(HttpHeaderNames.CONNECTION), response); } CharSequence accept = headers.get(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT); if (accept == null || !accept.equals(expectedChallengeResponseString)) { - throw new WebSocketHandshakeException(String.format( - "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString)); + throw new WebSocketClientHandshakeException(String.format( + "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString), response); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java index 762f85e1a4..f24e7c469a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java @@ -266,27 +266,26 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker { */ @Override protected void verify(FullHttpResponse response) { - final HttpResponseStatus status = HttpResponseStatus.SWITCHING_PROTOCOLS; - final HttpHeaders headers = response.headers(); - - if (!response.status().equals(status)) { - throw new WebSocketHandshakeException("Invalid handshake response getStatus: " + response.status()); + HttpResponseStatus status = response.status(); + if (!HttpResponseStatus.SWITCHING_PROTOCOLS.equals(status)) { + throw new WebSocketClientHandshakeException("Invalid handshake response getStatus: " + status, response); } + HttpHeaders headers = response.headers(); CharSequence upgrade = headers.get(HttpHeaderNames.UPGRADE); if (!HttpHeaderValues.WEBSOCKET.contentEqualsIgnoreCase(upgrade)) { - throw new WebSocketHandshakeException("Invalid handshake response upgrade: " + upgrade); + throw new WebSocketClientHandshakeException("Invalid handshake response upgrade: " + upgrade, response); } if (!headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true)) { - throw new WebSocketHandshakeException("Invalid handshake response connection: " - + headers.get(HttpHeaderNames.CONNECTION)); + throw new WebSocketClientHandshakeException("Invalid handshake response connection: " + + headers.get(HttpHeaderNames.CONNECTION), response); } CharSequence accept = headers.get(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT); if (accept == null || !accept.equals(expectedChallengeResponseString)) { - throw new WebSocketHandshakeException(String.format( - "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString)); + throw new WebSocketClientHandshakeException(String.format( + "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString), response); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java index 0afcbc06b1..70805611fa 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java @@ -267,27 +267,26 @@ public class WebSocketClientHandshaker13 extends WebSocketClientHandshaker { */ @Override protected void verify(FullHttpResponse response) { - final HttpResponseStatus status = HttpResponseStatus.SWITCHING_PROTOCOLS; - final HttpHeaders headers = response.headers(); - - if (!response.status().equals(status)) { - throw new WebSocketHandshakeException("Invalid handshake response getStatus: " + response.status()); + HttpResponseStatus status = response.status(); + if (!HttpResponseStatus.SWITCHING_PROTOCOLS.equals(status)) { + throw new WebSocketClientHandshakeException("Invalid handshake response getStatus: " + status, response); } + HttpHeaders headers = response.headers(); CharSequence upgrade = headers.get(HttpHeaderNames.UPGRADE); if (!HttpHeaderValues.WEBSOCKET.contentEqualsIgnoreCase(upgrade)) { - throw new WebSocketHandshakeException("Invalid handshake response upgrade: " + upgrade); + throw new WebSocketClientHandshakeException("Invalid handshake response upgrade: " + upgrade, response); } if (!headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true)) { - throw new WebSocketHandshakeException("Invalid handshake response connection: " - + headers.get(HttpHeaderNames.CONNECTION)); + throw new WebSocketClientHandshakeException("Invalid handshake response connection: " + + headers.get(HttpHeaderNames.CONNECTION), response); } CharSequence accept = headers.get(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT); if (accept == null || !accept.equals(expectedChallengeResponseString)) { - throw new WebSocketHandshakeException(String.format( - "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString)); + throw new WebSocketClientHandshakeException(String.format( + "Invalid challenge. Actual: %s. Expected: %s", accept, expectedChallengeResponseString), response); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerFactory.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerFactory.java index 44655e1ddf..a5dabdcbd5 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerFactory.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerFactory.java @@ -162,7 +162,7 @@ public final class WebSocketClientHandshakerFactory { webSocketURL, V00, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis); } - throw new WebSocketHandshakeException("Protocol version " + version + " not supported."); + throw new WebSocketClientHandshakeException("Protocol version " + version + " not supported."); } /** @@ -220,6 +220,6 @@ public final class WebSocketClientHandshakerFactory { maxFramePayloadLength, forceCloseTimeoutMillis, absoluteUpgradeUrl); } - throw new WebSocketHandshakeException("Protocol version " + version + " not supported."); + throw new WebSocketClientHandshakeException("Protocol version " + version + " not supported."); } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandler.java index 400204dae6..3c08a17749 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandler.java @@ -370,6 +370,11 @@ public class WebSocketClientProtocolHandler extends WebSocketProtocolHandler { super.decode(ctx, frame, out); } + @Override + protected WebSocketClientHandshakeException buildHandshakeException(String message) { + return new WebSocketClientHandshakeException(message); + } + @Override public void handlerAdded(ChannelHandlerContext ctx) { ChannelPipeline cp = ctx.pipeline(); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandshakeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandshakeHandler.java index bab6befd8a..0ef2ac2c35 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandshakeHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientProtocolHandshakeHandler.java @@ -73,7 +73,8 @@ class WebSocketClientProtocolHandshakeHandler extends ChannelInboundHandlerAdapt @Override public void channelInactive(ChannelHandlerContext ctx) throws Exception { if (!handshakePromise.isDone()) { - handshakePromise.tryFailure(new WebSocketHandshakeException("channel closed with handshake in progress")); + handshakePromise.tryFailure(new WebSocketClientHandshakeException("channel closed with handshake " + + "in progress")); } super.channelInactive(ctx); @@ -115,7 +116,7 @@ class WebSocketClientProtocolHandshakeHandler extends ChannelInboundHandlerAdapt return; } - if (localHandshakePromise.tryFailure(new WebSocketHandshakeException("handshake timed out"))) { + if (localHandshakePromise.tryFailure(new WebSocketClientHandshakeException("handshake timed out"))) { ctx.flush() .fireUserEventTriggered(ClientHandshakeStateEvent.HANDSHAKE_TIMEOUT) .close(); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java index f11d1f722d..59b6613622 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java @@ -128,7 +128,7 @@ abstract class WebSocketProtocolHandler extends MessageToMessageDecoderIMPORTANT: This exception does not contain any {@link ReferenceCounted} fields + * e.g. {@link FullHttpRequest}, so no special treatment is needed. + */ +public final class WebSocketServerHandshakeException extends WebSocketHandshakeException { + + private static final long serialVersionUID = 1L; + + private final HttpRequest request; + + public WebSocketServerHandshakeException(String message) { + this(message, null); + } + + public WebSocketServerHandshakeException(String message, HttpRequest httpRequest) { + super(message); + if (httpRequest != null) { + request = new DefaultHttpRequest(httpRequest.protocolVersion(), httpRequest.method(), + httpRequest.uri(), httpRequest.headers()); + } else { + request = null; + } + } + + /** + * Returns a {@link HttpRequest request} if exception occurs during request validation otherwise {@code null}. + */ + public HttpRequest request() { + return request; + } +} diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java index 07fe026970..046bf664c1 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java @@ -126,7 +126,7 @@ public class WebSocketServerHandshaker00 extends WebSocketServerHandshaker { // Serve the WebSocket handshake request. if (!req.headers().containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true) || !HttpHeaderValues.WEBSOCKET.contentEqualsIgnoreCase(req.headers().get(HttpHeaderNames.UPGRADE))) { - throw new WebSocketHandshakeException("not a WebSocket handshake request: missing upgrade"); + throw new WebSocketServerHandshakeException("not a WebSocket handshake request: missing upgrade", req); } // Hixie 75 does not contain these headers while Hixie 76 does @@ -136,7 +136,8 @@ public class WebSocketServerHandshaker00 extends WebSocketServerHandshaker { String origin = req.headers().get(HttpHeaderNames.ORIGIN); //throw before allocating FullHttpResponse if (origin == null && !isHixie76) { - throw new WebSocketHandshakeException("Missing origin header, got only " + req.headers().names()); + throw new WebSocketServerHandshakeException("Missing origin header, got only " + req.headers().names(), + req); } // Create the WebSocket handshake response. diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java index 25307cebaf..bde7e6ed4a 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java @@ -130,7 +130,7 @@ public class WebSocketServerHandshaker07 extends WebSocketServerHandshaker { protected FullHttpResponse newHandshakeResponse(FullHttpRequest req, HttpHeaders headers) { CharSequence key = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_KEY); if (key == null) { - throw new WebSocketHandshakeException("not a WebSocket request: missing key"); + throw new WebSocketServerHandshakeException("not a WebSocket request: missing key", req); } FullHttpResponse res = diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java index 3d99f66bfc..175031e53b 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java @@ -137,7 +137,7 @@ public class WebSocketServerHandshaker08 extends WebSocketServerHandshaker { protected FullHttpResponse newHandshakeResponse(FullHttpRequest req, HttpHeaders headers) { CharSequence key = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_KEY); if (key == null) { - throw new WebSocketHandshakeException("not a WebSocket request: missing key"); + throw new WebSocketServerHandshakeException("not a WebSocket request: missing key", req); } FullHttpResponse res = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.SWITCHING_PROTOCOLS, diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java index 569349a8d9..77d37aea20 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java @@ -136,7 +136,7 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker { protected FullHttpResponse newHandshakeResponse(FullHttpRequest req, HttpHeaders headers) { CharSequence key = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_KEY); if (key == null) { - throw new WebSocketHandshakeException("not a WebSocket request: missing key"); + throw new WebSocketServerHandshakeException("not a WebSocket request: missing key", req); } FullHttpResponse res = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.SWITCHING_PROTOCOLS, diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java index ac03aa250f..94204d93c8 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java @@ -246,6 +246,11 @@ public class WebSocketServerProtocolHandler extends WebSocketProtocolHandler { super.decode(ctx, frame, out); } + @Override + protected WebSocketServerHandshakeException buildHandshakeException(String message) { + return new WebSocketServerHandshakeException(message); + } + @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { if (cause instanceof WebSocketHandshakeException) { diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandshakeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandshakeHandler.java index df2f18c8dc..2022225ffe 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandshakeHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandshakeHandler.java @@ -158,7 +158,7 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt @Override public void run() { if (!localHandshakePromise.isDone() && - localHandshakePromise.tryFailure(new WebSocketHandshakeException("handshake timed out"))) { + localHandshakePromise.tryFailure(new WebSocketServerHandshakeException("handshake timed out"))) { ctx.flush() .fireUserEventTriggered(ServerHandshakeStateEvent.HANDSHAKE_TIMEOUT) .close(); diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java index 9a75a4a30f..6df84fa987 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java @@ -21,6 +21,7 @@ import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.SimpleChannelInboundHandler; import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.EmptyHttpHeaders; import io.netty.handler.codec.http.FullHttpRequest; @@ -31,6 +32,8 @@ import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpObjectAggregator; import io.netty.handler.codec.http.HttpRequestEncoder; import io.netty.handler.codec.http.HttpResponseDecoder; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpVersion; import io.netty.util.CharsetUtil; import org.junit.Test; @@ -387,4 +390,24 @@ public abstract class WebSocketClientHandshakerTest { request.release(); } + + @Test + public void testWebSocketClientHandshakeException() { + URI uri = URI.create("ws://localhost:9999/exception"); + WebSocketClientHandshaker handshaker = newHandshaker(uri, null, null, false); + FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.UNAUTHORIZED); + response.headers().set(HttpHeaderNames.WWW_AUTHENTICATE, "realm = access token required"); + + try { + handshaker.finishHandshake(null, response); + } catch (WebSocketClientHandshakeException exception) { + assertEquals("Invalid handshake response getStatus: 401 Unauthorized", exception.getMessage()); + assertEquals(HttpResponseStatus.UNAUTHORIZED, exception.response().status()); + assertTrue(exception.response().headers().contains(HttpHeaderNames.WWW_AUTHENTICATE, + "realm = access token required", false)); + } finally { + response.release(); + } + } } + diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeExceptionTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeExceptionTest.java new file mode 100644 index 0000000000..609411aada --- /dev/null +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeExceptionTest.java @@ -0,0 +1,74 @@ +/* + * Copyright 2020 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.http.websocketx; + +import io.netty.handler.codec.http.DefaultHttpRequest; +import io.netty.handler.codec.http.DefaultHttpResponse; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpVersion; +import org.junit.Test; + +import static org.junit.Assert.*; + +public class WebSocketHandshakeExceptionTest { + + @Test + public void testClientExceptionWithoutResponse() { + WebSocketClientHandshakeException clientException = new WebSocketClientHandshakeException("client message"); + + assertNull(clientException.response()); + assertEquals("client message", clientException.getMessage()); + } + + @Test + public void testClientExceptionWithResponse() { + HttpResponse httpResponse = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.BAD_REQUEST); + httpResponse.headers().set("x-header", "x-value"); + WebSocketClientHandshakeException clientException = new WebSocketClientHandshakeException("client message", + httpResponse); + + assertNotNull(clientException.response()); + assertEquals("client message", clientException.getMessage()); + assertEquals(HttpResponseStatus.BAD_REQUEST, clientException.response().status()); + assertEquals(httpResponse.headers(), clientException.response().headers()); + } + + @Test + public void testServerExceptionWithoutRequest() { + WebSocketServerHandshakeException serverException = new WebSocketServerHandshakeException("server message"); + + assertNull(serverException.request()); + assertEquals("server message", serverException.getMessage()); + } + + @Test + public void testClientExceptionWithRequest() { + HttpRequest httpRequest = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, + "ws://localhost:9999/ws"); + httpRequest.headers().set("x-header", "x-value"); + WebSocketServerHandshakeException serverException = new WebSocketServerHandshakeException("server message", + httpRequest); + + assertNotNull(serverException.request()); + assertEquals("server message", serverException.getMessage()); + assertEquals(HttpMethod.GET, serverException.request().method()); + assertEquals(httpRequest.headers(), serverException.request().headers()); + assertEquals(httpRequest.uri(), serverException.request().uri()); + } +} diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakerTest.java index 87bb767231..fcc6e7f60f 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakerTest.java @@ -78,4 +78,24 @@ public abstract class WebSocketServerHandshakerTest { } } } + + @Test + public void testWebSocketServerHandshakeException() { + WebSocketServerHandshaker serverHandshaker = newHandshaker("ws://example.com/chat", + "chat", WebSocketDecoderConfig.DEFAULT); + + FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, + "ws://example.com/chat"); + request.headers().set("x-client-header", "value"); + try { + serverHandshaker.handshake(null, request, null, null); + } catch (WebSocketServerHandshakeException exception) { + assertNotNull(exception.getMessage()); + assertEquals(request.headers(), exception.request().headers()); + assertEquals(HttpMethod.GET, exception.request().method()); + } finally { + request.release(); + } + } } +