Improve Origin header handing for WebSockets

[AchimKlotz]

In the NettyRequestFactory the origin header is calculated this way:
.set(ORIGIN, "http://" + uri.getHost() + ":" + uri.getExplicitPort())//

this is wrong for secure wss websocket request, if you for example have an request to
wss://yourdomain.com the origin header will be http://yourdomain.com:443

The correct header should be calculated like this:
String origin = (uri.getScheme().equals("wss") ? "https" : "http" ) +"://"+uri.getHost() +(uri.getExplicitPort() != 80 && uri.getExplicitPort() != 443 ? uri.getExplicitPort() : "");

[slandelle]

I've just check with Chrome on http://websocket.org/ and Origin header is really http://websocket.org when connection over wss.

[AchimKlotz]

but not if you call https://websocket.org/echo.html
then the origin header is https://websocket.org.
I've got the problem that a server is only reachable by https and wss and the websocket connection is rejected if the origin header is not set correctly. With Chrome this is working fine on that server
Please see also a related netty issue which has been fixed about a year ago: netty/netty#5402

[slandelle]

Oh my, you're right.
So browsers use the web page for Origin o_0.

But then, as you can see, your suggestion doesn't work either: you jump can't build the Origin header from the ws(s) url as it could be a different domain (here, WebSocket domain is "echo.websocket.org" while Origin header targets "websocket.org").

The best solution would be to honor existing "Origin" header.
Then, if it's undefined, it might indeed be a good idea to use https for wss.

WDYT?

[AchimKlotz]

I think that's a sensible solution: If the "Origin" header is passed use that one, otherwise construct it like in my suggested solution.
According to the specification https://tools.ietf.org/html/rfc6454#section-6.1 the port should be omitted if it is the standard port for the protocol:
5. If the port part of the origin triple is different from the default port for the protocol given by the scheme part of the origin triple:

[slandelle]

According to the specification https://tools.ietf.org/html/rfc6454#section-6.1 the port should be omitted if it is the standard port for the protocol

Yes, I noticed that too but didn't have time yesterday for logging the issue. This is a separate issue.

Would you mind providing your PR, please?

Also, you should be using Uri.isSecured instead of testing the scheme manually.

[AchimKlotz]

String origin =request.getHeaders().get(ORIGIN) != null ? request.getHeaders().get(ORIGIN) : (uri.isSecured() ? "https" : "http" ) +"://"+uri.getHost() + ((uri.getExplicitPort() != 80 && !uri.isSecured()) || (uri.getExplicitPort() != 443 && uri.isSecured()) ? uri.getExplicitPort() : "");
also attached as git diff to version 2.0.33

header.txt

[slandelle]

Never used Git, huh? ;-)