Skip to content

Conversation

@mullermp
Copy link

@mullermp mullermp commented Feb 21, 2020

This change adds WebSocket modules (WS and WSS) to parse WebSocket URIs.

My implementation was inspired by HTTP and HTTPS modules/tests. If I am missing anything, please let me know.

Resources I used:
https://en.wikipedia.org/wiki/WebSocket
https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax

Also looked at how Python does it:
https://websockets.readthedocs.io/en/stable/_modules/websockets/uri.html

Related AWS SDK For Ruby issue:
aws/aws-sdk-ruby#2232

RFC
https://tools.ietf.org/html/rfc6455#section-3
https://tools.ietf.org/html/rfc1738 (section 3.3 for "invalid URI with user info")

@mullermp
Copy link
Author

Ready for review @hsbt :) I notice there's also a PR from @ioquatix (#2) that's been out for a while, so this is just a friendly ping on these.

@ioquatix
Copy link
Member

I merged my own PR... does it simplify this implementation?

@mullermp
Copy link
Author

I merged my own PR... does it simplify this implementation?

Not really .. I saw you had an open PR sitting for a while with no approvals, so I figured I would be a nice gesture to get some attention on it. Didn't realize you had approval to merge it anyway X)

@mullermp mullermp requested a review from hsbt February 26, 2020 18:45
@ioquatix
Copy link
Member

Nice work updating the PR.

@mullermp
Copy link
Author

Let me know if there's anything else! This looks good to merge if the only comment was on documentation.

@mullermp
Copy link
Author

mullermp commented Mar 2, 2020

Friendly ping on this @hsbt @ioquatix. Is it all good to be merged?

@hsbt hsbt merged commit 98f4ce0 into ruby:master Mar 3, 2020
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

Successfully merging this pull request may close these issues.

3 participants