On 10/18/16 11:46, Stephen R. van den Berg wrote:
Arne Goedeke wrote:
- permessage-deflate is activated by default. i don't believe that
this is a good choice.
The default is what should help most users. I think it would have, but it's not a big deal.
My applications for websockets usually focus on binary data. I usually care more about latency and CPU than bandwidth and the payload does not compress well. This is probably different for situations which involve text. I don't know what other people do. I would propose the following: Let's wait until this extension support has become more stable and then decide which of the "extensions" make sense as a default (defragment is probably helpful too in most situations).
- the negotiation code which determines the final set of options is
faulty. For instance, it activates the deflate extension even if it was not actually negotiated. This breaks websockets for all clients which do not use this extension!
I tried to follow the standard here, but I admit that the different conditions spelled out in the standard are convoluted to begin with.
I just extended the extension support to client mode connections. That seems to work fine so far. The client does not currently offer any of the extension parameters during handshake.
To implement the handshake correctly we have to:
* fail connections with invalid parameters * implement fallback (clients can offer several combinations of parameters and a server is supposed to pick the first that works)
- the extension itself is defined on messages not frames. this is why
low_parse (for instance) is not necessarily the best place to do the inflate() and one reason why it does not work correctly at this point.
Ah, I misread the standard here, I think.
classes. I refactored your code to use that new API. The code now passes the complete 'autobahn-testsuite', including all tests for permessage-deflate. The code has lost the per-frame option overwrite for deflate, but I am sure we can fit that back in. Please check if it works with your setup.
I'll have a look. I'm in the middle of some SocketIO/EngineIO refactoring as well.
diverged quite a lot. Additionally, this introduces a new API, which might change again in 8.1. I therefore stand by my initial comment. This should not have been added to 8.0 and I think it should be removed again.
Ok, fair enough. I'll remove it again (minus minor fixes).
Thanks.