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.

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

Caddy doesn't pass the "Connection" header to websockets? #886

Closed
uhthomas opened this issue Jun 16, 2016 · 19 comments
Closed

Caddy doesn't pass the "Connection" header to websockets? #886

uhthomas opened this issue Jun 16, 2016 · 19 comments
Labels
bug 🐞 Something isn't working

Comments

@uhthomas
Copy link

(Are you asking for help with using Caddy? Please use our forum instead: https://forum.caddyserver.com. If you are filing a bug report, please answer the following questions. If your issue is not a bug report, you do not need to use this template. Either way, please consider donating if we've helped you. Thanks!)

1. What version of Caddy are you running (caddy -version)?

Caddy (untracked dev build) (master)

2. What are you trying to do?

Proxy to a websocket

3. What is your entire Caddyfile?

stream.6f.io {
  gzip {
    not /stream
  }
  proxy / 127.0.0.1:1339 {
    websocket
    max_fails 0
  }
}

4. How did you run Caddy (give the full command and describe the execution environment)?

./caddy

5. What did you expect to see?

Websockets being proxied correctly.

6. What did you see instead (give full error messages and/or log)?

The backend which is being proxied doesn't receive the header and therefore spits out

websocket: could not find connection header with token 'upgrade'

7. How can someone who is starting from scratch reproduce this behavior as minimally as possible?

Read the "Connection" header from a proxied backend, ensuring the request was a valid websocket one.

@mholt
Copy link
Member

mholt commented Jun 20, 2016

Thanks for the report. Hmm, I'm not able to reproduce this using a simple echo server backend (the same example in the godoc) with the added line of fmt.Println("Connection", ws.Request().Header.Get("Connection")) in the handler. I also tried a variety of different elements (wss vs. ws, http vs. https, etc) to try to reproduce the behavior.

Try pulling the latest and repeat the experiment maybe? As of right now though, the tests--which do cover websockets--are passing. If you are able to reproduce it I'll need more info. And please use build.bash to build Caddy with the proper version information.

Thank you!

@mholt mholt closed this as completed Jun 20, 2016
@uhthomas
Copy link
Author

Proper version information built with build.bash:

$ ./caddy -version Caddy 0.9-beta.2 (+15fa5cf Sun Jun 26 20:50:56 UTC 2016)

Same issue as before, websockets on proxies don't work because the Connection and Upgrade header are removed from the request.

@mholt mholt reopened this Jun 26, 2016
@mholt
Copy link
Member

mholt commented Jun 29, 2016

@6f7262 Thanks for that; I need more information though.

  • What is the full URL are you connecting to when you make the websocket connection?
  • What is your client? A browser? A custom program? Can you share the relevant client connect code?
  • How do you know the Connection header is being dropped?

So far on my end everything looks peachy... here I assume you are connecting to /stream:

GET /stream HTTP/1.1
Host: 127.0.0.1:1339
Accept-Encoding: gzip, deflate, sdch, br
Accept-Language: en-US,en;q=0.8
Cache-Control: no-cache
Connection: Upgrade
Dnt: 1
Origin: https://matt.dev:2015
Pragma: no-cache
Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits
Sec-Websocket-Key: X/c3mZJt/PZtzULXs7R7Ow==
Sec-Websocket-Version: 13
Upgrade: websocket
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36
X-Forwarded-For: 127.0.0.1

@mholt
Copy link
Member

mholt commented Jul 3, 2016

@6f7262 sent me this screenshot in Slack:

6f-upgrade-header

Could you try it without gzip, 6f? I am suspicious of it for another reason (maybe the underlying bug, maybe not) -- would be curious if you have better results without the gzip directive.

Meanwhile I'll keep trying to reproduce the bug.

@bpetri
Copy link

bpetri commented Jul 21, 2016

I do see something similar here:

While the Request seems to be okay:
screen shot 2016-07-21 at 17 05 04

I got the following when capturing the traffic between caddy and my API via tcpdump

GET /chsk?client-id=1cbcdc36-a410-4296-b292-c380eb00f9de HTTP/1.1
Host: caddy:8000
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.86 Safari/537.36
Accept-Encoding: gzip, deflate, sdch
Accept-Language: en-US,en;q=0.8
Cache-Control: no-cache
Connection:
Origin: http://caddy:8000
Pragma: no-cache
Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits
Sec-Websocket-Key: Js3agC5tc9eFC8JikdZVRA==
Sec-Websocket-Version: 13
Upgrade:
X-Forwarded-For: 172.19.0.4

1. What version of Caddy are you running (caddy -version)?
Caddy 0.9.0

2. What are you trying to do?
Proxy to a websocket

3. What is your entire Caddyfile?

0.0.0.0:8000 {
  root public/
  proxy /chsk frontend:3000 {
    websocket
    max_fails 0
  }

  ext .html 

  log log/caddy/access.log {
    size 50                       # Rotate after 50 MB
    age  30                       # Keep rotated files for 30 days
    keep 5                        # Keep at most 5 log files
  }
  errors {
    log log/caddy/error.log {
      size 50                     # Rotate after 50 MB
        age  30                     # Keep rotated files for 30 days
        keep 5                      # Keep at most 5 log files
    }
    404 404.html # Not Found
    500 500.html # Internal Server Error
  }
}

4. How did you run Caddy (give the full command and describe the execution environment)?
./caddy -conf Caddyfile.test

5. What did you expect to see?
Websockets being proxied correctly.

6. What did you see instead (give full error messages and/or log)?
Websocket is not initated.

7. How can someone who is starting from scratch reproduce this behavior as minimally as possible?
I could not reproduce this when running locally on my OSX machine. I see this when using different docker images, whereby one is running caddy and another one is running my frontend API. I'll check whether I can create a more simple test case.

@mholt
Copy link
Member

mholt commented Jul 22, 2016

@6f7262 I wonder, could you try the latest on master? I'm curious if a change for #916 affected this behavior at all.

@bpetri
Copy link

bpetri commented Jul 22, 2016

fyi: I can confirm that my setup works fine with both, 0.8.2 and 0.8.3.

@schollz
Copy link

schollz commented Jul 22, 2016

Seems it might be a similar issue then.

I tried the latest master (commit 6490ff6) and I'm getting the same behaviour as reported in #953.

@mholt
Copy link
Member

mholt commented Jul 22, 2016

I'm still unable to reproduce this; everything works fine for me on 0.9.

@evermax
Copy link

evermax commented Jul 26, 2016

Hi, I created docker containers to reproduce the issue: https://github.com/evermax/caddy-websocket-test

But it works for me. Can you give me some feedback as to how to get it closer to your own configuration?

@schollz
Copy link

schollz commented Jul 29, 2016

@evermax Thanks. Your test works for me on both 0.8.3 and 0.9.

I followed your example to make a self contained test (though not as elegant). I went and forked Gorilla's websocket chat example which is the essence of what I'm using in my code. This example works fine for me on 0.8.3, but again it does not work for 0.9 (or current master). Gorilla gives me an error: websocket: could not find connection header with token 'upgrade'.

If you'd like to try, you can try this: https://github.com/schollz/websocket-test. The Caddyfile I used is in there so you just need to do go run *.go and startup a version of caddy and open a browser to 127.0.0.1.

@uhthomas
Copy link
Author

Master branch seems to work fine for me header-wise also. Websockets are slow to connect but that's another issue entirely.

@mholt
Copy link
Member

mholt commented Jul 29, 2016

Okay, so @6f7262 , your original issue is resolved? Now we just have @schollz's lingering issue it seems?

@schollz
Copy link

schollz commented Jul 29, 2016

Sorry, my issue lingers. Just tried on Windows + Firefox/Chrome and still 0.8.3 works while 0.9 does not. Am I doing something wrong? Or am I just cursed? Feel free to close the issue if I'm the only one having problems.

@tw4452852
Copy link
Collaborator

Hi @schollz , your problem is fixed in the latest build with this commit 6490ff6
Just to confirm, I tried your example with the 7157bdc, it works well,
but it doesn't work with v0.9.0 and the issue is just what you described.

@mholt mholt added the bug 🐞 Something isn't working label Jul 30, 2016
@mholt
Copy link
Member

mholt commented Jul 30, 2016

This is really great news, thanks @tw4452852 for taking the time to confirm! @evermax and @schollz I will close this now, but if there's anything to add feel free to do so.

@mholt mholt closed this as completed Jul 30, 2016
@schollz
Copy link

schollz commented Jul 30, 2016

Brilliant! It works great now. Thanks @tw4452852, @mholt.

@slava-vishnyakov
Copy link

@mholt Would it be possible to make 0.9.1 with this fix? Thanks for Caddy, it's awesome!

@mholt
Copy link
Member

mholt commented Aug 2, 2016

Yes, 0.9.1 will include this fix. And thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants