Merge "Fix two edge cases with Range: header"

swift/common/swob.py

@@ -433,6 +433,13 @@ class Range(object):
     After initialization, "range.ranges" is populated with a list
     of (start, end) tuples denoting the requested ranges.
 
+    If there were any syntactically-invalid byte-range-spec values,
+    "range.ranges" will be an empty list, per the relevant RFC:
+
+    "The recipient of a byte-range-set that includes one or more syntactically
+    invalid byte-range-spec values MUST ignore the header field that includes
+    that byte-range-set."
+
     :param headerval: value of the header as a str
     """
     def __init__(self, headerval):
@@ -448,6 +455,13 @@ class Range(object):
                 start = None
             if end:
                 end = int(end)
+                if start is not None and not end >= start:
+                    # If the last-byte-pos value is present, it MUST be greater
+                    # than or equal to the first-byte-pos in that
+                    # byte-range-spec, or the byte- range-spec is syntactically
+                    # invalid.  [which "MUST" be ignored]
+                    self.ranges = []
+                    break
             else:
                 end = None
             self.ranges.append((start, end))
@@ -478,14 +492,20 @@ class Range(object):
         begin, end = self.ranges[0]
         if begin is None:
             if end == 0:
-                return (0, length)
-            if end > length:
                 return None
+            if end > length:
+                return (0, length)
             return (length - end, length)
         if end is None:
-            if begin == 0:
+            if begin < length:
-                return (0, length)
+                # If a syntactically valid byte-range-set includes at least one
+                # byte-range-spec whose first-byte-pos is LESS THAN THE CURRENT
+                # LENGTH OF THE ENTITY-BODY..., then the byte-range-set is
+                # satisfiable.
                 return (begin, length)
+            else:
+                # Otherwise, the byte-range-set is unsatisfiable.
+                return None
         if begin > length:
             return None
         return (begin, min(end + 1, length))
@@ -767,12 +787,16 @@ class Response(object):
 
     def _response_iter(self, app_iter, body):
         if self.request and self.request.method == 'HEAD':
+            # We explicitly do NOT want to set self.content_length to 0 here
             return ['']
         if self.conditional_response and self.request and \
-                self.request.range and not self.content_range:
+                self.request.range and self.request.range.ranges and \
+                not self.content_range:
             args = self.request.range.range_for_length(self.content_length)
             if not args:
                 self.status = 416
+                self.content_length = 0
+                return ['']
             else:
                 start, end = args
                 self.status = 206

test/functional/tests.py

@@ -1128,6 +1128,15 @@ class TestFile(Base):
 
             range_string = 'bytes=-%d' % (i)
             hdrs = {'Range': range_string}
+            if i == 0:
+                # RFC 2616 14.35.1
+                # "If a syntactically valid byte-range-set includes ... at
+                # least one suffix-byte-range-spec with a NON-ZERO
+                # suffix-length, then the byte-range-set is satisfiable.
+                # Otherwise, the byte-range-set is unsatisfiable.
+                self.assertRaises(ResponseError, file.read, hdrs=hdrs)
+                self.assert_status(416)
+            else:
                 self.assertEquals(file.read(hdrs=hdrs), data[-i:])
 
             range_string = 'bytes=%d-' % (i)
@@ -1147,6 +1156,13 @@ class TestFile(Base):
         hdrs = {'Range': '0-4'}
         self.assert_(file.read(hdrs=hdrs) == data, range_string)
 
+        # RFC 2616 14.35.1
+        # "If the entity is shorter than the specified suffix-length, the
+        # entire entity-body is used."
+        range_string = 'bytes=-%d' % (file_length + 10)
+        hdrs = {'Range': range_string}
+        self.assert_(file.read(hdrs=hdrs) == data, range_string)
+
     def testRangedGetsWithLWSinHeader(self):
         #Skip this test until webob 1.2 can tolerate LWS in Range header.
         file_length = 10000

test/unit/common/test_swob.py

@@ -127,13 +127,21 @@ class TestRange(unittest.TestCase):
         self.assertEquals(range.range_for_length(10), (1, 10))
         self.assertEquals(range.range_for_length(5), (1, 5))
         self.assertEquals(range.range_for_length(None), None)
+        # This used to freak out:
+        range = swift.common.swob.Range('bytes=100-')
+        self.assertEquals(range.range_for_length(5), None)
+        self.assertEquals(range.range_for_length(None), None)
 
     def test_range_for_length_no_start(self):
         range = swift.common.swob.Range('bytes=-7')
         self.assertEquals(range.range_for_length(10), (3, 10))
-        self.assertEquals(range.range_for_length(5), None)
+        self.assertEquals(range.range_for_length(5), (0, 5))
         self.assertEquals(range.range_for_length(None), None)
 
+    def test_range_invalid_syntax(self):
+        range = swift.common.swob.Range('bytes=10-2')
+        self.assertEquals(range.ranges, [])
+
 
 class TestMatch(unittest.TestCase):
     def test_match(self):
@@ -370,12 +378,47 @@ class TestResponse(unittest.TestCase):
         resp.conditional_response = True
         body = ''.join(resp([], start_response))
         self.assertEquals(body, '234')
+        self.assertEquals(resp.status, '206 Partial Content')
 
         resp = swift.common.swob.Response(
             body='1234567890', request=req,
             conditional_response=True)
         body = ''.join(resp([], start_response))
         self.assertEquals(body, '234')
+        self.assertEquals(resp.status, '206 Partial Content')
+
+        # No body for 416
+        req = swift.common.swob.Request.blank(
+            '/', headers={'Range': 'bytes=-0'})
+        resp = req.get_response(test_app)
+        resp.conditional_response = True
+        body = ''.join(resp([], start_response))
+        self.assertEquals(body, '')
+        self.assertEquals(resp.content_length, 0)
+        self.assertEquals(resp.status, '416 Request Range Not Satisfiable')
+
+        resp = swift.common.swob.Response(
+            body='1234567890', request=req,
+            conditional_response=True)
+        body = ''.join(resp([], start_response))
+        self.assertEquals(body, '')
+        self.assertEquals(resp.status, '416 Request Range Not Satisfiable')
+
+        # Syntactically-invalid Range headers "MUST" be ignored
+        req = swift.common.swob.Request.blank(
+            '/', headers={'Range': 'bytes=3-2'})
+        resp = req.get_response(test_app)
+        resp.conditional_response = True
+        body = ''.join(resp([], start_response))
+        self.assertEquals(body, '1234567890')
+        self.assertEquals(resp.status, '200 OK')
+
+        resp = swift.common.swob.Response(
+            body='1234567890', request=req,
+            conditional_response=True)
+        body = ''.join(resp([], start_response))
+        self.assertEquals(body, '1234567890')
+        self.assertEquals(resp.status, '200 OK')
 
     def test_content_type(self):
         resp = self._get_response()