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

pywebsocket3 vs. websockets #28863

Closed
foolip opened this issue May 6, 2021 · 5 comments
Closed

pywebsocket3 vs. websockets #28863

foolip opened this issue May 6, 2021 · 5 comments

Comments

@foolip
Copy link
Member

foolip commented May 6, 2021

We currently use two different Python libraries for WebSockets:

  • pywebsocket3. This is what is used in wptserve and websockets/handlers/ to test the WebSockets protocol itself.
  • websockets, recently introduced for testing WebDriver BiDi. It's designed for using WebSockets rather than testing it, and is probably not as easy to use for sending deliberately invalid data.

Both are vendored into tools/third_party/.

Note that even with pywebsocket3 we're using "private" APIs:

# TODO: We are using "private" methods of pywebsocket. We might want to
# refactor pywebsocket to expose those methods publicly. Also, _get_origin
# _check_version _set_protocol _parse_extensions and a large part of do_handshake
# are identical with hybi handshake. We need some refactoring to get remove that
# code duplication.
from mod_pywebsocket.extensions import get_extension_processor
from mod_pywebsocket.handshake._base import get_mandatory_header
from mod_pywebsocket.handshake._base import HandshakeException
from mod_pywebsocket.handshake._base import parse_token_list
from mod_pywebsocket.handshake._base import validate_mandatory_header
from mod_pywebsocket.handshake._base import validate_subprotocol
from mod_pywebsocket.handshake._base import VersionException

And note that with WebDriver BiDi, we'll also want to have the ability to send some invalid messages, such as text messages that aren't valid UTF-8.

Has anyone looked into these libraries in detail? Is there a possible path where we can have just one library which is patchable enough for all of our needs?

cc @web-platform-tests/wpt-core-team + websockets/ folks @jdm @ricea @zqzhang

@ricea
Copy link
Contributor

ricea commented May 6, 2021

It looks like "websockets" is much better for writing readable clients, and "pywebsocket3" is much better for writing servers that do evil things. Personally I don't see a problem with having two libraries as they have quite different purposes. I haven't used "websockets" but I would guess that making it do evil things would mess up the clean API. It would be possible to add a nice client API to pywebsocket3, but there isn't much motivation to do that since "websockets" does it better :-)

@foolip
Copy link
Member Author

foolip commented May 6, 2021

So is pywebsocket3 only ever used as a server? If we only use websockets as a client that would be an explainable split.

How deep does the "do evil things" go with pywebsocket3? Is it the ability to send arbitrary bytes in the network packets, that don't even resemble the WebSockets framing?

@zcorpan
Copy link
Member

zcorpan commented May 6, 2021

A few examples:

https://github.com/web-platform-tests/wpt/blob/master/websockets/handlers/sleep_10_v13_wsh.py trickles bytes in the handshake phase with 2 second pauses.

https://github.com/web-platform-tests/wpt/blob/master/websockets/handlers/invalid_wsh.py writes invalid bytes to the stream before the handshake.

https://github.com/web-platform-tests/wpt/blob/master/websockets/handlers/echo_raw_wsh.py writes whatever is in the incoming message data as bytes.

@foolip
Copy link
Member Author

foolip commented May 6, 2021

Thanks @zcorpan!

Messing with the handshake in websockets doesn't look straightforward, but from a quick look it seem like the size of the hack to allow sending or receiving arbitrary messages should be pretty small. But if most of what pywebsocket3 does is evil stuff and there's very little exchange of well-formed WebSocket frames, then there doesn't seem to be any overlap except for establishing the raw socket.

@foolip
Copy link
Member Author

foolip commented May 6, 2021

It would be nice to document this seeming redundancy somewhere, but right now there is no place where both libraries appear. I'll add a TODO in #28796.

@foolip foolip closed this as completed May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants