Skip to content
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

Open
timcoote opened this issue Jul 26, 2015 · 12 comments
Open

ipv6 #88

timcoote opened this issue Jul 26, 2015 · 12 comments

Comments

@timcoote
Copy link

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.

@timcoote
Copy link
Author

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:

diff --git a/requirements.txt b/requirements.txt
index f838ad3..8c734b3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,3 @@
 unittest2
-websocket-client==0.4.1
-ws4py
\ No newline at end of file
+websocket-client #==0.4.1
+ws4py
diff --git a/sockjs-protocol.py b/sockjs-protocol.py
index e6e8a3d..a5afc4f 100644
--- a/sockjs-protocol.py
+++ b/sockjs-protocol.py
@@ -543,9 +543,9 @@ class WebsocketHixie76(Test):
         self.assertEqual(ws.recv(), u'c[3000,"Go away!"]')

         # The connection should be closed after the close frame.
-        with self.assertRaises(websocket.ConnectionClosedException):
-            if ws.recv() is None:
-                raise websocket.ConnectionClosedException
+        with self.assertRaises(websocket.WebSocketConnectionClosedException):
+            if ws.recv() is "":
+                raise websocket.WebSocketConnectionClosedException
         ws.close()

     # Empty frames must be ignored by the server side.
@@ -639,9 +639,9 @@ class WebsocketHixie76(Test):
         ws = websocket.create_connection(ws_url)
         self.assertEqual(ws.recv(), u'o')
         ws.send(u'["a')
-        with self.assertRaises(websocket.ConnectionClosedException):
-            if ws.recv() is None:
-                raise websocket.ConnectionClosedException
+        with self.assertRaises(websocket.WebSocketConnectionClosedException):
+            if ws.recv() is "":
+                raise websocket.WebSocketConnectionClosedException
         ws.close()

@brycekahle
Copy link
Contributor

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.

@timcoote
Copy link
Author

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:

git diff 4561f500e92
diff --git a/tests/test_server/config.js b/tests/test_server/config.js
index 415e1eb..d5ae686 100644
--- a/tests/test_server/config.js
+++ b/tests/test_server/config.js
@@ -5,5 +5,5 @@ exports.config = {
     },

     port: 8081,
-    host: '0.0.0.0'
+    host: '::'
 };

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.

@brycekahle
Copy link
Contributor

Upgrading websocket-client means it no longer uses Hixie-76, which is why we can't upgrade it.

@timcoote
Copy link
Author

ok. I clearly don't understand what I'm looking at - not much change there, then :-)

 venv/bin/python sockjs-protocol.py WebsocketHixie76
......
----------------------------------------------------------------------
Ran 6 tests in 0.092s

OK

and on sockjs-node:

 make test_server
./node_modules/.bin/coffee -o lib/ -c src/*.coffee
node tests/test_server/server.js
SockJS v0.3.15 bound to "/echo"
SockJS v0.3.15 bound to "/disabled_websocket_echo"
SockJS v0.3.15 bound to "/cookie_needed_echo"
SockJS v0.3.15 bound to "/close"
SockJS v0.3.15 bound to "/ticker"
SockJS v0.3.15 bound to "/amplify"
SockJS v0.3.15 bound to "/broadcast"
 [*] Listening on :::8081
GET /echo/000/07285fff-78c9-4fc3-b0bf-b86b28ba2d83/websocket 7ms (unfinished)
    [+] echo open    <SockJSConnection d621f698-1dcc-40d9-bbda-c689e2abf2e0>
    [-] echo close   <SockJSConnection d621f698-1dcc-40d9-bbda-c689e2abf2e0>
GET /close/000/858f6238-8bde-4d31-ba90-fb48bef4057b/websocket 3ms (unfinished)
    [+] clos open    <SockJSConnection 1eb7928a-5e00-42ac-a6a5-e59f4bd16781>
GET /echo/000/98a437fb-e9d1-4858-8a74-855ace108c24/websocket 1ms (unfinished)
    [+] echo open    <SockJSConnection ff9ceb21-a74a-432b-9a9e-e51cbb391e5e>
    [ ] echo message <SockJSConnection ff9ceb21-a74a-432b-9a9e-e51cbb391e5e> "a"
    [-] echo close   <SockJSConnection ff9ceb21-a74a-432b-9a9e-e51cbb391e5e>
GET /echo/000/66bdc4aa-0a02-40ad-b3d2-bd9ba6f52795/websocket 1ms (unfinished)
    [+] echo open    <SockJSConnection 1a2f9ac9-6f1a-4608-b386-c44309529660>
    [-] echo close   <SockJSConnection 1a2f9ac9-6f1a-4608-b386-c44309529660>
GET /echo/000/c23837ee-ed39-4734-9fc0-f20964bc74e0/websocket 1ms (unfinished)
    [+] echo open    <SockJSConnection 30f9e70b-1087-4d9c-a172-6e1eb9abf9ed>
GET /echo/000/c23837ee-ed39-4734-9fc0-f20964bc74e0/websocket 0ms (unfinished)
    [+] echo open    <SockJSConnection 77c7c517-79d8-4e34-8b26-87d585e083e3>
    [ ] echo message <SockJSConnection 30f9e70b-1087-4d9c-a172-6e1eb9abf9ed> "a"
    [ ] echo message <SockJSConnection 77c7c517-79d8-4e34-8b26-87d585e083e3> "b"
    [-] echo close   <SockJSConnection 30f9e70b-1087-4d9c-a172-6e1eb9abf9ed>
    [-] echo close   <SockJSConnection 77c7c517-79d8-4e34-8b26-87d585e083e3>
GET /echo/000/c23837ee-ed39-4734-9fc0-f20964bc74e0/websocket 0ms (unfinished)
    [+] echo open    <SockJSConnection 5128a6cf-e057-4219-b845-40e73e5201f4>
    [ ] echo message <SockJSConnection 5128a6cf-e057-4219-b845-40e73e5201f4> "a"
    [-] echo close   <SockJSConnection 5128a6cf-e057-4219-b845-40e73e5201f4>
GET /echo/000/4ec205c8-5c76-4adb-9770-1e37fa0c3aaa/websocket 3ms (unfinished)
    [+] echo open    <SockJSConnection f7cacbd0-99f9-4a29-bfe2-e09a55dc4501>
    [ ] echo message <SockJSConnection f7cacbd0-99f9-4a29-bfe2-e09a55dc4501> "a"
    [-] echo close   <SockJSConnection f7cacbd0-99f9-4a29-bfe2-e09a55dc4501>

what would I need to do for the ipv6 validation patch?

@brycekahle
Copy link
Contributor

If you look at https://github.com/liris/websocket-client it says

websocket-client supports only hybi-13

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.

@timcoote
Copy link
Author

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.

@brycekahle
Copy link
Contributor

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.

@timcoote
Copy link
Author

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.

@timcoote
Copy link
Author

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.

@timcoote
Copy link
Author

doh! that's on the node end!

@timcoote
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants