Skip to content

Conversation

@dmwilcox
Copy link

@dmwilcox dmwilcox commented Nov 8, 2018

Per my comment on #101

Still having mild issues with the configuration file option doing what is expected. Let me know if I should move the lists of headers in reqs.h, how to not warn on my bad pointer casting at compile time, or any other style or bad things I'm doing.

Thank you for working on such an awesomely readable little proxy! I really enjoyed the process of re-visiting C.

@rofl0r
Copy link
Contributor

rofl0r commented Nov 8, 2018

your commit doesn't even compile. maybe you can start with explaining what your plan is ?

@dmwilcox
Copy link
Author

Sorry about that, I had a missing comma it should compile now.

The idea behind the PR is issue #101 -- which from my testing is really just some websocket libraries not using CONNECT to setup the connection. Instead they assume an old HTTP/1.0 style proxy where setting the 'Host' header is all that's needed to setup the connection.

Tinyproxy, as you know, when not using connect assumes HTTP/1.0 and adds a Connection: close header which makes websocket libraries with this behavior not function.

I added an config file option to Tinyproxy to support this antique proxy behavior and called it "AllowUpgrade" -- better name suggestions welcome.

I just tested it and it works as expected for me -- I can pretend to be a bad websocket client and just make a request (no CONNECT) and get a status code 101 in response.

I've been using curl (without -p which turns on the connect method) to simulate setting up a websocket and monitoring what hits the wire with wireshark. And I can drive it via netcat and get a proper upgrade now.

Running tinyproxy on 10.137.0.14 port 3128 with the new build

curl -vv --proxy http://10.137.0.14:3128 --http1.1 -i -N -H "Connection: Upgrade" -H "Upgrade: websocket" -H "Host: echo.websocket.org" -H "Origin: http://www.websocket.org" -H "Sec-WebSocket-Key: u0ofNYzJQpbLbmDvezinyy==" -H "Sec-WebSocket-Version: 13"  http://echo.websocket.org
$ nc 10.137.0.14 3128
GET / HTTP/1.1
Host: echo.websocket.org
Connection: Upgrade
Upgrade: websocket
Origin: http://www.websocket.org
Sec-WebSocket-Key: u0ofNYzJQpbLbmDvezinyy==
Sec-WebSocket-Version: 13

HTTP/1.0 101 Web Socket Protocol Handshake
Via: 1.1 tinyproxy (tinyproxy/1.10.0-git-3-gf44d0f3)
Sec-WebSocket-Accept: 7jOEwIWrg7+8UHkMd+c7vitIQyc=
Server: Kaazing Gateway
Access-Control-Allow-Origin: http://www.websocket.org
Access-Control-Allow-Credentials: true
Access-Control-Allow-Headers: content-type
Access-Control-Allow-Headers: authorization
Access-Control-Allow-Headers: x-websocket-extensions
Access-Control-Allow-Headers: x-websocket-version
Access-Control-Allow-Headers: x-websocket-protocol
Date: Tue, 20 Nov 2018 07:05:22 GMT
Upgrade: websocket
Connection: Upgrade

Couple of things I'd like some pointers on:

  • I don't know the best way to handle getting rid of the incompatible pointer warnings -- to switch the lists of headers to strip based on my new config option
  • and whether it makes sense to allow non-Connect connections to egress as HTTP/1.1 rather than 1.0 when hitting the reqs.c:308 branch

Thanks!

@rofl0r
Copy link
Contributor

rofl0r commented Nov 20, 2018

sorry, i don't think it's a good idea to add a new config option for something that should work out of the box, and especially an option that requires a warning "only use if you know what you're doing". therefore i prepared a PR which should the solve the issue cleanly. please check if my pr #211 works for you, thanks!

@rofl0r rofl0r closed this Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants