websocket icon indicating copy to clipboard operation
websocket copied to clipboard

Add optional method ProxyTLSConnection (closes #779)

Open sleeyax opened this issue 3 years ago • 4 comments

Previously, it was impossible to specify both Proxy and NetDialTLSContext on the websocket Dialer without experiencing connection issues. This commit brings a change to the proxy CONNECT flow so that the initial connection is always a normal proxy CONNECT over TCP, while allowing TLS customizations on the existing connection at a later point in time via the new ProxyTLSConnection method.

Fixes #779

Summary of Changes

Please see my comment on the relevant issue for a detailed explanation and my thought process: https://github.com/gorilla/websocket/issues/779#issuecomment-1125185896

Tests are not included, I'd appreciate some help or guidance on that front because I'm unsure what to test, thanks!

sleeyax avatar May 12 '22 16:05 sleeyax

Thanks for the commit. I know it's been a long time but it would be good to see a test to go with this PR. I'm not fully confident in merging without associated tests.

jaitaiwan avatar Jun 19 '24 07:06 jaitaiwan

It also looks like this PR does something similar: https://github.com/gorilla/websocket/pull/740/files maybe consider using that PR to bolster your own.

jaitaiwan avatar Jun 19 '24 07:06 jaitaiwan

It also looks like this PR does something similar: https://github.com/gorilla/websocket/pull/740/files maybe consider using that PR to bolster your own.

Not sure if this fixes the issue completely. The PR you mention adds support for HTTPS proxies whereas this PR fixes a bug where NetDialTLSContext and Proxy fields on the websocket.Dialer don't work together. See the referenced issues for more details.

sleeyax avatar Jun 19 '24 09:06 sleeyax

Thanks for the commit. I know it's been a long time but it would be good to see a test to go with this PR. I'm not fully confident in merging without associated tests.

It's been so long. Can't say when/if I'll be able to find the time to look into this again and add tests.

sleeyax avatar Jun 19 '24 09:06 sleeyax