websockets icon indicating copy to clipboard operation
websockets copied to clipboard

Add support for HTTP(S) proxies to connect()

Open aaugustin opened this issue 6 years ago • 15 comments

Fix #364.

aaugustin avatar Jun 02 '18 15:06 aaugustin

Codecov Report

Merging #422 into master will decrease coverage by 1.35%. The diff coverage is 56.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
- Coverage     100%   98.64%   -1.36%     
==========================================
  Files          29       29              
  Lines        3170     3252      +82     
  Branches      333      352      +19     
==========================================
+ Hits         3170     3208      +38     
- Misses          0       37      +37     
- Partials        0        7       +7
Impacted Files Coverage Δ
websockets/test_http.py 100% <ø> (ø) :arrow_up:
websockets/http.py 100% <100%> (ø) :arrow_up:
websockets/test_uri.py 100% <100%> (ø) :arrow_up:
websockets/uri.py 100% <100%> (ø) :arrow_up:
websockets/client.py 77.43% <33.33%> (-22.57%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 68371dd...c9883d4. Read the comment docs.

codecov[bot] avatar Jun 02 '18 15:06 codecov[bot]

Most likely this doesn't close one of the two transports when the connection terminates.

aaugustin avatar Jun 02 '18 16:06 aaugustin

So any updates?

lambdalisue avatar Aug 02 '18 10:08 lambdalisue

Nothing more than the public comments here.

aaugustin avatar Aug 02 '18 11:08 aaugustin

I'm really looking forward to this 👍

lambdalisue avatar May 30 '19 03:05 lambdalisue

@aaugustin could you please update this, so that I can still use it with the latest version?

kb-1000 avatar May 31 '19 04:05 kb-1000

Feel free to do the rebase yourself and post a link to it here, if you're really in a hurry that's probably the best course of action. Others may pitch in by testing said new branch and giving feedback on it as well. Cheers

RemiCardona avatar May 31 '19 05:05 RemiCardona

Well, going from this PoC to a properly tested implementation that supports all cases (HTTP/HTTPS => HTTP/HTTPS) is rather boring. I haven't been motivated to do it.

Besides writing tests:

  • I'm not confident that connection termination works well.
  • I'd like to revisit the part after # Wrap socket with TLS. This ugly hack ... Perhaps there's a better solution now that websockets requires Python > 3.6.

Proxies are most common in corporate networks. If you work at a company that relies on websockets and needs this feature, please consider paying for its development.

Get in touch at aymeric.augustin at fractalideas.com if you're interested.

Before downvotes pour in, I'd like to say that paying people is a time-proven way to get them to do things they wouldn't do by themselves otherwise ;-)

aaugustin avatar May 31 '19 06:05 aaugustin

So what should we do for this feature (not PR) to be merged? Shall we continue this PR (I'm sorry but I haven't tested this yet) or start from a scratch? I'm not really familiar so I'm not sure if I can help but at least I can try.

Or buy enough ☕️ for you and wait? If it works, I think it's a fair request 👍 (but then how much? :grin:)

lambdalisue avatar Jun 04 '19 15:06 lambdalisue

Python 3.7 added start_tls, which removes the need for the ugly hack.

If we use it, proxying to a WSS endpoint will require Python ≥ 3.7. I think that's fine.

aaugustin avatar Jun 04 '19 15:06 aaugustin

Agree. Less code to maintain. :slightly_smiling_face:

lgrahl avatar Jun 05 '19 07:06 lgrahl

Hi guys... I had to make proxies work with websockets, where the proxy server required NTLM authentication (just to make things interesting)... My fork is here with explanatory notes added to README: https://github.com/mdymike/websockets

Hopefully someone finds that useful. I don't feel it's polished enough yet to do a pull request so just putting it out there 'as-is' for now :)

Best, Mike

mdymike avatar Nov 12 '19 19:11 mdymike

@aaugustin, hi, I don’t want to bug you with this too much, but now that start_tls is available and Python 3.8 is already around, maybe you can find some time to have a look at this again?

Thanks in advance, and I hope you stay safe in this pandemic.

andrewshadura avatar Mar 25 '20 19:03 andrewshadura

Hi guys... I had to make proxies work with websockets, where the proxy server required NTLM authentication (just to make things interesting)... My fork is here with explanatory notes added to README: https://github.com/mdymike/websockets

Hopefully someone finds that useful. I don't feel it's polished enough yet to do a pull request so just putting it out there 'as-is' for now :)

Best, Mike

I tested the fork by Mike and can confirm that it works behind a corporate proxy 😄 👍 (I did not test the NTLM authentication though)

It would be nice to see proxy support in the official websockets library.

Loffe avatar Apr 21 '20 07:04 Loffe

I have forward-ported this pull request to the current websockets code using start_tls; I will submit a pull request superseding this one soon.

andrewshadura avatar Apr 21 '20 07:04 andrewshadura