Just in case someone wants to use the 'autobahn-testsuite', I have published two simple pike scripts which serve as corresponding client/server endpoints here:
https://github.com/arneg/pike-autobahntestsuite
Arne
On 10/18/16 11:29, Arne Goedeke wrote:
I have finally found some time to actually test the new extension support. Unfortunately, it breaks several test cases. Problems I found:
- permessage-deflate is activated by default. i don't believe that this is a good choice.
- 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!
- 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.
I initially started by fixing the first two problems but then decided that solving the third problem will require some refactoring. I therefore added a new API which allows implementing extensions using 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.
The same extension API is now also used to implement defragmentation and conformance checks. None of the extensions is used by default, they need to be explicitly specified in calls to websocket_accept(). The client side code does not currently support them but I will add that later in 8.1.
I do not intend to backport this to 8.0 since the code has already 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.
Arne
On 09/26/16 19:54, Stephen R. van den Berg wrote:
Arne Goedeke wrote:
over time. My general feeling about performance in Pike is also that in many cases these things do not make a huge difference. However, feel
True. Then again, I'm planning on having large Pike farms serving hundreds of Socket.IO clients/browsers. Then performance starts to matter.
I have a general comment about adding things to pike 8.0. If you change existing code it means that those additions somehow need be properly tested before a release can be made. I think it makes more sense to add those changes to 8.1 first and backport them when they have been found to be sufficiently stable to be included in a _stable_ release. It also allows letting the APIs and conventions settle without shipping that to users. I know that there are no official rules about this, but I think it makes life alot easier.
Well, I agree, obviously.
The issue here is clouded a bit by the following factors:
- I discovered that WebSocket support in Pike 8.0 is a bit flaky here and there, so in all likelihood, not many people are using it in its current 8.0 form.
- In order to get it working, I ended up fixing parts of it first.
- I'm trying to run a Socket.IO farm on a production system, so I'd like the platform to be Pike 8.0 instead of 8.1.
- I didn't expect to fix this much. I'd had hoped that compression was already supported by the current implementation.
- The EngineIO and SocketIO stuff is completely new, so no backwards compatibility issues. I expect the API to settle within a few days, because then I've built a running application with it.
All that said, if a Pike 8.0 release needs to be done and the code is not deemed sanctioned/stable yet, I'll happily (temporarily) strip it back out again, and then reschedule putting it back in at some later point in time.