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

Add support for retrieving the websocket close code/reason #1675

Merged
merged 3 commits into from Feb 9, 2017

Conversation

Projects
None yet
2 participants
@b1naryth1ef
Contributor

b1naryth1ef commented Feb 6, 2017

This PR adds support for retrieving a websocket connections close code and reason. Fairly straightforward, but vibe.d's source is all new territory so please advise if something is glaringly wrong or horrible.

The TL;DR implementation;

  1. If we receive a close frame with at least enough bytes for a short, we read the short in as the close code
  2. If the close frame contains more than enough bytes for a short, we read the short in as the close code and read the remaining bytes in as a character array for the close reason.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 6, 2017

Member

Thanks, looks good. The only thing that I noticed is that the WebSocket spec defines some specific values if no explicit close status was sent, so after close, the status should always be != 0:

If this Close control frame contains no status code, The WebSocket
Connection Close Code
is considered to be 1005. If The WebSocket
Connection is Closed
and no Close control frame was received by the
endpoint (such as could occur if the underlying transport connection
is lost), The WebSocket Connection Close Code is considered to be 1006.

(https://tools.ietf.org/html/rfc6455#section-7.1.5)

Member

s-ludwig commented Feb 6, 2017

Thanks, looks good. The only thing that I noticed is that the WebSocket spec defines some specific values if no explicit close status was sent, so after close, the status should always be != 0:

If this Close control frame contains no status code, The WebSocket
Connection Close Code
is considered to be 1005. If The WebSocket
Connection is Closed
and no Close control frame was received by the
endpoint (such as could occur if the underlying transport connection
is lost), The WebSocket Connection Close Code is considered to be 1006.

(https://tools.ietf.org/html/rfc6455#section-7.1.5)

@b1naryth1ef

This comment has been minimized.

Show comment
Hide comment
@b1naryth1ef

b1naryth1ef Feb 8, 2017

Contributor

@s-ludwig should match the RFC better now.

Contributor

b1naryth1ef commented Feb 8, 2017

@s-ludwig should match the RFC better now.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 9, 2017

Member

Not sure about the reason for the Travis-CI failures, this happens since a while, but it's not an issue in vibe.d itself, at least not directly. I'll just restart the test and will wait until it finishes before merging.

Member

s-ludwig commented Feb 9, 2017

Not sure about the reason for the Travis-CI failures, this happens since a while, but it's not an issue in vibe.d itself, at least not directly. I'll just restart the test and will wait until it finishes before merging.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 9, 2017

Member

Okay, failures have been fixed externally.

Member

s-ludwig commented Feb 9, 2017

Okay, failures have been fixed externally.

@s-ludwig s-ludwig merged commit 7b511db into vibe-d:master Feb 9, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment