SeekableBufferedInputBase.read(0) returns data

[janpom]

Steps to reproduce:

from smart_open.s3 import SeekableBufferedInputBase
input = SeekableBufferedInputBase(...)
assert input.read(0) == b''

I quickly investigated the problem. It seems the bug is in the read() method in the parent BufferedInputBase class:

    def read(self, size=-1):
        """Read up to size bytes from the object and return them."""
        if size <= 0:
            if len(self._buffer):
                from_buf = self._read_from_buffer(len(self._buffer))
            else:
                from_buf = b''
            self._current_pos = self._content_length
            return from_buf + self._raw_reader.read()

       ...

if size < 0 maybe?

[menshikh-iv]

CC: @mpenkov

[janpom]

A quick workaround is to subclass SeekableBufferedInputBase and handle the special case correctly:

from smart_open.s3 import SeekableBufferedInputBase

class S3SeekableInput(SeekableBufferedInputBase):
    def read(self, size=-1):
        if size == 0:
            return b''
        else:
            return super(S3SeekableInput, self).read(size)

[mpenkov]

@janpom I think that fixing the superclass' method is preferable to the override. Are you able to submit a PR? If not, I'll look into it during the week.

[janpom]

@mpenkov I wasn't suggesting the workaround as a fix. I was merely pointing out that users can work around the problem (outside of smart_open code).

The fix should definitely be be in the BufferedInputBase. Some more investigation would be worthwhile. The size < 0 is just my wild guess. I don't really have time for delivering a proper fix, sorry.

[mpenkov]

That's OK, I understand. Thanks for pointing this out. I'll take it from here.

…

On Wed, Feb 14, 2018 at 1:25 AM Jan Pomikálek ***@***.***> wrote: @mpenkov <https://github.com/mpenkov> I wasn't suggesting the workaround as a fix. I was merely pointing out that users can work around the problem (outside of smart_open code). The fix should definitely be be in the BufferedInputBase. Some more investigation would be worthwhile. The size < 0 is just my wild guess. I don't really have time for delivering a proper fix, sorry. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#171 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABDOVF4CBt1K9MPYROxglxClIMmF4RtQks5tUbdygaJpZM4SDas9> .