From e2b03267fd0f33e46d21d938f2d19085f4bb3e21 Mon Sep 17 00:00:00 2001 From: Darrell Bishop Date: Wed, 3 Oct 2012 14:20:52 -0700 Subject: [PATCH] Fix two edge cases with Range: header This fixes swob to handle "Range: bytes=-X" where X > len(content); ex. "Range: bytes=-17" when the object has 10 bytes. Based on the RFC, the range is satisfiable and all bytes should be returned. It also fixes "Range: bytes=-0" to be, correctly, not satisfiable. In addition, this case's response has Content-Length: 0 and has a zero-byte body. It also fixes an existing regression in swob for the case "Range: bytes=100-" for a body of length < 100 (Content-Length was negative and the body was returned). The relevant RFC is 2616, section 14.35.1. Change-Id: Ib3dc672e083173eb970c10801283813623f26e0e --- swift/common/swob.py | 36 +++++++++++++++++++++++----- test/functional/tests.py | 18 +++++++++++++- test/unit/common/test_swob.py | 45 ++++++++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/swift/common/swob.py b/swift/common/swob.py index 49cceb71ec..7f6a31590f 100644 --- a/swift/common/swob.py +++ b/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: - return (0, length) - return (begin, length) + if begin < 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)) @@ -766,12 +786,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 diff --git a/test/functional/tests.py b/test/functional/tests.py index cc87954448..852dee88b0 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -1128,7 +1128,16 @@ class TestFile(Base): range_string = 'bytes=-%d' % (i) hdrs = {'Range': range_string} - self.assertEquals(file.read(hdrs=hdrs), data[-i:]) + 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) hdrs = {'Range': range_string} @@ -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 diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index b3d2329bad..e5ffb81a8c 100644 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -126,13 +126,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): @@ -359,12 +367,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()