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

[system] WebSocket continuation frame incorrectly handles the message type #1345

Merged

Conversation

mattleibow
Copy link
Contributor

When a frame marked with OPCODE 0, it is a continuation frame.
This frame does not carry the message type and should be derived from the previous frame.

… type opcode

When a frame marked with OPCODE 0, it is a continuation frame. This frame does not carry the message type and should be derived from the previous frame.
@monoadmin
Copy link

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@mattleibow
Copy link
Contributor Author

Gist of some logging:
https://gist.github.com/mattleibow/b7a532cb52bd7026d4ff

On line 18, the WireToMessageType is 1, aka text, and this is correctly translated to the WebSocketMessageType on line 21.

However, on line 46, there is a continuation frame, (FIN is set to 0), aka OPCODE 0, which is incorrectly translated into a Close on line 49.

See more info: http://tools.ietf.org/html/rfc6455#section-5.2

@mattleibow
Copy link
Contributor Author

Added a bugzilla bug: https://bugzilla.xamarin.com/show_bug.cgi?id=23838

@mattleibow
Copy link
Contributor Author

My knowledge of how to create a server message that will cause this is very limited, so I just used the SignalR sample. In order to recreate the issue, run the sample webserver from the SignalR repo and then use the console client. Make sure to force the connection to use web sockets as if it fails it reverts to SSE

@akoeplinger
Copy link
Member

This isn't related to #1115, right?

@mattleibow
Copy link
Contributor Author

@akoeplinger, no that seems to be for sending. This one is for the receiving of multiple frames.

@alexrp
Copy link
Contributor

alexrp commented Nov 26, 2014

Can we get an NUnit test case for this? (Do we have tests for websockets?)

@alexrp
Copy link
Contributor

alexrp commented Mar 16, 2015

approve

@alexrp
Copy link
Contributor

alexrp commented May 15, 2015

build

@monojenkins
Copy link
Contributor

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

@lewurm
Copy link
Contributor

lewurm commented Mar 28, 2016

@mattleibow is there still interest to merge this or should it be closed?

@lewurm
Copy link
Contributor

lewurm commented Mar 28, 2016

build

@dnfclas
Copy link

dnfclas commented Apr 1, 2016

@mattleibow, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@migueldeicaza
Copy link
Contributor

This looks like a bug, sadly, I can not find a case that breaks.

Would love to know what this fixes in the wild.

@akoeplinger akoeplinger merged commit f959605 into mono:master Apr 27, 2016
@introspection3
Copy link

@mattleibow there is something wrong with signlar(websocket)

1 similar comment
@introspection3
Copy link

@mattleibow there is something wrong with signlar(websocket)

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ion-frame-fix

[system] WebSocket continuation frame incorrectly handles the message type

Commit migrated from mono/mono@f959605
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.

9 participants