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

Stream parser throws away data if too much read from socket #218

Open
daggaz opened this issue May 23, 2017 · 2 comments · May be fixed by #220
Open

Stream parser throws away data if too much read from socket #218

daggaz opened this issue May 23, 2017 · 2 comments · May be fixed by #220

Comments

@daggaz
Copy link

daggaz commented May 23, 2017

If the socket reads more data than requested (as in the SSL case), then stream parser can get more data than it requested.

This means that the some_bytes variable here can contain more than one frame's worth of data.

The frame parser then correctly reads one frame out of the data, but the rest of the data is thrown away.

@willmcgugan
Copy link

This is exactly what happens. We worked around it with by overriding the once method as follows:

    def once(self):
        """
        Performs the operation of reading from the underlying
        connection in order to feed the stream of bytes.
        We start with a small size of two bytes to be read
        from the connection so that we can quickly parse an
        incoming frame header. Then the stream indicates
        whatever size must be read from the connection since
        it knows the frame payload length.
        It returns `False` if an error occurred at the
        socket level or during the bytes processing. Otherwise,
        it returns `True`.
        """
        if self.terminated:
            return False

        try:
            b = self.sock.recv(self.reading_buffer_size)
        except (socket.error, OSError, pyOpenSSLError) as e:
            self.unhandled_error(e)
            return False
        else:
            if not self.process(b):
                return False

        return True

That fix appears to be good enough for the threaded client.

@hyperair
Copy link

hyperair commented Jun 5, 2017

I just spent the last few hours debugging this issue before realizing there was an open issue on it:

hyperair@bf5e1f0

I'll file a PR later.

hyperair added a commit to hyperair/WebSocket-for-Python that referenced this issue Jun 6, 2017
When Frame.parser is given more data than expected, the next frame's header +
contents can end up as part of the payload, causing a UTF-8 validation error, or
ends up being thrown away. This can happen in the SSL case.

This commit fixes that by amending WebSocket.once() to only provide bytes <=
reading_buffer_size to the stream parser and keeping the rest in a buffer,
giving the Frame a chance to stop receiving data at a frame boundary.

Fixes: Lawouach#218
hyperair added a commit to hyperair/WebSocket-for-Python that referenced this issue Jun 6, 2017
When Frame.parser is given more data than expected, the next frame's header +
contents can end up as part of the payload, causing a UTF-8 validation error, or
ends up being thrown away. This can happen in the SSL case if we read slowly and
allow the socket's buffer to fill up a little.

This commit fixes that by amending WebSocket.once() to only provide bytes <=
reading_buffer_size to the stream parser and keeping the rest in a buffer,
giving the Frame a chance to stop receiving data at a frame boundary.

Fixes: Lawouach#218
@hyperair hyperair linked a pull request Jun 6, 2017 that will close this issue
hyperair added a commit to hyperair/WebSocket-for-Python that referenced this issue Jun 6, 2017
When Frame.parser is given more data than expected, the next frame's header +
contents can end up as part of the payload, causing a UTF-8 validation error, or
ends up being thrown away. This can happen in the SSL case if we read slowly and
allow the socket's buffer to fill up a little.

This commit fixes that by amending WebSocket.once() to only provide bytes <=
reading_buffer_size to the stream parser and keeping the rest in a buffer,
giving the Frame a chance to stop receiving data at a frame boundary.

Fixes: Lawouach#218
tsavola pushed a commit to ninchat/ninchat-python that referenced this issue Oct 30, 2017
- ws4py 0.4.2 has a known issue which breaks the client:
  Lawouach/WebSocket-for-Python#218

- gevent 1.1.x has an infinite recursion problem with ssl context
  initialization
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 a pull request may close this issue.

3 participants