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

Websocket issues while headers are fragmented #1951

Closed
molszowy opened this issue Jun 5, 2017 · 14 comments
Closed

Websocket issues while headers are fragmented #1951

molszowy opened this issue Jun 5, 2017 · 14 comments
Labels

Comments

@molszowy
Copy link

molszowy commented Jun 5, 2017

I am looking for help to narrow down an issue with websockets. Intermittently, the server sends a frame that is split in multiple TCP/IP packets which seems to be the problem for the websocket client implementation in aiohttp. I observe the errors like : "Received frame with non-zero reserved bits" or no response to PING which drops the connection as 'Idle'.

The example is PING which is normally send in one single packet(packet 10902) and parsed correctly by the library but when it comes split in two packets(11135, 11136), it's not recognized as PING. I put the debug line in http_websocket.py to see what's coming in to parse_frame()

http_websocket.py

def _feed_data(self, data):

      DEV_LOGGER.debug('parse-frame: %s' % data)

      for fin, opcode, payload in self.parse_frame(data):

Packet capture:

image

Logs from my client:
2017-06-02 15:57:10,960http_websocket.py:191: _feed_data() parse-frame: b'\x89\x00'
2017-06-02 15:57:10,961 Detail="The websocket received a ping."
2017-06-02 15:57:10,961 Detail="Sent pong response."
2017-06-02 15:57:10,961 Detail="Listening for messages on websocket."
2017-06-02 15:57:26,087http_websocket.py:191: _feed_data() parse-frame: b'\x89'
2017-06-02 15:57:26,087http_websocket.py:191: _feed_data() parse-frame: b'\x00'

If I am not mistaken the library treats the received data as two independent frames(framing issue?) and does not detect PING.

Thanks in advance for help!

@fafhrd91
Copy link
Member

fafhrd91 commented Jun 5, 2017

Thanks for detailed description! I will look into this problem

@molszowy
Copy link
Author

molszowy commented Jun 6, 2017

Thanks Nikolay! Did you have a chance to investigate this further ? Let me know if you need more data, etc.

@jonathanslenders
Copy link

Hi @fafhrd91,

Probably this was caused by a bug in the parse_frame function in http_websocket.py.
Every time we break because of not enough data yet, the remaining data should be assigned to self._tail.

So, self._tail = buf[start_pos:] needs to be placed before every break in parse_frame.
@molszowy: can you confirm that?

Thanks,
Jonathan

@fafhrd91
Copy link
Member

fafhrd91 commented Jun 6, 2017

haven't looked yet.

@jonathanslenders would you create PR?

@pjknkda
Copy link
Contributor

pjknkda commented Jun 6, 2017

I'm working on this issue and other websocket-related issues (based on https://rawgit.com/pjknkda/ws-spec-check/master/reports/fuzzingclient/index.html) and it's almost done.

@molszowy
Copy link
Author

@pjknkda thanks for fixing this problem! Seems to work fine :)
When can we expect a new release ?

@asvetlov
Copy link
Member

I'll publish 2.1.1 bugfix with #1962 very soon.

@fafhrd91
Copy link
Member

Wait befor I review #1955

@asvetlov
Copy link
Member

Sure. Any other issues?
I wanted to cherry-pick 03984c2 only without publishing all other 2.2.0 features.

@fafhrd91
Copy link
Member

I don't think we have any breaking changes, lets just release everything as 2.1.1

@asvetlov
Copy link
Member

It works to me.
Should I wait for #1955 ?

@fafhrd91
Copy link
Member

yes, I want to fix it

@willmcgugan
Copy link
Contributor

This issue effects us. Working on the master branch fixes it. Any ETA on 2.1.1 ?

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants