From 581f23bc832dc2b095bdd28f0781161d13d2e7a7 Mon Sep 17 00:00:00 2001 From: Peter Adam Date: Wed, 15 Jul 2015 13:15:23 +0200 Subject: [PATCH] feat(req+resp): Add support for Content-Range units other than 'bytes' In RFC 2616 it says that "The only range unit defined by HTTP/1.1 is "bytes". HTTP/1.1 implementations MAY ignore ranges specified using other units.". Current falcon implementation only accepts 'bytes', which does not work with some client implementations using different range units. For example, dojo/store/JsonRest requires the server to support 'items' as range unit. Add a request property 'range_unit', which contains the range-unit parsed from Range header. It enhances format_range to accept a range unit as optional parameter in range tuple. This range unit default is set to 'bytes' to not break current API. Closes #570 --- falcon/request.py | 32 ++++++++++++++++++++++++-------- falcon/response.py | 9 +++++---- falcon/response_helpers.py | 8 ++++++-- tests/test_headers.py | 22 ++++++++++++++++++++-- tests/test_req_vars.py | 28 +++++++++++++++++++++++----- 5 files changed, 78 insertions(+), 21 deletions(-) diff --git a/falcon/request.py b/falcon/request.py index e809bcb..a39e09a 100644 --- a/falcon/request.py +++ b/falcon/request.py @@ -158,6 +158,8 @@ class Request(object): Only continous ranges are supported (e.g., "bytes=0-0,-1" would result in an HTTPBadRequest exception when the attribute is accessed.) + range_unit (str): Unit of the range parsed from the value of the + Range header, or ``None`` if the header is missing if_match (str): Value of the If-Match header, or ``None`` if the header is missing. if_none_match (str): Value of the If-None-Match header, or ``None`` @@ -371,20 +373,20 @@ class Request(object): def range(self): try: value = self.env['HTTP_RANGE'] - if value.startswith('bytes='): - value = value[6:] + if '=' in value: + unit, sep, req_range = value.partition('=') else: - msg = "The value must be prefixed with 'bytes='" + msg = "The value must be prefixed with a range unit, e.g. 'bytes='" raise HTTPInvalidHeader(msg, 'Range') except KeyError: return None - if ',' in value: - msg = 'The value must be a continuous byte range.' + if ',' in req_range: + msg = 'The value must be a continuous range.' raise HTTPInvalidHeader(msg, 'Range') try: - first, sep, last = value.partition('-') + first, sep, last = req_range.partition('-') if not sep: raise ValueError() @@ -394,16 +396,30 @@ class Request(object): elif last: return (-int(last), -1) else: - msg = 'The byte offsets are missing.' + msg = 'The range offsets are missing.' raise HTTPInvalidHeader(msg, 'Range') except ValueError: href = 'http://goo.gl/zZ6Ey' href_text = 'HTTP/1.1 Range Requests' - msg = ('It must be a byte range formatted according to RFC 2616.') + msg = ('It must be a range formatted according to RFC 7233.') raise HTTPInvalidHeader(msg, 'Range', href=href, href_text=href_text) + @property + def range_unit(self): + try: + value = self.env['HTTP_RANGE'] + + if '=' in value: + unit, sep, req_range = value.partition('=') + return unit + else: + msg = "The value must be prefixed with a range unit, e.g. 'bytes='" + raise HTTPInvalidHeader(msg, 'Range') + except KeyError: + return None + @property def app(self): return self.env.get('SCRIPT_NAME', '') diff --git a/falcon/response.py b/falcon/response.py index 293747c..a1cf4db 100644 --- a/falcon/response.py +++ b/falcon/response.py @@ -509,10 +509,11 @@ class Response(object): 'Content-Range', """A tuple to use in constructing a value for the Content-Range header. - The tuple has the form (*start*, *end*, *length*), where *start* and - *end* designate the byte range (inclusive), and *length* is the - total number of bytes, or '\*' if unknown. You may pass ``int``'s for - these numbers (no need to convert to ``str`` beforehand). + The tuple has the form (*start*, *end*, *length*, [*unit*]), where *start* and + *end* designate the range (inclusive), and *length* is the + total length, or '\*' if unknown. You may pass ``int``'s for + these numbers (no need to convert to ``str`` beforehand). The optional value + *unit* describes the range unit and defaults to 'bytes' Note: You only need to use the alternate form, 'bytes \*/1234', for diff --git a/falcon/response_helpers.py b/falcon/response_helpers.py index fd09504..9ffb3e3 100644 --- a/falcon/response_helpers.py +++ b/falcon/response_helpers.py @@ -51,12 +51,16 @@ def format_range(value): Args: value: ``tuple`` passed to `req.range` - """ # PERF: Concatenation is faster than % string formatting as well # as ''.join() in this case. - return ('bytes ' + + if len(value) == 4: + unit = value[3] + ' ' + else: + unit = 'bytes ' + + return (unit + str(value[0]) + '-' + str(value[1]) + '/' + str(value[2])) diff --git a/tests/test_headers.py b/tests/test_headers.py index 0863b98..7b2f82c 100644 --- a/tests/test_headers.py +++ b/tests/test_headers.py @@ -64,8 +64,11 @@ class HeaderHelpersResource: resp.location = '/things/87' resp.content_location = '/things/78' - # bytes 0-499/10240 - resp.content_range = (0, 499, 10 * 1024) + if req.range_unit is None or req.range_unit == 'bytes': + # bytes 0-499/10240 + resp.content_range = (0, 499, 10 * 1024) + else: + resp.content_range = (0, 25, 100, req.range_unit) self.resp = resp @@ -347,6 +350,21 @@ class TestHeaders(testing.TestBase): self.assertIn(('content-range', 'bytes 0-499/10240'), self.srmock.headers) + resp.content_range = (0, 499, 10 * 1024, 'bytes') + self.assertEqual('bytes 0-499/10240', resp.content_range) + self.assertIn(('content-range', 'bytes 0-499/10240'), + self.srmock.headers) + + req_headers = { + 'Range': 'items=0-25', + } + self.simulate_request(self.test_route, headers=req_headers) + + resp.content_range = (0, 25, 100, 'items') + self.assertEqual('items 0-25/100', resp.content_range) + self.assertIn(('content-range', 'items 0-25/100'), + self.srmock.headers) + # Check for duplicate headers hist = defaultdict(lambda: 0) for name, value in self.srmock.headers: diff --git a/tests/test_req_vars.py b/tests/test_req_vars.py index 7e7f414..9e88754 100644 --- a/tests/test_req_vars.py +++ b/tests/test_req_vars.py @@ -403,6 +403,24 @@ class TestReqVars(testing.TestBase): req = Request(testing.create_environ()) self.assertIs(req.range, None) + def test_range_unit(self): + headers = {'Range': 'bytes=10-'} + req = Request(testing.create_environ(headers=headers)) + self.assertEqual(req.range, (10, -1)) + self.assertEqual(req.range_unit, 'bytes') + + headers = {'Range': 'items=10-'} + req = Request(testing.create_environ(headers=headers)) + self.assertEqual(req.range, (10, -1)) + self.assertEqual(req.range_unit, 'items') + + headers = {'Range': ''} + req = Request(testing.create_environ(headers=headers)) + self.assertRaises(falcon.HTTPInvalidHeader, lambda: req.range_unit) + + req = Request(testing.create_environ()) + self.assertIs(req.range_unit, None) + def test_range_invalid(self): headers = {'Range': 'bytes=10240'} req = Request(testing.create_environ(headers=headers)) @@ -410,7 +428,7 @@ class TestReqVars(testing.TestBase): headers = {'Range': 'bytes=-'} expected_desc = ('The value provided for the Range header is ' - 'invalid. The byte offsets are missing.') + 'invalid. The range offsets are missing.') self._test_error_details(headers, 'range', falcon.HTTPInvalidHeader, 'Invalid header value', expected_desc) @@ -461,8 +479,8 @@ class TestReqVars(testing.TestBase): headers = {'Range': 'bytes=x-y'} expected_desc = ('The value provided for the Range header is ' - 'invalid. It must be a byte range formatted ' - 'according to RFC 2616.') + 'invalid. It must be a range formatted ' + 'according to RFC 7233.') self._test_error_details(headers, 'range', falcon.HTTPInvalidHeader, 'Invalid header value', expected_desc) @@ -470,7 +488,7 @@ class TestReqVars(testing.TestBase): headers = {'Range': 'bytes=0-0,-1'} expected_desc = ('The value provided for the Range ' 'header is invalid. The value must be a ' - 'continuous byte range.') + 'continuous range.') self._test_error_details(headers, 'range', falcon.HTTPInvalidHeader, 'Invalid header value', expected_desc) @@ -478,7 +496,7 @@ class TestReqVars(testing.TestBase): headers = {'Range': '10-'} expected_desc = ("The value provided for the Range " "header is invalid. The value must be " - "prefixed with 'bytes='") + "prefixed with a range unit, e.g. 'bytes='") self._test_error_details(headers, 'range', falcon.HTTPInvalidHeader, 'Invalid header value', expected_desc)