Comments (5)
The default value of on_exc
is very important. Otherwise errors would be swallowed, which is something you never want. It's also consistent with lwt.
The default value is of check_request
is also important. The alternative of not checking it by default would leave many websocket servers open to cross-origin requests.
The real problem seems to be that we raise when the header checks fail. I don't think we should fail and call on_exc
when a client sends bad headers or is failing the check_request
test. We're already handling these case gracefully by replying with 403 Forbidden
to the client. Instead of failing, we should log the problem.
This is problematic when the server is behind a WS proxy because this control will fail and the resulting exception does not give a lot of information (so it took me some time to find it).
Shouldn't the proxy forward headers? Note that check_origin_with_host
doesn't fail if the origin header is missing, only if there is a mismatch. If proxies don't forward the headers for a good reason, we should document this in establish_server
.
from ocaml-websocket.
The real problem seems to be that we raise when the header checks fail. I don't think we should fail and call on_exc when a client sends bad headers or is failing the check_request test. We're already handling these case gracefully by replying with 403 Forbidden to the client. Instead of failing, we should log the problem.
Definitely agree. Sorry that you took time debugging @zoggy . I think we should stop raising an exception on client error, this does not make sense I probably coded this a long time ago and never made this part of the code evolve. I'm quite ashamed of this actually. I'll fix this soon, thanks.
from ocaml-websocket.
@vbmithr No problem. We can't focus on everything at the same time, then we forget :)
@copy I agree that on_exc
is important so it could be mandatory instead of optional. But since this would break backward compatiblity, having a default function which log the exception seems fine to me.
Regarding the check_request
defaulting to check_origin_with_host
, well this does not protect a lot since the client can put whatever it wants in the origin field but it may be better than nothing and it should be documented. Note that the problem with my proxy was that it changed the host:
field in the request from the original server name to localhost:XXX
when tunnelling the request to the docker container running the server. It seems that this is the default in apache (https://httpd.apache.org/docs/2.4/mod/mod_proxy.html#proxypreservehost) so others after me will probably get bitten by this.
By now, I provide as check_request
a function comparing the origin, if present, to the "public hostname" (i.e. the hostname of the proxy) of my application. So I think it could be useful to have an additional function check_origin_host
taking a hostname list and a request and checking that the origin of the request is in the given hostname list. check_origin_with_host
could use this function. check_origin_host
could be used as check_request
parameter when the server can handle websocket requests from different hosts.
from ocaml-websocket.
Regarding the check_request defaulting to check_origin_with_host, well this does not protect a lot since the client can put whatever it wants in the origin field but it may be better than nothing and it should be documented.
The JavaScript API in browsers doesn't let you override the Origin, so our default enforces the same-origin policy. It was inspired by a Go websocket implementation: https://godoc.org/github.com/gorilla/websocket#hdr-Origin_Considerations
check_origin_host could be used as check_request parameter when the server can handle websocket requests from different hosts.
This sounds useful, care to make a PR?
from ocaml-websocket.
Indeed the javascript API prevents setting the origin but I can still use Curl of any other library or tool to send any headers and pass the checks.
Here is a patch to add Websocket_lwt.check_origin
: #86
from ocaml-websocket.
Related Issues (20)
- Spurious Socket.t argument in Websocket_async.client_ez
- Unbound module Conduit_async_ssl in async/wscat.ml HOT 2
- Error: Unbound module Lwt_log_core HOT 1
- Usage with Mirage HOT 8
- Websocket_cohttp_lwt.upgrade_connection doesn't work with TLS connections HOT 5
- Release 2.13 on opam HOT 3
- cohttp_lwt: may be pass frames_out_fn into incoming_handler? HOT 1
- Make an opam release HOT 1
- Exception when client closes connection. Is this the expected behavior for async? HOT 2
- Lwt example fails with "Bad headers" when connecting from Firefox HOT 2
- Update to Conduit 3.0.0 HOT 2
- Closing client connections HOT 2
- New release HOT 3
- Expose a way to flush writes in `Websocket_lwt_unix` HOT 5
- Frame continuation ops code closes the websocket pipe reader prematurely. HOT 1
- End_of_file exception when doing `send` HOT 8
- websockets + react hangs on frequent messages HOT 1
- cohttp.1.0.0 support HOT 2
- lwt/websocket_cohttp_lwt.mli, line 24, characters 23-40: # Error: Unbound module Cohttp_lwt_body HOT 1
- How do I respend to client with subprotocol (Sec-WebSocket-Protocol). HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ocaml-websocket.