On 07/12/2017 07:25 PM, Jacob Champion wrote: > On 07/11/2017 05:36 AM, Yann Ylavic wrote: >> I think it's quite hazardous to use/allow ANY and would prefer the >> upgrade_method (worker->s->upgrade) to be a list of acceptable protocols. > > I think both ANY *and* NONE are dangerous. Both of them turn > proxy_wstunnel into a generic TCP forwarder (and NONE does so without > any opt-in on the client's part).
So how I have the following to proxy: <request> GET /jboss-websocket-hello/websocket/helloName HTTP/1.1 Host: localhost:8080 User-Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 Accept-Language: en-US,en;q=0.7,ca;q=0.3 Accept-Encoding: gzip, deflate Sec-WebSocket-Version: 13 Origin: http://localhost:8080 Sec-WebSocket-Extensions: permessage-deflate Sec-WebSocket-Key: CMVwRmu3A0Ozj0og8cnrlA== Connection: keep-alive, Upgrade Pragma: no-cache Cache-Control: no-cache Upgrade: websocket </request> <response> HTTP/1.1 101 Upgrade: websocket Connection: upgrade Sec-WebSocket-Accept: p4OcxSGQGdGqMJi7cxMnp8Sjrxc= Sec-WebSocket-Extensions: permessage-deflate Date: Thu, 13 Jul 2017 12:47:45 GMT </response> ...,..9e...,.$.H...W(I-.QH+..U(OM*.O.N-QH.K)...+.. Tomcat web socket stuff... So yes the HTTP/1.1 really needs to upgrade and NONE is just a work-around. > >> The admin surely knows which protocol(s) the backend supports, the >> issue being that otherwise most backends will ignore the Upgrade and >> hence the connection will continue in normal HTTP (tunneled w/o any >> protocol checking). > > +1. Even once we implement the protocol list, we should still > double-check that the protocol is actually upgraded before we start > forwarding back and forth. Actually the tunnel allows nearly everything. Cheers Jean-Frederic > >> IMO the Upgrade handling should be part of mod_proxy_http (not >> _wstunnel) and depend on whether or not the backend accepted it. > > This I don't necessarily agree with as much... for now, Upgrade handling > belongs where it's needed, and if there are duplicate pieces of code, we > probably need to pull them into the core, not a different proxy module. > >> It was already discussed in [1], well, I can't say that the idea was >> unanimous at that time... > > Yeah, I don't understand the turn that conversation took. We're talking > about a feature that can be used for reverse-proxying, and there's > nothing to CONNECT to. > > --Jacob >
