Skip to content

Commit

Permalink
Bugfix #9673: Origin header is always sent from WebSocket client (#9692)
Browse files Browse the repository at this point in the history 
### Motivation:

Those who need 'Origin' or 'Sec-WebSocket-Origin' headers should provide them explicitly, like it is stated in WebSocket specs.

E.g. through custom headers:

    HttpHeaders customHeaders = new DefaultHttpHeaders()
        .add(HttpHeaderNames.ORIGIN, "http://localhost:8080");
    new WebSocketClientProtocolHandler(
        new URI("ws://localhost:1234/test"), WebSocketVersion.V13, subprotocol, 
        allowExtensions, customHeaders, maxFramePayloadLength, handshakeTimeoutMillis)


### Modification:

* Remove enforced origin headers.
* Update tests

### Result:

Fixes #9673: Origin header is always sent from WebSocket client
  • Loading branch information
@ursaj @normanmaurer
ursaj authored and normanmaurer committed Nov 27, 2019
1 parent 6d0c0e9 commit 19a4633
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 53 deletions.
29 changes: 0 additions & 29 deletions ...-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java
View file
Expand Up @@ -48,8 +48,6 @@
*/
public abstract class WebSocketClientHandshaker {

private static final String HTTP_SCHEME_PREFIX = HttpScheme.HTTP + "://";
private static final String HTTPS_SCHEME_PREFIX = HttpScheme.HTTPS + "://";
protected static final int DEFAULT_FORCE_CLOSE_TIMEOUT_MILLIS = 10000;

private final URI uri;
Expand Down Expand Up @@ -601,31 +599,4 @@ static CharSequence websocketHostValue(URI wsURL) {
// See http://tools.ietf.org/html/rfc6454#section-6.2
return NetUtil.toSocketAddressString(host, port);
}

static CharSequence websocketOriginValue(URI wsURL) {
String scheme = wsURL.getScheme();
final String schemePrefix;
int port = wsURL.getPort();
final int defaultPort;
if (WebSocketScheme.WSS.name().contentEquals(scheme)
|| HttpScheme.HTTPS.name().contentEquals(scheme)
|| (scheme == null && port == WebSocketScheme.WSS.port())) {

schemePrefix = HTTPS_SCHEME_PREFIX;
defaultPort = WebSocketScheme.WSS.port();
} else {
schemePrefix = HTTP_SCHEME_PREFIX;
defaultPort = WebSocketScheme.WS.port();
}

// Convert uri-host to lower case (by RFC 6454, chapter 4 "Origin of a URI")
String host = wsURL.getHost().toLowerCase(Locale.US);

if (port != defaultPort && port != -1) {
// if the port is not standard (80/443) its needed to add the port to the header.
// See http://tools.ietf.org/html/rfc6454#section-6.2
return schemePrefix + NetUtil.toSocketAddressString(host, port);
}
return schemePrefix + host;
}
}
4 changes: 0 additions & 4 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java
View file
Expand Up @@ -192,10 +192,6 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY1, key1)
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY2, key2);

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

String expectedSubprotocol = expectedSubprotocol();
if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
Expand Down
5 changes: 0 additions & 5 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java
View file
Expand Up @@ -187,7 +187,6 @@ public WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, S
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Sec-WebSocket-Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 7
* </pre>
Expand Down Expand Up @@ -225,10 +224,6 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
.set(HttpHeaderNames.HOST, websocketHostValue(wsURL));

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

String expectedSubprotocol = expectedSubprotocol();
if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
Expand Down
5 changes: 0 additions & 5 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java
View file
Expand Up @@ -189,7 +189,6 @@ public WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, S
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Sec-WebSocket-Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 8
* </pre>
Expand Down Expand Up @@ -227,10 +226,6 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
.set(HttpHeaderNames.HOST, websocketHostValue(wsURL));

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

String expectedSubprotocol = expectedSubprotocol();
if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
Expand Down
5 changes: 0 additions & 5 deletions ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java
View file
Expand Up @@ -190,7 +190,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 @@ -228,10 +227,6 @@ protected FullHttpRequest newHandshakeRequest() {
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
.set(HttpHeaderNames.HOST, websocketHostValue(wsURL));

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

String expectedSubprotocol = expectedSubprotocol();
if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
Expand Down
1 change: 0 additions & 1 deletion ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java
View file
Expand Up @@ -109,7 +109,6 @@ public WebSocketServerHandshaker07(String webSocketURL, String subprotocols, Web
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Sec-WebSocket-Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 7
* </pre>
Expand Down
1 change: 0 additions & 1 deletion ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java
View file
Expand Up @@ -116,7 +116,6 @@ public WebSocketServerHandshaker08(
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Sec-WebSocket-Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 8
* </pre>
Expand Down
1 change: 0 additions & 1 deletion ...ttp/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java
View file
Expand Up @@ -115,7 +115,6 @@ public WebSocketServerHandshaker13(
* Upgrade: websocket
* Connection: Upgrade
* Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
* Origin: http://example.com
* Sec-WebSocket-Protocol: chat, superchat
* Sec-WebSocket-Version: 13
* </pre>
Expand Down
37 changes: 36 additions & 1 deletion ...p/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java
View file
Expand Up @@ -31,19 +31,27 @@
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.HttpScheme;
import io.netty.util.CharsetUtil;
import io.netty.util.NetUtil;
import org.junit.Test;

import java.net.URI;
import java.util.Locale;

import static org.junit.Assert.*;

public abstract class WebSocketClientHandshakerTest {
private static final String HTTP_SCHEME_PREFIX = HttpScheme.HTTP + "://";
private static final String HTTPS_SCHEME_PREFIX = HttpScheme.HTTPS + "://";

protected abstract WebSocketClientHandshaker newHandshaker(URI uri, String subprotocol, HttpHeaders headers,
boolean absoluteUpgradeUrl);

protected WebSocketClientHandshaker newHandshaker(URI uri) {
return newHandshaker(uri, null, null, false);
HttpHeaders headers = new DefaultHttpHeaders()
.add(getOriginHeaderName(), websocketOriginValue(uri));
return newHandshaker(uri, null, headers, false);
}

protected abstract CharSequence getOriginHeaderName();
Expand Down Expand Up @@ -360,4 +368,31 @@ public void testDuplicateWebsocketHandshakeHeaders() {

request.release();
}

static CharSequence websocketOriginValue(URI wsURL) {
String scheme = wsURL.getScheme();
final String schemePrefix;
int port = wsURL.getPort();
final int defaultPort;
if (WebSocketScheme.WSS.name().contentEquals(scheme)
|| HttpScheme.HTTPS.name().contentEquals(scheme)
|| (scheme == null && port == WebSocketScheme.WSS.port())) {

schemePrefix = HTTPS_SCHEME_PREFIX;
defaultPort = WebSocketScheme.WSS.port();
} else {
schemePrefix = HTTP_SCHEME_PREFIX;
defaultPort = WebSocketScheme.WS.port();
}

// Convert uri-host to lower case (by RFC 6454, chapter 4 "Origin of a URI")
String host = wsURL.getHost().toLowerCase(Locale.US);

if (port != defaultPort && port != -1) {
// if the port is not standard (80/443) its needed to add the port to the header.
// See http://tools.ietf.org/html/rfc6454#section-6.2
return schemePrefix + NetUtil.toSocketAddressString(host, port);
}
return schemePrefix + host;
}
}
2 changes: 1 addition & 1 deletion .../src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java
View file
Expand Up @@ -115,7 +115,7 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Except
assertTrue(serverReceivedHandshake);
assertNotNull(serverHandshakeComplete);
assertEquals("/test", serverHandshakeComplete.requestUri());
assertEquals(8, serverHandshakeComplete.requestHeaders().size());
assertEquals(7, serverHandshakeComplete.requestHeaders().size());
assertEquals("test-proto-2", serverHandshakeComplete.selectedSubprotocol());

// Transfer the handshake response and the websocket message to the client
Expand Down

0 comments on commit 19a4633

Please sign in to comment.