make HTTP/S seeking less strict

smart_open/http.py

@@ -241,13 +241,14 @@ def __init__(self, url, mode='r', buffer_size=DEFAULT_BUFFER_SIZE,
 
         logger.debug('self.response: %r, raw: %r', self.response, self.response.raw)
 
-        self._seekable = True
-
         self.content_length = int(self.response.headers.get("Content-Length", -1))
-        if self.content_length < 0:
-            self._seekable = False
-        if self.response.headers.get("Accept-Ranges", "none").lower() != "bytes":
-            self._seekable = False
+        #
+        # We assume the HTTP stream is seekable unless the server explicitly
+        # tells us it isn't.  It's better to err on the side of "seekable"
+        # because we don't want to prevent users from seeking a stream that
+        # does not appear to be seekable but really is.
+        #
+        self._seekable = self.response.headers.get("Accept-Ranges", "").lower() != "none"
 
         self._read_iter = self.response.iter_content(self.buffer_size)
         self._read_buffer = bytebuffer.ByteBuffer(buffer_size)
@@ -270,7 +271,7 @@ def seek(self, offset, whence=0):
             raise ValueError('invalid whence, expected one of %r' % constants.WHENCE_CHOICES)
 
         if not self.seekable():
-            raise OSError
+            raise OSError('stream is not seekable')
 
         if whence == constants.WHENCE_START:
             new_pos = offset
@@ -279,7 +280,8 @@ def seek(self, offset, whence=0):
         elif whence == constants.WHENCE_END:
             new_pos = self.content_length + offset
 
-        new_pos = smart_open.utils.clamp(new_pos, 0, self.content_length)
+        if self.content_length != -1:
+            new_pos = smart_open.utils.clamp(new_pos, 0, self.content_length)
 
         if self._current_pos == new_pos:
             return self._current_pos

smart_open/tests/test_http.py

@@ -5,9 +5,11 @@
 # This code is distributed under the terms and conditions
 # from the MIT License (MIT).
 #
+import functools
 import os
 import unittest
 
+import pytest
 import responses
 
 import smart_open.http
@@ -24,19 +26,19 @@
 }
 
 
-def request_callback(request):
+def request_callback(request, headers=HEADERS, data=BYTES):
     try:
         range_string = request.headers['range']
     except KeyError:
-        return (200, HEADERS, BYTES)
+        return (200, headers, data)
 
     start, end = range_string.replace('bytes=', '').split('-', 1)
     start = int(start)
     if end:
         end = int(end)
     else:
-        end = len(BYTES)
-    return (200, HEADERS, BYTES[start:end])
+        end = len(data)
+    return (200, headers, data[start:end])
 
 
 @unittest.skipIf(os.environ.get('TRAVIS'), 'This test does not work on TravisCI for some reason')
@@ -158,3 +160,28 @@ def test_timeout_attribute(self):
         reader = smart_open.open(URL, "rb", transport_params={'timeout': timeout})
         assert hasattr(reader, 'timeout')
         assert reader.timeout == timeout
+
+
+@responses.activate
+def test_seek_implicitly_enabled(numbytes=10):
+    """Can we seek even if the server hasn't explicitly allowed it?"""
+    callback = functools.partial(request_callback, headers={})
+    responses.add_callback(responses.GET, HTTPS_URL, callback=callback)
+    with smart_open.open(HTTPS_URL, 'rb') as fin:
+        assert fin.seekable()
+        first = fin.read(size=numbytes)
+        fin.seek(-numbytes, whence=smart_open.constants.WHENCE_CURRENT)
+        second = fin.read(size=numbytes)
+        assert first == second
+
+
+@responses.activate
+def test_seek_implicitly_disabled():
+    """Does seeking fail when the server has explicitly disabled it?"""
+    callback = functools.partial(request_callback, headers={'Accept-Ranges': 'none'})
+    responses.add_callback(responses.GET, HTTPS_URL, callback=callback)
+    with smart_open.open(HTTPS_URL, 'rb') as fin:
+        assert not fin.seekable()
+        fin.read()
+        with pytest.raises(OSError):
+            fin.seek(0)

smart_open/utils.py

@@ -119,7 +119,9 @@ def make_range_string(start=None, stop=None):
     #
     if start is None and stop is None:
         raise ValueError("make_range_string requires either a stop or start value")
-    return 'bytes=%s-%s' % ('' if start is None else start, '' if stop is None else stop)
+    start_str = '' if start is None else str(start)
+    stop_str = '' if stop is None else str(stop)
+    return 'bytes=%s-%s' % (start_str, stop_str)
 
 
 def parse_content_range(content_range):