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

Change encoding of WebSocket keyValue to forgiving-base64-encoding #844

Merged
merged 3 commits into from Dec 13, 2018

Conversation

2 participants
@Garee
Copy link
Contributor

commented Dec 1, 2018

This improves the consistency of the document by referencing the
same type of base64-encoding throughout.

Fixes #712.


Preview | Diff

Change encoding of WebSocket keyValue to forgiving-base64-encoding
This improves the consistency of the document by referencing the
same type of base64-encoding throughout.

Fixes #712.
@annevk

annevk approved these changes Dec 11, 2018

Copy link
Member

left a comment

Thanks, would you like to add your name to the Acknowledgments section?

There's a potential follow-up fix here, if you want. Namely the return value of this encoding process is a string, so we need to account for that in the example and we'll need to "isomorphic encode" this string before assigning it as header value (as that does require a byte sequence).

Garee added some commits Dec 11, 2018

Add isomorphic encoding to WebSocket keyValue generation
The keyValue is used as the value for the Sec-WebSocket-Key
header. As header values are byte sequences, we must isomorphic
encode the string that is returned from the forgiving-base64-encode.
@Garee

This comment has been minimized.

Copy link
Contributor Author

commented Dec 12, 2018

Thanks @annevk -- I have added my name to the Acknowledgements.

I also attempted to address the issue that you pointed out. Please let me know if it requires changes.

@annevk annevk merged commit e263769 into whatwg:master Dec 13, 2018

2 checks passed

Participation @Garee has signed up to participate as an individual
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@annevk

This comment has been minimized.

Copy link
Member

commented Dec 13, 2018

That's great, thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.