Bug 115863 – [WebSocket] Update pywebsocket to r760

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Lamarque V. Souza]

This pywebsocket version supports draft-ietf-hybi-permessage-compression-08. I am going to port the patch in https://code.google.com/p/chromium/issues/detail?id=229290 to WebKit. I plan to also port the permessage-deflate extension from there.

[Lamarque V. Souza]

Created attachment 201248 [details]
Patch

Proposed patch

[Alexey Proskuryakov]

Comment on attachment 201248 [details]
Patch

rs=me

[Alexey Proskuryakov]

Comment on attachment 201248 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201248&action=review

> LayoutTests/ChangeLog:21
> +        * http/tests/websocket/tests/hybi/url-with-empty-query-expected.txt: Removed.
> +        * http/tests/websocket/tests/hybi/url-with-empty-query.html: Removed.
> +        * http/tests/websocket/tests/hybi/url-with-query-expected.txt: Removed.
> +        * http/tests/websocket/tests/hybi/url-with-query.html: Removed.

Wait, why is this not supported? This has nothing to do with compression, please explain.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 201248 [details]:

media/track/track-remove-crash.html bug 115892 (author: [e-mail])
The commit-queue is continuing to process your patch.

[Lamarque V. Souza]

(In reply to comment #3)
> (From update of attachment 201248 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201248&action=review
> 
> > LayoutTests/ChangeLog:21
> > +        * http/tests/websocket/tests/hybi/url-with-empty-query-expected.txt: Removed.
> > +        * http/tests/websocket/tests/hybi/url-with-empty-query.html: Removed.
> > +        * http/tests/websocket/tests/hybi/url-with-query-expected.txt: Removed.
> > +        * http/tests/websocket/tests/hybi/url-with-query.html: Removed.
> 
> Wait, why is this not supported? This has nothing to do with compression, please explain.

Blink's developers deleted them because they failed after pywebsocket update. I have just researched and the tests were restored in a later commit [1]. I need to figure out what changes are necessary to make them work properly (they still does not work here).

[1] https://code.google.com/p/chromium/issues/detail?id=234541

[Lamarque V. Souza]

Created attachment 201413 [details]
Patch

Fix url-with-empty-query.html and url-with-query.html instead of removing them. The fix came from the patch in https://code.google.com/p/chromium/issues/detail?id=234541 . The fix does some changes that I think are not completely necessary, but for the sake of future updates coming from Blink I did not remove them.

[Takeshi Yoshino]

(In reply to comment #5)
> (In reply to comment #3)
> > Wait, why is this not supported? This has nothing to do with compression, please explain.
> 
> Blink's developers deleted them because they failed after pywebsocket update. I have just researched and the tests were restored in a later commit [1]. I need to figure out what changes are necessary to make them work properly (they still does not work here).

Yes. They were originally testing the Sec-WebSocket-Location header of HyBi 00 that no longer exists in RFC 6455. As side effect, they were testing that the string passed to the WebSocket constructor as url argument are correctly parsed (in order to build the Sec-WebSocket-Location header). To keep this code path tested for RFC 6455 protocol, too, we were setting the ws_location attribute of _StandaloneRequest object using one manually built from the host header and the resource inside the _wsh.py file.

Since Chromium no longer supports HyBi 00, we've cleaned up these Sec-WebSocket-Location related files and code, and added back some new layout test items that directly check the host header and the resource.

http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/websocket/tests/hybi/echo-host_wsh.py
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/websocket/tests/hybi/echo-path_wsh.py

[Lamarque V. Souza]

(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > Wait, why is this not supported? This has nothing to do with compression, please explain.
> > 
> > Blink's developers deleted them because they failed after pywebsocket update. I have just researched and the tests were restored in a later commit [1]. I need to figure out what changes are necessary to make them work properly (they still does not work here).
> 
> Yes. They were originally testing the Sec-WebSocket-Location header of HyBi 00 that no longer exists in RFC 6455. As side effect, they were testing that the string passed to the WebSocket constructor as url argument are correctly parsed (in order to build the Sec-WebSocket-Location header). To keep this code path tested for RFC 6455 protocol, too, we were setting the ws_location attribute of _StandaloneRequest object using one manually built from the host header and the resource inside the _wsh.py file.

Thank you for the explanation.
 
> Since Chromium no longer supports HyBi 00, we've cleaned up these Sec-WebSocket-Location related files and code, and added back some new layout test items that directly check the host header and the resource.

The remove of support for HyBi 00 was done before the WebKit fork (or Blink was created), correct? The last mention of Sec-WebSocket-Location in WebKit is from last year by what I can see in git's log.

[Alexey Proskuryakov]

Comment on attachment 201413 [details]
Patch

rs=me

[WebKit Commit Bot]

Comment on attachment 201413 [details]
Patch

Clearing flags on attachment: 201413

Committed r150225: <http://trac.webkit.org/changeset/150225>

[Takeshi Yoshino]

(In reply to comment #8)
> The remove of support for HyBi 00 was done before the WebKit fork (or Blink was created), correct? The last mention of Sec-WebSocket-Location in WebKit is from last year by what I can see in git's log.

Right. We added a flag to select the protocol version in July 2011 and stopped supporting HyBi 00 for Chromium.

http://trac.webkit.org/browser/trunk/Source/WebCore/websockets/WebSocketChannel.cpp?rev=90704

and completely removed HyBi 00 code in this change made in July 2012.

http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp?rev=122199