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

_fileobject.read() is pretty badly broken #18

Open
jim-minter opened this issue Jan 19, 2016 · 1 comment
Open

_fileobject.read() is pretty badly broken #18

jim-minter opened this issue Jan 19, 2016 · 1 comment

Comments

@jim-minter
Copy link

There are several significant issues here:

  1. Depending on timing, when read() is called and the remote end terminates its SSL connection cleanly, SSLZeroReturnError will be raised to the user and the final bytes of the stream are lost forever.

                data = _safe_ssl_call(False, self._sock, 'recv', maxbufsize)
                if not data:
                    break
    

    In the above snippet, the second line will not execute when SSLZeroReturnError is raised. data is a local variable and its contents are lost.

  2. There are identical code patterns which need fixing in _fileobject.readline() as well (maybe other places too, I'm not sure).

  3. In the case size < 0, read() does not keep the contract of reading until EOF.

        if size < 0:
            # Read until EOF
            self._rbuf = BytesIO()  # reset _rbuf.  we consume it via buf.
            data = _safe_ssl_call(False, self._sock, 'recv', rbufsize)
            buf.write(data)
            return buf.getvalue()
    

    In the above snippet, the comment is mistaken. There would have to be a loop for it to be true. No more than rbufsize additional bytes are read from the channel.

@benlawraus
Copy link

Hi
Is this related?
mjs/imapclient#268

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

No branches or pull requests

2 participants