Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WebSocketClientHandshaker java.lang.NullPointerException #12933

Closed
heixiaoma opened this issue Oct 26, 2022 · 12 comments
Closed

WebSocketClientHandshaker java.lang.NullPointerException #12933

heixiaoma opened this issue Oct 26, 2022 · 12 comments
Milestone
4.1.86.Final

Comments

@heixiaoma
Copy link

WebSocketClientHandshaker
1666764902075
@heixiaoma
Copy link
Author

Missing: header-> origin

@hyperxpro
Copy link
Contributor

Please explain in more detail.

@heixiaoma
Copy link
Author

When the websocketclient connection request header is missing the origin, a null pointer problem will occur

@chrisvest
Copy link
Contributor

@amizurov What's your take? Do we need validation to check for the origin header somewhere?

@heixiaoma
Copy link
Author

Now he throws a null pointer. I think we can give some warning logs

@amizurov
Copy link
Sponsor Contributor

Hi @heixiaoma , @chrisvest. Let me take a look.

@amizurov
Copy link
Sponsor Contributor

@heixiaoma Could you please clarify, did you set webSocketURL -> "/ws" via constructor and customHeaders which contains Host but does not contain Origin header ?

@heixiaoma
Copy link
Author

@heixiaoma 您能否澄清一下，您是否webSocketURL通过构造函数设置了 -> "/ws" 并且customHeaders包含Host但不包含Originheader ？

yes

@amizurov amizurov mentioned this issue Oct 31, 2022
Merged
@amizurov
Copy link
Sponsor Contributor

amizurov commented Nov 1, 2022 •
edited

So the main problem here, that if we pass the webSocketURL without host (e.g. '/ws') we faced with NPE even when we building the Host header value. But if we pass the Host header value via customHeaders everything should works fine, but now we got the NPE on the building Origin header value (see #9673).

I created the PR which should fix the Origin header behaviour (#12941). Also i think we can add additional check that if webSocketURL does not contain host and Host header does not present in custom headers we throw an invalid arg exception. @chrisvest WDYT ?

@chrisvest
Copy link
Contributor

chrisvest commented Nov 1, 2022 •
edited

@amizurov Sounds good. Do you want to add that check to #12941, or in a different PR?

@amizurov
Copy link
Sponsor Contributor

amizurov commented Nov 2, 2022

@amizurov Sounds good. Do you want to add that check to #12941, or in a different PR?

Added to #12941

normanmaurer added a commit that referenced this issue Nov 10, 2022
@amizurov @normanmaurer
Fix generating the Origin header value for websocket handshake requ…
ae9b823 
…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>
@normanmaurer normanmaurer added this to the 4.1.86.Final milestone Nov 10, 2022
normanmaurer added a commit that referenced this issue Nov 10, 2022
@amizurov @normanmaurer
Fix generating the Origin header value for websocket handshake requ…
f3c27ae 
…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>
@chrisvest
Copy link
Contributor

I think this was fixed by #12941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.1.86.Final
Development

No branches or pull requests
5 participants
@chrisvest @normanmaurer @amizurov @heixiaoma @hyperxpro