Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Opcode shouldn't be overridden on continuation frame #330

Closed
Lawouach opened this Issue Aug 8, 2011 · 6 comments

Comments

Projects
None yet
2 participants

Lawouach commented Aug 8, 2011

The opcode is overridden on each continuation frame the server receives which means that on the final frame, it's not set to the proper opcode (text or binar) but instead set to continuation.

Can be easily fixed by using:

if self._fragmented_message_opcode is None:
      self._fragmented_message_opcode = self._frame_opcode
Owner

bdarnell commented Aug 14, 2011

Where exactly are you talking about?

Sorry I should've been clearer. In the websocket module:

https://github.com/facebook/tornado/blob/master/tornado/websocket.py#L455

Owner

bdarnell commented Aug 14, 2011

I'm not completely familiar with the logic here, but isn't this taken care of by the _fragmented_message_buffer check just above? I think the flow is that non-fragmented messages go to line 463, and fragmented messages have their first frame handled at 455, middle frames at 453, and the last frame at 459.

Also, _fragmented_message_opcode is never reset to None once it's been set, so for your change to work that would need to happen somewhere (probably at line 461).

On fragmented frame, this is what I'm understanding from the spec:

F0 is either text or binary with fin = 0
F1...Fn-1 is continuation with fin = 0
Fn is continuation with fin=1

With your current implementation, the initial opcode is loast since it's overriden on each continuation frame. This means that on the final frame, the opcode you have is not text or binary as you should but continuation.

Perhaps my understanding of that bit of the protocol is wrong.

Owner

bdarnell commented Aug 15, 2011

I think your understanding of the protocol is right, but I think the code handles this correctly - the opcode is transferred from the first packet to the last; the middle frames don't touch it. Do you have evidence that this is failing?

Just tried again and indeed it does seem to work. I believe I met the issue while developing ws4py and it was probably a bug in my code that was fixed along the way.

https://github.com/Lawouach/WebSocket-for-Python

Let me close this issue then.

@Lawouach Lawouach closed this Aug 15, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment