Skip to content

Commit

Permalink
Fix generating the Origin header value for websocket handshake requ…
Browse files Browse the repository at this point in the history 
…est (#12941)

Motivation:

We have the old erroneous behavior of generating the `Origin| Sec-WebSocket-Origin` for client websocket handshake request (#9673). In Netty5 this fixed and auto-generation has been deleted at all, only if the client passed the `Origin` header via custom headers. The same we can do for Netty4 but it could potentially break some clients (unlikely), or introduce an additional parameter to disable or enable this behavior.

Modification:

Introduce new `generateOriginHeader` parameter in client config and generate the `Origin|Sec-WebSocket-Origin` header value only if it enabled. Add additional check for webSocketURI if it contains host or passed through `customHeaders` to prevent NPE in `newHandshakeRequest()`.

Result:

Fixes #9673 #12933 


Co-authored-by: Norman Maurer <norman_maurer@apple.com>
  • Loading branch information
@amizurov @normanmaurer
amizurov and normanmaurer committed Nov 10, 2022
1 parent bb86eb0 commit ae9b823
Show file tree
Hide file tree
Showing 13 changed files with 401 additions and 51 deletions.
55 changes: 55 additions & 0 deletions ...-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java
View file
Expand Up @@ -85,6 +85,8 @@ public abstract class WebSocketClientHandshaker {

private final boolean absoluteUpgradeUrl;

protected final boolean generateOriginHeader;

/**
* Base constructor
*
Expand Down Expand Up @@ -151,13 +153,44 @@ protected WebSocketClientHandshaker(URI uri, WebSocketVersion version, String su
protected WebSocketClientHandshaker(URI uri, WebSocketVersion version, String subprotocol,
HttpHeaders customHeaders, int maxFramePayloadLength,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl) {
this(uri, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl, true);
}

/**
* Base constructor
*
* @param uri
* URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
* sent to this URL.
* @param version
* Version of web socket specification to use to connect to the server
* @param subprotocol
* Sub protocol request sent to the server.
* @param customHeaders
* Map of custom headers to add to the client request
* @param maxFramePayloadLength
* Maximum length of a frame's payload
* @param forceCloseTimeoutMillis
* Close the connection if it was not closed by the server after timeout specified
* @param absoluteUpgradeUrl
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
* @param generateOriginHeader
* Allows to generate the `Origin`|`Sec-WebSocket-Origin` header value for handshake request
* according to the given webSocketURL
*/
protected WebSocketClientHandshaker(URI uri, WebSocketVersion version, String subprotocol,
HttpHeaders customHeaders, int maxFramePayloadLength,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl, boolean generateOriginHeader) {
this.uri = uri;
this.version = version;
expectedSubprotocol = subprotocol;
this.customHeaders = customHeaders;
this.maxFramePayloadLength = maxFramePayloadLength;
this.forceCloseTimeoutMillis = forceCloseTimeoutMillis;
this.absoluteUpgradeUrl = absoluteUpgradeUrl;
this.generateOriginHeader = generateOriginHeader;
}

/**
Expand Down Expand Up @@ -265,6 +298,28 @@ public final ChannelFuture handshake(Channel channel, final ChannelPromise promi
}
}

if (uri.getHost() == null) {
if (customHeaders == null || !customHeaders.contains(HttpHeaderNames.HOST)) {
promise.setFailure(new IllegalArgumentException("Cannot generate the 'host' header value," +
" webSocketURI should contain host or passed through customHeaders"));
return promise;
}

if (generateOriginHeader && !customHeaders.contains(HttpHeaderNames.ORIGIN)) {
final String originName;
if (version == WebSocketVersion.V07 || version == WebSocketVersion.V08) {
originName = HttpHeaderNames.SEC_WEBSOCKET_ORIGIN.toString();
} else {
originName = HttpHeaderNames.ORIGIN.toString();
}

promise.setFailure(new IllegalArgumentException("Cannot generate the '" + originName + "' header" +
" value, webSocketURI should contain host or disable generateOriginHeader or pass value" +
" through customHeaders"));
return promise;
}
}

FullHttpRequest request = newHandshakeRequest();

channel.writeAndFlush(request).addListener(new ChannelFutureListener() {
Expand Down
46 changes: 42 additions & 4 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java
View file
Expand Up @@ -109,11 +109,42 @@ public WebSocketClientHandshaker00(URI webSocketURL, WebSocketVersion version, S
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
*/
WebSocketClientHandshaker00(URI webSocketURL, WebSocketVersion version, String subprotocol,
HttpHeaders customHeaders, int maxFramePayloadLength,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl) {
this(webSocketURL, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl, true);
}

/**
* Creates a new instance with the specified destination WebSocket location and version to initiate.
*
* @param webSocketURL
* URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
* sent to this URL.
* @param version
* Version of web socket specification to use to connect to the server
* @param subprotocol
* Sub protocol request sent to the server.
* @param customHeaders
* Map of custom headers to add to the client request
* @param maxFramePayloadLength
* Maximum length of a frame's payload
* @param forceCloseTimeoutMillis
* Close the connection if it was not closed by the server after timeout specified
* @param absoluteUpgradeUrl
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
* @param generateOriginHeader
* Allows to generate the `Origin` header value for handshake request
* according to the given webSocketURL
*/
WebSocketClientHandshaker00(URI webSocketURL, WebSocketVersion version, String subprotocol,
HttpHeaders customHeaders, int maxFramePayloadLength,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl) {
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl,
boolean generateOriginHeader) {
super(webSocketURL, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl);
absoluteUpgradeUrl, generateOriginHeader);
}

/**
Expand Down Expand Up @@ -182,15 +213,22 @@ protected FullHttpRequest newHandshakeRequest() {

if (customHeaders != null) {
headers.add(customHeaders);
if (!headers.contains(HttpHeaderNames.HOST)) {
// Only add HOST header if customHeaders did not contain it.
//
// See https://github.com/netty/netty/issues/10101
headers.set(HttpHeaderNames.HOST, websocketHostValue(wsURL));
}
} else {
headers.set(HttpHeaderNames.HOST, websocketHostValue(wsURL));
}

headers.set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
.set(HttpHeaderNames.HOST, websocketHostValue(wsURL))
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY1, key1)
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY2, key2);

if (!headers.contains(HttpHeaderNames.ORIGIN)) {
if (generateOriginHeader && !headers.contains(HttpHeaderNames.ORIGIN)) {
headers.set(HttpHeaderNames.ORIGIN, websocketOriginValue(wsURL));
}

Expand Down
46 changes: 43 additions & 3 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java
View file
Expand Up @@ -164,12 +164,52 @@ public WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, S
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
*/
WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch, long forceCloseTimeoutMillis,
boolean absoluteUpgradeUrl) {
this(webSocketURL, version, subprotocol, allowExtensions, customHeaders, maxFramePayloadLength, performMasking,
allowMaskMismatch, forceCloseTimeoutMillis, absoluteUpgradeUrl, true);
}

/**
* Creates a new instance.
*
* @param webSocketURL
* URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
* sent to this URL.
* @param version
* Version of web socket specification to use to connect to the server
* @param subprotocol
* Sub protocol request sent to the server.
* @param allowExtensions
* Allow extensions to be used in the reserved bits of the web socket frame
* @param customHeaders
* Map of custom headers to add to the client request
* @param maxFramePayloadLength
* Maximum length of a frame's payload
* @param performMasking
* Whether to mask all written websocket frames. This must be set to true in order to be fully compatible
* with the websocket specifications. Client applications that communicate with a non-standard server
* which doesn't require masking might set this to false to achieve a higher performance.
* @param allowMaskMismatch
* When set to true, frames which are not masked properly according to the standard will still be
* accepted
* @param forceCloseTimeoutMillis
* Close the connection if it was not closed by the server after timeout specified.
* @param absoluteUpgradeUrl
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
* @param generateOriginHeader
* Allows to generate a `Sec-WebSocket-Origin` header value for handshake request
* according to the given webSocketURL
*/
WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch, long forceCloseTimeoutMillis,
boolean absoluteUpgradeUrl) {
boolean absoluteUpgradeUrl, boolean generateOriginHeader) {
super(webSocketURL, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl);
absoluteUpgradeUrl, generateOriginHeader);
this.allowExtensions = allowExtensions;
this.performMasking = performMasking;
this.allowMaskMismatch = allowMaskMismatch;
Expand Down Expand Up @@ -232,7 +272,7 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key);

if (!headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
if (generateOriginHeader && !headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(wsURL));
}

Expand Down
48 changes: 44 additions & 4 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java
View file
Expand Up @@ -134,7 +134,7 @@ public WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, S
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch, long forceCloseTimeoutMillis) {
this(webSocketURL, version, subprotocol, allowExtensions, customHeaders, maxFramePayloadLength, performMasking,
allowMaskMismatch, forceCloseTimeoutMillis, false);
allowMaskMismatch, forceCloseTimeoutMillis, false, true);
}

/**
Expand Down Expand Up @@ -166,12 +166,52 @@ public WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, S
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
*/
WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch, long forceCloseTimeoutMillis,
boolean absoluteUpgradeUrl) {
this(webSocketURL, version, subprotocol, allowExtensions, customHeaders, maxFramePayloadLength, performMasking,
allowMaskMismatch, forceCloseTimeoutMillis, absoluteUpgradeUrl, true);
}

/**
* Creates a new instance.
*
* @param webSocketURL
* URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
* sent to this URL.
* @param version
* Version of web socket specification to use to connect to the server
* @param subprotocol
* Sub protocol request sent to the server.
* @param allowExtensions
* Allow extensions to be used in the reserved bits of the web socket frame
* @param customHeaders
* Map of custom headers to add to the client request
* @param maxFramePayloadLength
* Maximum length of a frame's payload
* @param performMasking
* Whether to mask all written websocket frames. This must be set to true in order to be fully compatible
* with the websocket specifications. Client applications that communicate with a non-standard server
* which doesn't require masking might set this to false to achieve a higher performance.
* @param allowMaskMismatch
* When set to true, frames which are not masked properly according to the standard will still be
* accepted
* @param forceCloseTimeoutMillis
* Close the connection if it was not closed by the server after timeout specified.
* @param absoluteUpgradeUrl
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
* @param generateOriginHeader
* Allows to generate a `Sec-WebSocket-Origin` header value for handshake request
* according to the given webSocketURL
*/
WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch, long forceCloseTimeoutMillis,
boolean absoluteUpgradeUrl) {
boolean absoluteUpgradeUrl, boolean generateOriginHeader) {
super(webSocketURL, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl);
absoluteUpgradeUrl, generateOriginHeader);
this.allowExtensions = allowExtensions;
this.performMasking = performMasking;
this.allowMaskMismatch = allowMaskMismatch;
Expand Down Expand Up @@ -234,7 +274,7 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key);

if (!headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
if (generateOriginHeader && !headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(wsURL));
}

Expand Down
48 changes: 44 additions & 4 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java
View file
Expand Up @@ -167,12 +167,53 @@ public WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, S
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
*/
WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl) {
this(webSocketURL, version, subprotocol, allowExtensions, customHeaders, maxFramePayloadLength, performMasking,
allowMaskMismatch, forceCloseTimeoutMillis, absoluteUpgradeUrl, true);
}

/**
* Creates a new instance.
*
* @param webSocketURL
* URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
* sent to this URL.
* @param version
* Version of web socket specification to use to connect to the server
* @param subprotocol
* Sub protocol request sent to the server.
* @param allowExtensions
* Allow extensions to be used in the reserved bits of the web socket frame
* @param customHeaders
* Map of custom headers to add to the client request
* @param maxFramePayloadLength
* Maximum length of a frame's payload
* @param performMasking
* Whether to mask all written websocket frames. This must be set to true in order to be fully compatible
* with the websocket specifications. Client applications that communicate with a non-standard server
* which doesn't require masking might set this to false to achieve a higher performance.
* @param allowMaskMismatch
* When set to true, frames which are not masked properly according to the standard will still be
* accepted
* @param forceCloseTimeoutMillis
* Close the connection if it was not closed by the server after timeout specified.
* @param absoluteUpgradeUrl
* Use an absolute url for the Upgrade request, typically when connecting through an HTTP proxy over
* clear HTTP
* @param generateOriginHeader
* Allows to generate the `Origin` header value for handshake request
* according to the given webSocketURL
*/
WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, String subprotocol,
boolean allowExtensions, HttpHeaders customHeaders, int maxFramePayloadLength,
boolean performMasking, boolean allowMaskMismatch,
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl) {
long forceCloseTimeoutMillis, boolean absoluteUpgradeUrl,
boolean generateOriginHeader) {
super(webSocketURL, version, subprotocol, customHeaders, maxFramePayloadLength, forceCloseTimeoutMillis,
absoluteUpgradeUrl);
absoluteUpgradeUrl, generateOriginHeader);
this.allowExtensions = allowExtensions;
this.performMasking = performMasking;
this.allowMaskMismatch = allowMaskMismatch;
Expand All @@ -190,7 +231,6 @@ public WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, S
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 13
* </pre>
Expand Down Expand Up @@ -235,7 +275,7 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key);

if (!headers.contains(HttpHeaderNames.ORIGIN)) {
if (generateOriginHeader && !headers.contains(HttpHeaderNames.ORIGIN)) {
headers.set(HttpHeaderNames.ORIGIN, websocketOriginValue(wsURL));
}

Expand Down

0 comments on commit ae9b823

Please sign in to comment.