_If you are filing a bug report, please answer these 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!_
caddy -version)?Untracted Dev build, latest basically.
I'm trying to reverse proxy a websocket connection.
my.domain {
proxy / localhost:8080 {
proxy_header Host {host}
proxy_header X-Real-IP {remote}
proxy_header X-Forwarded-For {remote}
websocket
}
}
Just caddy, the execution environment is Windows Operating System 2008 R2
I expected to see a normal websocket connection like I do without Caddy.
read: websocket: close 1006 (abnormal close): unexpected EOF
recv: 313330313023e3933313031
Here's the backend websocket server: https://gist.github.com/kryptodev/2ee48eb9b4c68cf9722d182ec4aba7b3
Please note, the EOF error only occurs with Caddy in between. So far I have tried with TLS off, TLS on, HTTP2 off and also on port 80. Everywhere it's the same scenario. Makes me wonder if this could be windows related?
Thank you.
Thanks for the report. Can you run ./ecaddy -version after building it with build.bash? That will give important information in case we don't solve this right away.
Caddy 0.8.2 (+b149a86 Wed Apr 13 00:52:31 UTC 2016)
1 file changed, 7 insertions(+), 44 deletions(-)
caddy/directives.go
Just realip added via caddyext
I wasn't able to reproduce, using a modified version of @kryptodev's gist that adds an index.html test client and a static dir handler to serve it: https://gist.github.com/edrex/105a4c9577aa13c7c2f52048be3295ce
upgrade:websocket: origin not allowed
It works for me through a proxy if I instantiate the Upgrader with a permissive checkOrigin:
var upgrader = websocket.Upgrader{
CheckOrigin: func(r *http.Request) bool { return true },
} // use default options
@kryptodev, what client are you using to test this? I can't reproduce using the client setup in my version of your gist.
Might also be a windows-only thing. I'm testing on a OS X.
I have a similar issue. I try to proxy a codebox instance.
Codebox is written in nodejs and serves websocket connections under a dynamic URL: /socket.io/1/?t=SECONDS_SINCE_EPOCH.
I use caddy 0.8.2, downloaded the binary.
My Caddyfile is:
codebox.domain.com
proxy /socket.io/ 172.17.0.2:8000/ {
proxy_header Host {host}
proxy_header X-Real-IP {remote}
proxy_header X-Forwarded-Proto {scheme}
websocket
}
proxy / 172.17.0.2:8000 {
proxy_header Host {host}
proxy_header X-Real-IP {remote}
proxy_header X-Forwarded-Proto {scheme}
}
errors stdout
log stdout
The error that codebox reports is warn: client not handshaken client should reconnect.
I've also tried with an in-between proxy written in golang ( caddy <-> golang proxy <-> codebox).
The proxy reports as error read tcp 127.0.0.1:8080->127.0.0.1:47180: use of closed network connection.
The error happens during a bufio.ReaderWriter read from the proxy to client (caddy), so I think it is the client (caddy) who closed the connection.
I tried with TLS and http2 off, but nothing changed. I'll investigate a bit more.
@andmarios that's probably because of https://github.com/mholt/caddy/issues/763.
@edrex indeed!
If I understand correctly, socket.io (a websocket library) negotiates a websocket and then passes a unique, one-time-use URL to the client.
Caddy's proxy initiates a connection (http request) by itself on that URL to test if it accepts websockets and then starts a second connection (net.dial) to serve the client.
But socket.io has already invalidated the URL, since it got one connection on that.
A potential fix would be to trust the client and treat the connection as a websocket if the client requested it. This way the proxy wouldn't need special websocket directives. But in the case where the server denied a websocket connection, there would be problems.
Ideally we would be able to hijack the caddy <-> backend connection, but that doesn't seem possible: https://github.com/golang/go/issues/8285
@andmarios something very similar happens with when proxying to net-browserify. Good summary.
We could indeed skip the handshake with upstream if it looks like the client is requesting an upgrade.
I believe it's possible to do the full handshake conditionally though, just not via the http.RoundTripper interface. See WebsocketProxy.ServeHTTP.
I was attempting this last night:
p.Transport (a http.RoundTripper), instead defer to #WebsocketProxy.NewProxy().ServeHTTP()WebsocketProxy doesn't handle TLS for the upstream connection though, so if we get it working we'd either have to add the support upstream or inline the parts we need.
I could push up my work in progress if anyone wants to review and try to understand why it isn't passing. I'm not sure when I'll have time to look at it again.
What if the user gave the exact path for websocket connection, otherwise it's treated as a regular reverse proxy? In other words, the path must match exactly, and if so, we will attempt a websocket connection without checking the Upgrade header.
Still, though, what's the point of sending the Upgrade header if backend services don't support it? (Or is it that they do, they just require that the same TCP connection be used?)
@edrex I would certainly like to give it a try. I have a "works for me" patch that I can't submit because it has a much different philosophy and potential problems. It checks if the connection and upgrade request headers are set for websocket, and in that case it skips the transport and hijacks the connection.
@mholt An exact prefix would be better, since the websocket URLs may be dynamically generated. But the websocket RFC states that unless the server aknowledges the connection upgrade, http semantics remain. Thus an edge case issue would still remain.
@andmarios Interesting, your "works for me" patch sounds similar to what I just mentioned, where the proxy's base path is the exact and full path to the websocket; because if we knew for sure the request was going to be a websocket, we could skip the roundtrip in the first place.
But I agree it's not ideal. And I kind of would rather find a proper way to do this by reusing the connection.
Could we write our own transport from which we can obtain the connection? Maybe embed http.DefaultTransport or something?
(Either way, +1 on you trying -- go for it!)
I'm of two minds re: explicit websockets vs "just works always". I think making it "just work", if it's possible, is more in line with the Caddy philosophy.
@mholt I like your idea about changing the type of p.Transport from http.RoundTripper to something that lets us get the connection, and from which we can get an http.RoundTripper. Seems like a straightforward refactoring which would make a clean solution using p.Transport possible.
I'd also remove the proxy { websocket } directive param, unless folks want to keep the ability to leave this handling off at runtime.
I'll try these things and push up soon. If anybody is itching to make this happen I can push my WIP and let you have at it.
Please go for it, a pull request to review is progress and learning for the rest of us too :smile:
Today I'm working on $DayJob but tomorrow I'll be back to getting Caddy 0.8.3 ready to release, which hopefully will include a fix for this too! Let me know if you have any questions, we're also in gitter.
@edrex are you still working on this issue?
This issue has prevented me from using caddy, I have to fall back to nginx since no walkaround found
I have the same with the socket.io
@cgcgbcbc sorry, was away from internet for several weeks. I'm not currently working on this, see https://github.com/mholt/caddy/pull/764
Is the issue here with socket.io's implementation of websockets that necessitates a single connection to the backend? Or are there other implementations that break as well?
As far as I understand it, the relatively simple solution would be to just provide our own Dial function to Transport that would let us keep a reference to the connection. I could test something, but I'm not 100% sure on how to reproduce the issue discussed in this thread.
@nemothekid my WIP in https://github.com/mholt/caddy/pull/764 includes a failing test. The first two commits would be a good starting point, if you blow away the last one.
@edrex Can you test out #987 in a "real world" test?
@nemothekid I'm able to connect to a socket.io server through caddy with your patch. Nice!
This issue should be fixed with the inclusion #987.
Most helpful comment
Please go for it, a pull request to review is progress and learning for the rest of us too :smile:
Today I'm working on $DayJob but tomorrow I'll be back to getting Caddy 0.8.3 ready to release, which hopefully will include a fix for this too! Let me know if you have any questions, we're also in gitter.