bridge server side ws:// protocol support added for libwebsocket build
Thank you for contributing your time to the Mosquitto project!
Before you go any further, please note that we cannot accept contributions if you haven't signed the Eclipse Contributor Agreement. If you aren't able to do that, or just don't want to, please describe your bug fix/feature change in an issue. For simple bug fixes it is can be just as easy for us to be told about the problem and then go fix it directly.
Then please check the following list of things we ask for in your pull request:
- [ ] Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
- [ ] Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
- [ ] If you are contributing a new feature, is your work based off the develop branch?
- [ ] If you are contributing a bugfix, is your work based off the fixes branch?
- [ ] Have you added an explanation of what your changes do and why you'd like us to include them?
- [ ] Have you successfully run
make testwith your changes locally?
Thanks for this PR. I have a couple of comments:
- From what I can see, the description doesn't match the code. Could you please check it?
- I don't want to support libwebsockets in the future. There is a built-in websockets implementation that already supports the same TLS functionality as the normal listeners - perhaps this means the TLS-PSK changes aren't needed
- I'd want to have some tests to cover the new code
Codecov Report
:x: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/net.c | 25.00% | 3 Missing :warning: |
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/net.c | 65.07% <25.00%> (-0.43%) |
:arrow_down: |
... and 1 file with indirect coverage changes
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
This patch add websocket protocol support to bridge server side. For client side is external tpc to websocket conversion tool still required.
This extension save server resources, there is not necessary websocket conversion back to tcp and allow transparent access throw one access point (web server) for applications, web pages and bridge. This solution is useable with preceding ssl termination server only (ngix or others). It removes complication for application with nonstandard ports (administrations and firewalls limitations) because https only protocol can be used.
We using this for some time, but new developed extension (build in websocket) is not compatible with "duplicit" ssl configuration (http server and bridge), this is reason why I push this feature now, before new buildin websocket will be developed finally. I don't know full future plans and supports libraries, but I did suppose then you can remove libwebsocket dependecy.
Sorry, but maybe I'm being stupid but I can't match up the idea of adding websocket support to bridges with what you have here, most of which is related to setting TLS-PSK communication over an incoming websocket connection and is nothing to do with bridges.
Could you please provide some automated tests to demonstrate the use of the code? There are existing examples in test/broker but if you would like some hints please let me know.
So, there is bigger question "why do not add websocket support to bridge" :-) especially in world, where everething becomes to by web.
You can imagine this. There is one mosquitto server with bridge interface and hundreds another installations which using mosquitto as local broker and connection to central server. In 99.9% organizations is no problem to access https, but specific bridge port to server need IT administrator support, answer the question "is that really necessary?", this is annoying and making troubles. Using websocket for bridge is elegant solution with one access to server. MQTT deliver messages, other applications access same server through https. Elegant and simple solution.
Of course there is double SSL processing (https and bridge PSK), this may look be extra and unnecessary, but let's leave it alone. New implementation of buildin websocket has one big stop. Currently ssl configuration using one shared configuration for websocket and for bridge separated by certificate configuration. That is why is pull limited to ws protocol and do not support wss. With buildin websocket is ssl configuration especially for websocket and there is missing structure or flag to use that configuration for bridge. It complicate bridge websocket implementation. So that's point of pull request.
Finally would be great to support websocket bridge on listener and connections side, with ws and wss support for course. Without specific conversion tools. That's reason why there are no test available yet, connection side support is missing and external software for conversion tpc to websocket is required.
I understand your arguments about the use of websockets. This PR does not do that though. All it does is allow incoming connections to tunnel TLS-PSK data over websockets. It has nothing to do with bridges.
There are no tests, and I'm sorry but I don't trust you based on your responses so I'm closing this PR.
Ok, accepter. You are wrong about bridge and TLS-PSK usage, so may be later again.