-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Ensured Netty provider uses CONNECT when proxying ws #657
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.
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
Ensured Netty provider uses CONNECT when proxying ws #657
Conversation
The WS standard requires that clients use the CONNECT method when proxying WebSockets, even if they aren't using SSL. The Grizzly provider already supported this, but the Netty didn't. I also implemented a test that tested this case.
This needed to be done both for the Grizzly and the Netty provider. Since this is essentially the same configuration parameter as the useRelativeURIsWithSSLProxies, I reused that parameter, but I renamed it to useRelativeURIsWithConnectProxies to reflect its true meaning since it's no longer used just for SSL, and I deprecated the old parameter.
I've attached a second commit to this pull request, it ensures that when proxying websockets using the CONNECT method, a relative path is used. This slightly changed the meaning of the Let me know if you'd prefer these two commits to be squashed. |
@jroper Thanks! Could you squash the commits please? I'll then port on 1.9 and master. |
Ensured Netty provider uses CONNECT when proxying ws
I guess you don't want me to squash the commits anymore? |
Nope, thanks :) I thought you were offline at this time and I wanted to port this ASAP on the other branches. |
@jroper I guess you need me to release 1.8.13? |
No rush, we fixed this for a customer but that customer can use their own builds until you release it. |
That customer is very happy. Tested in a corporate environment and ws/wss worked for me. Thanks! |
Thanks for your feedback, and thanks again to @jroper for the fix! |
* Fix HTTP2StreamChannel leak * Update code comments.
The WS standard requires that clients use the CONNECT method when proxying WebSockets, even if they aren't using SSL. The Grizzly provider already supported this, but the Netty didn't.
I also implemented a test that tested this case.
I've made this change and PR against the 1.8.x branch, master is significantly different but briefly reading the code it looks like the bug is there as well. If you'd prefer it against master first, and then backported, or want me to submit another PR against master/1.9.x, let me know.