-
-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ipv6 #88
Comments
I've checked the latest update and worked through the websocket-client changes between 0.4.1 and the current version (0.32.0), both in the tests and with the dev team for websocket-client. There are two changes that break the tests: the name of an exception has changed and the value returned from websockets when a connection disappears has changed from None to "". Ironically, the feedback from the websocket-client team was that None is probably a more sensible return value, but the newer version is now too widely deployed to change the API contract, again. From my pov, the sockjs stuff now all works over ipv6. fwiw, here's my patch for the change to this repo - although I favour pinning the package versions, rather than simply pulling the latest as my patch implies:
|
As mentioned in #86, we don't want to upgrade websocket-client because it used to test Hixie/Draft 76 compatibility. If you can find another module to use to test ipv6, I would welcome that change. |
I believe that the patch makes the Hixie-76 tests pass - I had two errors in test_closed () and test_broken_json (), which were both due to the exception name change from ConnectionClosedException to WebSocketConnectionClosedException. When the name change is fixed, both tests fail due to the change in semantics of the returned value (ie utils.py's WebSocket8Client.recv() returns an empty string, rather than None.), which is the change in tested returned value. So I don't think that there's anything else to do. If there is, pls explain and I'll see if I can have a crack at it. On the sockjs-node end, I'm using the 'make test_server' and changing the line in config.js:
The test_server then listens on both ipv4 and ipv6, although I've not built automated tests to confirm that, which I would do to support a patch. fwiw, the tests run end to end (with a fixed dns server) with the server accessed directly, or via an intermediate ipv4 host, which routes to ipv6 with a simple socat application bridge. Newer versions of the various dependent libraries now seem to support stuff like parsing ipv6 addresses, handling different dns connections, etc, so ipv6 'just works'. Test hosts are fedora 21 for both ends. |
Upgrading websocket-client means it no longer uses Hixie-76, which is why we can't upgrade it. |
ok. I clearly don't understand what I'm looking at - not much change there, then :-)
and on sockjs-node:
what would I need to do for the ipv6 validation patch? |
If you look at https://github.com/liris/websocket-client it says
but, if you go back to the version we are using in the tests, 0.4.1, it does not say that, because it is using Hixie-76. So the reason we are using the old version is test our compatibility with Hixie-76. I think if we wanted to update the tests to support IPv6, we would need to find a module that supports both Hixie-76 and IPv6. |
ok. I see what you mean now. Presumably the Hixie tests are being swallowed somewhere in the webclient code and should be failing. Do you collect stats on which protocol versions are actually used - although it makes sense to support widely used versions, it's a killer to support all old versions and drains a lot of effort from supporting new stuff. An approach could be to not support Hixie-76 on ipv6. That said, I seem to be the only person wanting ipv6, so my views should be discounted. I'll see if I can spot a websocket client library that supports both ipv6 and Hixie-76. |
A main use case of sockjs is to support older browsers and transports, so keeping support for Hixie-76 is desired. This combo of Hixie-76 and IPv6 support will also extend to any of the server implementations beyond these tests, so having the tests verify correct behavior is important. I do think IPv6 support is desired, but not at the cost of other features. I will try and do some research to find a way forward. |
ok. I must say, now that I've read a bit more of the background, different protocol version support looks like a product management nightmare. Happy to help if I can. |
support's not moved to faye-websocket has it? I had some issues with getting the compatible versions of websocket-client and faye-websocket sorted out (cruft left by the build process), and faye-websocket has references to Hixie-76 in it. I'm not clear how these components interact. |
doh! that's on the node end! |
This page has some stats on protocol support for browsers: http://bit.ly/1JgKR3z. If it's correct then ~86% of browsers in use support the newer versions and <1% the older ones. I don't know the source data, but I have used netcraft for distributions in the past. Of course, this doesn't identify the actual market use and it could well be that there are important subgroups that require older protocol support. |
Following on from #86, I think that I removed the dependencies on the old websocket-client and ws4py, so that I could run the tests over ipv6 - I was wanting to see if I could get sockjs working between ipv4 and ipv6. I could.
However, there are some tests that fail against the sockjs-node server, so I'm not confident about my patches. Also, the semantics of the websocket-client WebSocket8Client.recv () method may changed -I had to change the util_03.py to match, which could also be wrong.
If you're interested in the patches, I'll fork this repo and send you a pull request.
otoh, ipv6 is, I think, about to become more important: US client penetration is now >20%, with 7-8% globally. Back in March, when Google was measuring 6%, fb was seeing 9-12% global penetration.
The text was updated successfully, but these errors were encountered: