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

Correct the client Websocket handshake #1508

Merged
merged 3 commits into from Jun 3, 2016

Conversation

Projects
None yet
2 participants
@Geod24
Contributor

Geod24 commented Jun 2, 2016

The RFC specify in section 4.1 (Opening Handshake, Client Requirements) that:
"The value of this header field MUST be a nonce consisting of a randomly
selected 16-byte value that has been base64-encoded"

However randomUUID.toString would return a 36 chars string.
As a result, the base64 encoding was 48 chars long instead of 24.

Geod24 added some commits Jun 2, 2016

websocket: Simplify server / client difference
Before, the code used to have a flag for server code, and pass around
a SystemRNG as source of entropy.
This was a bit weird as their two parameters where not explicitly tied
together.
With this change, we assume that the presence of a source of entropy
means it's a client socket, and a `null` value means its a server.
websocket: Correct length for Sec-WebSocket-Key during protocol upgrade
The RFC specify in section 4.1 (Opening Handshake, Client Requirements) that:
"The value of this header field MUST be a nonce consisting of a randomly
selected 16-byte value that has been base64-encoded"

However `randomUUID.toString` would return a 36 chars string.
As a result, the base64 encoding was 48 chars long instead of 24.

Part of vibe-d/vibe.d#1505
@Geod24

This comment has been minimized.

Show comment
Hide comment
@Geod24

Geod24 Jun 2, 2016

Contributor

Added a commit that do case insensitive comparison of the protocol string in the http client (caused Upgrade: WebSockets to be wrongly rejected).

Contributor

Geod24 commented Jun 2, 2016

Added a commit that do case insensitive comparison of the protocol string in the http client (caused Upgrade: WebSockets to be wrongly rejected).

http client: Do case-insensitive comparison of protocol upgrade
It's unlikely that the case matter for any protocol upgrade.
For websockets it doesn't.  If any protocol need case-sensitive
comparison they can still enforce it themselves.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 3, 2016

Member

LGTM!

Member

s-ludwig commented Jun 3, 2016

LGTM!

@s-ludwig s-ludwig merged commit 5e0afff into vibe-d:master Jun 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Geod24 Geod24 deleted the Geod24:websockets-handshake branch Jun 3, 2016

Geod24 added a commit to Geod24/vibe.d that referenced this pull request Jun 4, 2016

Fixup PR #1508: Do case-insensitive comparison of protocol upgrade
The original P.R. only fixed one of the overload.

s-ludwig added a commit that referenced this pull request Jun 6, 2016

Merge pull request #1511 from Geod24/fixup-pr-1508
Fixup PR #1508: Do case-insensitive comparison of protocol upgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment