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

Modify CloseWebSocketFrame#statusCode() to change the fetch c… #13114

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

zhangjinying
Copy link
Contributor

Motivation:

According to the websocket protocol description, change the get code method in the CloseWebSocketFrame to unsigned.
image

Modification:

Modify changed CloseWebSocketFrame#statusCode() to change the fetch code to unsigned

Result:

Fixes #13113.

@zhangjinying zhangjinying changed the title Modify changed CloseWebSocketFrame#statusCode() to change the fetch c… Modify CloseWebSocketFrame#statusCode() to change the fetch c… Jan 10, 2023
@normanmaurer
Copy link
Member

Can you please sign our ICLA: https://netty.io/s/icla and let me know once done

@zhangjinying
Copy link
Contributor Author

ICLA

Agreed, done

@mostroverkhov
Copy link
Contributor

On the next line the spec says status code with value /code/ defined in [Section 7.4](https://www.rfc-editor.org/rfc/rfc6455#section-7.4). rfc6455 section-7.4 defines maximum value of 4999: so while this change is "formally" correct, I am wondering what library/software/user-agent is a) using status > 4999 b) using status > 32_767 which would lead to overflow with current code?

@zhangjinying
Copy link
Contributor Author

On the next line the spec says status code with value /code/ defined in [Section 7.4](https://www.rfc-editor.org/rfc/rfc6455#section-7.4). rfc6455 section-7.4 defines maximum value of 4999: so while this change is "formally" correct, I am wondering what library/software/user-agent is a) using status > 4999 b) using status > 32_767 which would lead to overflow with current code?

Yes, but rfc6455 section-7.4 is no restriction that you cannot use more than 32_767, and custom code should not overflow if it is greater than 32_767 (according to https://www.rfc-editor.org/rfc/rfc6455#section-5.5.1the first two bytes of
the body MUST be a 2-byte unsigned integer (in network byte order))

@normanmaurer normanmaurer merged commit c769fd8 into netty:4.1 Jan 13, 2023
@normanmaurer normanmaurer added this to the 4.1.88.Final milestone Jan 13, 2023
normanmaurer pushed a commit that referenced this pull request Jan 13, 2023
…ode to unsigned (#13114)

Motivation:

According to the websocket protocol description, change the get code method in the CloseWebSocketFrame to unsigned.

Modification:

Modify changed CloseWebSocketFrame#statusCode() to change the fetch code to unsigned

Result:

Fixes #13113.

Co-authored-by: jinying.zhang <jinying.zhang@okg.com>
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.

CloseWebSocketFrame statusCode() get unexpected results
3 participants