From 3ba96f6aee80b8f77d0f1a752b6200d4fccbcecd Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 13:31:40 +1100 Subject: [PATCH 01/19] Add support for http range header in static file handling. Generally follows https://tools.ietf.org/html/rfc7233 however does not currently support multiple ranges in one request. --- aiohttp/file_sender.py | 43 +++++++++++-- tests/test_web_sendfile_functional.py | 90 ++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 6 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index e32821b3141..2fd0f37bcc8 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -1,6 +1,7 @@ import asyncio import mimetypes import os +import re from . import hdrs from .helpers import create_future @@ -152,16 +153,48 @@ def send(self, request, filepath): if not ct: ct = 'application/octet-stream' - resp = self._response_factory() + file_size = st.st_size + + status = 200 + start = 0 + end = file_size - 1 + + # Handle 206 range response if requested + if 'range' in request.headers: + from .web_exceptions import HTTPPartialContent, HTTPRequestRangeNotSatisfiable + status = HTTPPartialContent.status_code + + range_header = request.headers['range'].lower() + if not range_header.startswith('bytes='): + raise HTTPRequestRangeNotSatisfiable + + try: + range_start, range_end = re.findall(r'bytes=(\d*)-(\d*)', request.headers['range'].lower())[0] + except IndexError: + raise HTTPRequestRangeNotSatisfiable + if range_start and range_end: + start = int(range_start) + end = int(range_end) + elif range_start and not range_end: + start = int(range_start) + elif range_end and not range_start: + start = file_size - int(range_end) + else: + raise HTTPRequestRangeNotSatisfiable + + if start >= file_size or end >= file_size or start > end: + raise HTTPRequestRangeNotSatisfiable + + resp = self._response_factory(status=status) resp.content_type = ct if encoding: resp.headers[hdrs.CONTENT_ENCODING] = encoding resp.last_modified = st.st_mtime - file_size = st.st_size - - resp.content_length = file_size + count = end - start + 1 + resp.content_length = count with filepath.open('rb') as f: - yield from self._sendfile(request, resp, f, file_size) + f.seek(start) + yield from self._sendfile(request, resp, f, count) return resp diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index b8a3d9dedc5..88480d7c5b5 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -1,7 +1,6 @@ import asyncio import os import pathlib - import pytest import aiohttp @@ -323,3 +322,92 @@ def test_static_file_huge(loop, test_client, tmpdir): off += len(chunk) cnt += 1 f.close() + + +@asyncio.coroutine +def test_static_file_range(loop, test_client, sender): + filepath = (pathlib.Path(__file__).parent / + 'software_development_in_picture.jpg') + + @asyncio.coroutine + def handler(request): + resp = yield from sender(chunk_size=16).send(request, filepath) + return resp + + app = web.Application(loop=loop) + app.router.add_get('/', handler) + client = yield from test_client(lambda loop: app) + + with filepath.open('rb') as f: + content = f.read() + + # Ensure the whole file requested in parts is correct + resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) + assert resp.status == 206 + body1 = yield from resp.read() + assert len(body1) == 1000 + resp.close() + + resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) + assert resp.status == 206 + body1 = yield from resp.read() + assert len(body1) == 1000 + resp.close() + + resp = yield from client.get('/', headers={'Range': 'bytes=1000-1999'}) + assert resp.status == 206 + body2 = yield from resp.read() + assert len(body1) == 1000 + resp.close() + + resp = yield from client.get('/', headers={'Range': 'bytes=2000-'}) + assert resp.status == 206 + body3 = yield from resp.read() + resp.close() + + assert content == (body1 + body2 + body3) + + # Ensure the tail of the file is correct + resp = yield from client.get('/', headers={'Range': 'bytes=-500'}) + assert resp.status == 206 + body4 = yield from resp.read() + resp.close() + assert content[-500:] == body4 + + + +@asyncio.coroutine +def test_static_file_invalid_range(loop, test_client, sender): + filepath = (pathlib.Path(__file__).parent / + 'software_development_in_picture.jpg') + + @asyncio.coroutine + def handler(request): + resp = yield from sender(chunk_size=16).send(request, filepath) + return resp + + app = web.Application(loop=loop) + app.router.add_get('/', handler) + client = yield from test_client(lambda loop: app) + + file_len = filepath.stat().st_size + + # range must be in bytes + resp = yield from client.get('/', headers={'Range': 'blocks=0-10'}) + assert resp.status == 416, 'Range must be in bytes' + resp.close() + + # Range end is inclusive + resp = yield from client.get('/', headers={'Range': 'bytes=0-%d' % file_len}) + assert resp.status == 416, 'Range end must be inclusive' + resp.close() + + # start > end + resp = yield from client.get('/', headers={'Range': 'bytes=10-5'}) + assert resp.status == 416, "Range start can't be greater than end" + resp.close() + + # non-number range + resp = yield from client.get('/', headers={'Range': 'bytes=a-f'}) + assert resp.status == 416, 'Range must be integers' + resp.close() From 3ddd69d289958b0267a01058b5283f8719fefdee Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 14:17:23 +1100 Subject: [PATCH 02/19] Add details to changelog and update contributors --- CHANGES.rst | 2 ++ CONTRIBUTORS.txt | 1 + 2 files changed, 3 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 70f9f0bd0ba..b1b1e2d72a9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,8 @@ CHANGES ======= +- Added support for HTTP Range Requests in static file handling + 1.1.1 (2016-11-04) ------------------ diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 4e9278340d9..cde5bf3cac7 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -21,6 +21,7 @@ Alex Lisovoy Amy Boyle Andrei Ursulenko Andrej Antonov +Andrew Leech Andrew Svetlov Andrii Soldatenko Anton Kasyanov From 5fe0de363a46d877ba87ba9dc83ec71a83577ccd Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 14:38:02 +1100 Subject: [PATCH 03/19] Update CHANGES.rst Add PR number to change --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index b1b1e2d72a9..6667644cb0a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ CHANGES ======= -- Added support for HTTP Range Requests in static file handling +- Added support for HTTP Range Requests in static file handling #1382 1.1.1 (2016-11-04) ------------------ From bb9c9092d6ecb9b36a36612dc08649dfaf497875 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 14:41:54 +1100 Subject: [PATCH 04/19] Update CHANGES.rst Revert changelog update to allow tests to run --- CHANGES.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6667644cb0a..70f9f0bd0ba 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,8 +1,6 @@ CHANGES ======= -- Added support for HTTP Range Requests in static file handling #1382 - 1.1.1 (2016-11-04) ------------------ From 7b477f3e28881b587172a07dde83b6e06990fb67 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 15:21:10 +1100 Subject: [PATCH 05/19] Clean up imports and formatting --- aiohttp/file_sender.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index 2fd0f37bcc8..f8e85b1fd90 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -6,7 +6,8 @@ from . import hdrs from .helpers import create_future from .web_reqrep import StreamResponse - +from .web_exceptions import HTTPPartialContent, \ + HTTPRequestRangeNotSatisfiable, HTTPNotModified class FileSender: """"A helper that can be used to send files. @@ -146,7 +147,6 @@ def send(self, request, filepath): modsince = request.if_modified_since if modsince is not None and st.st_mtime <= modsince.timestamp(): - from .web_exceptions import HTTPNotModified raise HTTPNotModified() ct, encoding = mimetypes.guess_type(str(filepath)) @@ -161,7 +161,6 @@ def send(self, request, filepath): # Handle 206 range response if requested if 'range' in request.headers: - from .web_exceptions import HTTPPartialContent, HTTPRequestRangeNotSatisfiable status = HTTPPartialContent.status_code range_header = request.headers['range'].lower() @@ -169,7 +168,8 @@ def send(self, request, filepath): raise HTTPRequestRangeNotSatisfiable try: - range_start, range_end = re.findall(r'bytes=(\d*)-(\d*)', request.headers['range'].lower())[0] + pattern = r'bytes=(\d*)-(\d*)' + range_start, range_end = re.findall(pattern, range_header)[0] except IndexError: raise HTTPRequestRangeNotSatisfiable if range_start and range_end: From 943a3f64f172349c55647d1c8cb4124fcf40bbcf Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 15:34:21 +1100 Subject: [PATCH 06/19] Refactor range header parsing into a helpers function for easy reuse --- aiohttp/file_sender.py | 36 +++--------------------------------- aiohttp/helpers.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index f8e85b1fd90..081944bcf9b 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -4,10 +4,8 @@ import re from . import hdrs -from .helpers import create_future +from .helpers import create_future, parse_range_header from .web_reqrep import StreamResponse -from .web_exceptions import HTTPPartialContent, \ - HTTPRequestRangeNotSatisfiable, HTTPNotModified class FileSender: """"A helper that can be used to send files. @@ -147,6 +145,7 @@ def send(self, request, filepath): modsince = request.if_modified_since if modsince is not None and st.st_mtime <= modsince.timestamp(): + from .web_exceptions import HTTPNotModified raise HTTPNotModified() ct, encoding = mimetypes.guess_type(str(filepath)) @@ -154,36 +153,7 @@ def send(self, request, filepath): ct = 'application/octet-stream' file_size = st.st_size - - status = 200 - start = 0 - end = file_size - 1 - - # Handle 206 range response if requested - if 'range' in request.headers: - status = HTTPPartialContent.status_code - - range_header = request.headers['range'].lower() - if not range_header.startswith('bytes='): - raise HTTPRequestRangeNotSatisfiable - - try: - pattern = r'bytes=(\d*)-(\d*)' - range_start, range_end = re.findall(pattern, range_header)[0] - except IndexError: - raise HTTPRequestRangeNotSatisfiable - if range_start and range_end: - start = int(range_start) - end = int(range_end) - elif range_start and not range_end: - start = int(range_start) - elif range_end and not range_start: - start = file_size - int(range_end) - else: - raise HTTPRequestRangeNotSatisfiable - - if start >= file_size or end >= file_size or start > end: - raise HTTPRequestRangeNotSatisfiable + status, start, end = parse_range_header(request, file_size) resp = self._response_factory(status=status) resp.content_type = ct diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 53d155efd7f..c61f2bc2aea 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -19,6 +19,8 @@ from multidict import MultiDict, MultiDictProxy from . import hdrs +from .web_exceptions import HTTPOk, HTTPPartialContent, \ + HTTPRequestRangeNotSatisfiable try: from asyncio import ensure_future @@ -667,3 +669,39 @@ def content_length(self, *, _CONTENT_LENGTH=hdrs.CONTENT_LENGTH): def check_loop(loop): if loop is None: loop = asyncio.get_event_loop() + + +def parse_range_header(request, content_size): + ret = namedtuple('range_request', ['status', 'start', 'end']) + + status = 200 + start = 0 + end = content_size - 1 + + # Handle 206 range response if requested + if 'range' in request.headers: + status = HTTPPartialContent.status_code + + range_header = request.headers['range'].lower() + if not range_header.startswith('bytes='): + raise HTTPRequestRangeNotSatisfiable + + try: + pattern = r'bytes=(\d*)-(\d*)' + range_start, range_end = re.findall(pattern, range_header)[0] + except IndexError: + raise HTTPRequestRangeNotSatisfiable + if range_start and range_end: + start = int(range_start) + end = int(range_end) + elif range_start and not range_end: + start = int(range_start) + elif range_end and not range_start: + start = content_size - int(range_end) + else: + raise HTTPRequestRangeNotSatisfiable + + if start >= content_size or end >= content_size or start > end: + raise HTTPRequestRangeNotSatisfiable + + return ret(status, start, end) From 6d0f017941aeb6da2fa93612a5eff535dad6a81f Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 15:38:23 +1100 Subject: [PATCH 07/19] Fix flake errors --- aiohttp/file_sender.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index 081944bcf9b..758b688bbfd 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -1,12 +1,12 @@ import asyncio import mimetypes import os -import re from . import hdrs from .helpers import create_future, parse_range_header from .web_reqrep import StreamResponse + class FileSender: """"A helper that can be used to send files. """ From f82a5ef0815bfc740a75d5c75b0a8ba992571ded Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 15:41:35 +1100 Subject: [PATCH 08/19] Use descriptive status code --- aiohttp/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index c61f2bc2aea..9ef25409009 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -674,7 +674,7 @@ def check_loop(loop): def parse_range_header(request, content_size): ret = namedtuple('range_request', ['status', 'start', 'end']) - status = 200 + status = HTTPOk.status_code start = 0 end = content_size - 1 From 7575b2137a2a138d18ac14744d269a992517c9c2 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 16:23:34 +1100 Subject: [PATCH 09/19] Fix import error and line length in test --- aiohttp/helpers.py | 5 +++-- tests/test_web_sendfile_functional.py | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 9ef25409009..c1406cb1404 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -19,8 +19,6 @@ from multidict import MultiDict, MultiDictProxy from . import hdrs -from .web_exceptions import HTTPOk, HTTPPartialContent, \ - HTTPRequestRangeNotSatisfiable try: from asyncio import ensure_future @@ -673,6 +671,9 @@ def check_loop(loop): def parse_range_header(request, content_size): ret = namedtuple('range_request', ['status', 'start', 'end']) + # Import here to avoid circular dependency on imports + from .web_exceptions import (HTTPOk, HTTPPartialContent, + HTTPRequestRangeNotSatisfiable) status = HTTPOk.status_code start = 0 diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 88480d7c5b5..ea33480f439 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -340,7 +340,7 @@ def handler(request): with filepath.open('rb') as f: content = f.read() - + # Ensure the whole file requested in parts is correct resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) assert resp.status == 206 @@ -375,7 +375,6 @@ def handler(request): assert content[-500:] == body4 - @asyncio.coroutine def test_static_file_invalid_range(loop, test_client, sender): filepath = (pathlib.Path(__file__).parent / @@ -390,7 +389,7 @@ def handler(request): app.router.add_get('/', handler) client = yield from test_client(lambda loop: app) - file_len = filepath.stat().st_size + flen = filepath.stat().st_size # range must be in bytes resp = yield from client.get('/', headers={'Range': 'blocks=0-10'}) @@ -398,7 +397,7 @@ def handler(request): resp.close() # Range end is inclusive - resp = yield from client.get('/', headers={'Range': 'bytes=0-%d' % file_len}) + resp = yield from client.get('/', headers={'Range': 'bytes=0-%d' % flen}) assert resp.status == 416, 'Range end must be inclusive' resp.close() From 96f92f21c767114a93e1e75682344ddcf5103cf3 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 16:43:48 +1100 Subject: [PATCH 10/19] Fix range support in _sendfile_system() --- aiohttp/file_sender.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index 758b688bbfd..9136005462c 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -81,8 +81,8 @@ def write_eof(): # See https://github.com/KeepSafe/aiohttp/issues/958 for details # send headers - headers = ['HTTP/{0.major}.{0.minor} 200 OK\r\n'.format( - request.version)] + headers = ['HTTP/{0.major}.{0.minor} {1} OK\r\n'.format( + request.version, resp.status)] for hdr, val in resp.headers.items(): headers.append('{}: {}\r\n'.format(hdr, val)) headers.append('\r\n') @@ -91,6 +91,7 @@ def write_eof(): out_socket.setblocking(False) out_fd = out_socket.fileno() in_fd = fobj.fileno() + offset = fobj.tell() bheaders = ''.join(headers).encode('utf-8') headers_length = len(bheaders) @@ -100,7 +101,7 @@ def write_eof(): try: yield from loop.sock_sendall(out_socket, bheaders) fut = create_future(loop) - self._sendfile_cb(fut, out_fd, in_fd, 0, count, loop, False) + self._sendfile_cb(fut, out_fd, in_fd, offset, count, loop, False) yield from fut finally: From 134798519d4361eddf20cd792f00a576d422b9f3 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 8 Nov 2016 17:43:21 +1100 Subject: [PATCH 11/19] Give more descriptive error text Remove duplicate code and fix test --- tests/test_web_sendfile_functional.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index ea33480f439..ec1b9a6ecb2 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -343,25 +343,19 @@ def handler(request): # Ensure the whole file requested in parts is correct resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) - assert resp.status == 206 - body1 = yield from resp.read() - assert len(body1) == 1000 - resp.close() - - resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) - assert resp.status == 206 + assert resp.status == 206, resp.reason body1 = yield from resp.read() assert len(body1) == 1000 resp.close() resp = yield from client.get('/', headers={'Range': 'bytes=1000-1999'}) - assert resp.status == 206 + assert resp.status == 206, resp.reason body2 = yield from resp.read() - assert len(body1) == 1000 + assert len(body2) == 1000 resp.close() resp = yield from client.get('/', headers={'Range': 'bytes=2000-'}) - assert resp.status == 206 + assert resp.status == 206, resp.reason body3 = yield from resp.read() resp.close() @@ -369,7 +363,7 @@ def handler(request): # Ensure the tail of the file is correct resp = yield from client.get('/', headers={'Range': 'bytes=-500'}) - assert resp.status == 206 + assert resp.status == 206, resp.reason body4 = yield from resp.read() resp.close() assert content[-500:] == body4 From 6e3a09ff332db1054ac5cab200091180d80da8f5 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Thu, 10 Nov 2016 16:06:23 +1100 Subject: [PATCH 12/19] Refactor the core parsing of the range header into a property on Request(), returns (start,end) in format ready to use in a slice of a string/bytearray for convenient reuse Handling specific to file offset and length calculations brought back into send() of FileSender. Similar handling could be implemented in other request handlers depending on type of data being requested. --- aiohttp/file_sender.py | 34 ++++++++++++++++++++++++++++------ aiohttp/helpers.py | 39 --------------------------------------- aiohttp/web_reqrep.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index 9136005462c..b82d93a33bc 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -3,9 +3,10 @@ import os from . import hdrs -from .helpers import create_future, parse_range_header +from .helpers import create_future from .web_reqrep import StreamResponse - +from .web_exceptions import (HTTPOk, HTTPPartialContent, + HTTPRequestRangeNotSatisfiable, HTTPNotModified) class FileSender: """"A helper that can be used to send files. @@ -146,15 +147,36 @@ def send(self, request, filepath): modsince = request.if_modified_since if modsince is not None and st.st_mtime <= modsince.timestamp(): - from .web_exceptions import HTTPNotModified raise HTTPNotModified() ct, encoding = mimetypes.guess_type(str(filepath)) if not ct: ct = 'application/octet-stream' + status = HTTPOk.status_code file_size = st.st_size - status, start, end = parse_range_header(request, file_size) + count = file_size + + try: + start, end = request.http_range + except ValueError: + raise HTTPRequestRangeNotSatisfiable + + # If a range request has been made, convert start, end slice notation + # into file pointer offset and count + if start is not end is not None: + status = HTTPPartialContent.status_code + if start is None: # return tail of file + if end < 0: + start = file_size + end + count = -end + else: + raise HTTPRequestRangeNotSatisfiable + else: + count = (end or file_size) - start + + if start + count > file_size: + raise HTTPRequestRangeNotSatisfiable resp = self._response_factory(status=status) resp.content_type = ct @@ -162,10 +184,10 @@ def send(self, request, filepath): resp.headers[hdrs.CONTENT_ENCODING] = encoding resp.last_modified = st.st_mtime - count = end - start + 1 resp.content_length = count with filepath.open('rb') as f: - f.seek(start) + if start: + f.seek(start) yield from self._sendfile(request, resp, f, count) return resp diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index c1406cb1404..53d155efd7f 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -667,42 +667,3 @@ def content_length(self, *, _CONTENT_LENGTH=hdrs.CONTENT_LENGTH): def check_loop(loop): if loop is None: loop = asyncio.get_event_loop() - - -def parse_range_header(request, content_size): - ret = namedtuple('range_request', ['status', 'start', 'end']) - # Import here to avoid circular dependency on imports - from .web_exceptions import (HTTPOk, HTTPPartialContent, - HTTPRequestRangeNotSatisfiable) - - status = HTTPOk.status_code - start = 0 - end = content_size - 1 - - # Handle 206 range response if requested - if 'range' in request.headers: - status = HTTPPartialContent.status_code - - range_header = request.headers['range'].lower() - if not range_header.startswith('bytes='): - raise HTTPRequestRangeNotSatisfiable - - try: - pattern = r'bytes=(\d*)-(\d*)' - range_start, range_end = re.findall(pattern, range_header)[0] - except IndexError: - raise HTTPRequestRangeNotSatisfiable - if range_start and range_end: - start = int(range_start) - end = int(range_end) - elif range_start and not range_end: - start = int(range_start) - elif range_end and not range_start: - start = content_size - int(range_end) - else: - raise HTTPRequestRangeNotSatisfiable - - if start >= content_size or end >= content_size or start > end: - raise HTTPRequestRangeNotSatisfiable - - return ret(status, start, end) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index d5c6a3756a9..2c466b97f3e 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -10,6 +10,7 @@ import math import time import warnings +import re from email.utils import parsedate from types import MappingProxyType @@ -310,6 +311,35 @@ def cookies(self): return MappingProxyType( {key: val.value for key, val in parsed.items()}) + @property + def http_range(self, *, _RANGE=hdrs.RANGE): + """ + The content of Range HTTP header. + :returns tuple (start, end): values that can be used for slice eg. content[start:end] + """ + rng = self.headers.get(_RANGE) + start, end = None, None + if rng is not None: + try: + pattern = r'bytes=(\d*)-(\d*)' + start, end = re.findall(pattern, rng)[0] + except IndexError: # pattern was not found in header + raise ValueError("range not in acceptible format") + + if end is not None: + end = int(end) + if start is None: + end = -end # end with no start is to return tail of content + + if start is not None: + start = int(start) + if end is not None: + end += 1 # end is inclusive in range header, exclusive in slice + + if start is end is None: # No valid range supplied + raise ValueError('No start or end of range specified') + return start, end + @property def content(self): """Return raw payload stream.""" From fec91625f857d537d4d255f5ee9a86e1332e8a9b Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 09:23:16 +1100 Subject: [PATCH 13/19] Clean up flake8 violations --- aiohttp/file_sender.py | 3 ++- aiohttp/web_reqrep.py | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index b82d93a33bc..8856f56ca29 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -8,6 +8,7 @@ from .web_exceptions import (HTTPOk, HTTPPartialContent, HTTPRequestRangeNotSatisfiable, HTTPNotModified) + class FileSender: """"A helper that can be used to send files. """ @@ -164,7 +165,7 @@ def send(self, request, filepath): # If a range request has been made, convert start, end slice notation # into file pointer offset and count - if start is not end is not None: + if start is not None or end is not None: status = HTTPPartialContent.status_code if start is None: # return tail of file if end < 0: diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index 2c466b97f3e..32038d86c6d 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -315,7 +315,8 @@ def cookies(self): def http_range(self, *, _RANGE=hdrs.RANGE): """ The content of Range HTTP header. - :returns tuple (start, end): values that can be used for slice eg. content[start:end] + :returns tuple (start, end): values that can be used for slice + eg. content[start:end] """ rng = self.headers.get(_RANGE) start, end = None, None @@ -329,12 +330,14 @@ def http_range(self, *, _RANGE=hdrs.RANGE): if end is not None: end = int(end) if start is None: - end = -end # end with no start is to return tail of content + # end with no start is to return tail of content + end = -end if start is not None: start = int(start) if end is not None: - end += 1 # end is inclusive in range header, exclusive in slice + # end is inclusive in range header, exclusive in slice + end += 1 if start is end is None: # No valid range supplied raise ValueError('No start or end of range specified') From 61b94c254fad6499225545085be644c45cbd10cb Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 10:14:03 +1100 Subject: [PATCH 14/19] Fixed range header parsing --- aiohttp/web_reqrep.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index 32038d86c6d..b10196283ec 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -327,17 +327,19 @@ def http_range(self, *, _RANGE=hdrs.RANGE): except IndexError: # pattern was not found in header raise ValueError("range not in acceptible format") - if end is not None: - end = int(end) - if start is None: - # end with no start is to return tail of content - end = -end - - if start is not None: - start = int(start) - if end is not None: - # end is inclusive in range header, exclusive in slice - end += 1 + end = int(end) if end else None + start = int(start) if start else None + + if start is None and end is not None: + # end with no start is to return tail of content + end = -end + + if start is not None and end is not None: + # end is inclusive in range header, exclusive for slice + end += 1 + + if start > end: + raise ValueError('start cannot be after end') if start is end is None: # No valid range supplied raise ValueError('No start or end of range specified') From 27d3950b4315501234b6fede50021e6a1241f035 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 11:26:23 +1100 Subject: [PATCH 15/19] fix import order with 'make isort' --- aiohttp/file_sender.py | 4 ++-- aiohttp/web_reqrep.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index 8856f56ca29..dc5a2be5e6e 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -4,9 +4,9 @@ from . import hdrs from .helpers import create_future +from .web_exceptions import (HTTPNotModified, HTTPOk, HTTPPartialContent, + HTTPRequestRangeNotSatisfiable) from .web_reqrep import StreamResponse -from .web_exceptions import (HTTPOk, HTTPPartialContent, - HTTPRequestRangeNotSatisfiable, HTTPNotModified) class FileSender: diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index b10196283ec..c7543c85e50 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -8,9 +8,9 @@ import io import json import math +import re import time import warnings -import re from email.utils import parsedate from types import MappingProxyType From 78d3fcf1002f235d21092789f7c7f70312f13933 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 12:06:23 +1100 Subject: [PATCH 16/19] Split up main range unit test so it doesn't take too long and timeout. --- tests/test_web_sendfile_functional.py | 50 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index ec1b9a6ecb2..50b81ee903c 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -342,24 +342,44 @@ def handler(request): content = f.read() # Ensure the whole file requested in parts is correct - resp = yield from client.get('/', headers={'Range': 'bytes=0-999'}) - assert resp.status == 206, resp.reason - body1 = yield from resp.read() - assert len(body1) == 1000 - resp.close() + responses = yield from asyncio.gather( + client.get('/', headers={'Range': 'bytes=0-999'}), + client.get('/', headers={'Range': 'bytes=1000-1999'}), + client.get('/', headers={'Range': 'bytes=2000-'}), + loop=loop + ) + assert len(responses) == 3 + assert responses[0].status == 206, "failed 'bytes=0-999': %s" % responses[0].reason + assert responses[1].status == 206, "failed 'bytes=1000-1999': %s" % responses[1].reason + assert responses[2].status == 206, "failed 'bytes=2000-': %s" % responses[2].reason - resp = yield from client.get('/', headers={'Range': 'bytes=1000-1999'}) - assert resp.status == 206, resp.reason - body2 = yield from resp.read() - assert len(body2) == 1000 - resp.close() + body = yield from asyncio.gather(*(resp.read() for resp in responses), loop=loop) - resp = yield from client.get('/', headers={'Range': 'bytes=2000-'}) - assert resp.status == 206, resp.reason - body3 = yield from resp.read() - resp.close() + assert len(body[0]) == 1000, "failed 'bytes=0-999', received length was %d" % len(body[0]) + assert len(body[1]) == 1000, "failed 'bytes=1000-1999', received length was %d" % len(body[1]) + responses[0].close() + responses[1].close() + responses[2].close() + + assert content == b"".join(body) + + +@asyncio.coroutine +def test_static_file_range_tail(loop, test_client, sender): + filepath = (pathlib.Path(__file__).parent / + 'software_development_in_picture.jpg') + + @asyncio.coroutine + def handler(request): + resp = yield from sender(chunk_size=16).send(request, filepath) + return resp - assert content == (body1 + body2 + body3) + app = web.Application(loop=loop) + app.router.add_get('/', handler) + client = yield from test_client(lambda loop: app) + + with filepath.open('rb') as f: + content = f.read() # Ensure the tail of the file is correct resp = yield from client.get('/', headers={'Range': 'bytes=-500'}) From 88d98e0c4e25af233ddc99d7c9242fb79c6c3b98 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 14:23:05 +1100 Subject: [PATCH 17/19] Adhere to line length restrictions --- tests/test_web_sendfile_functional.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 50b81ee903c..f02c1ad5938 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -349,14 +349,22 @@ def handler(request): loop=loop ) assert len(responses) == 3 - assert responses[0].status == 206, "failed 'bytes=0-999': %s" % responses[0].reason - assert responses[1].status == 206, "failed 'bytes=1000-1999': %s" % responses[1].reason - assert responses[2].status == 206, "failed 'bytes=2000-': %s" % responses[2].reason - - body = yield from asyncio.gather(*(resp.read() for resp in responses), loop=loop) + assert responses[0].status == 206, \ + "failed 'bytes=0-999': %s" % responses[0].reason + assert responses[1].status == 206, \ + "failed 'bytes=1000-1999': %s" % responses[1].reason + assert responses[2].status == 206, \ + "failed 'bytes=2000-': %s" % responses[2].reason + + body = yield from asyncio.gather( + *(resp.read() for resp in responses), + loop=loop + ) - assert len(body[0]) == 1000, "failed 'bytes=0-999', received length was %d" % len(body[0]) - assert len(body[1]) == 1000, "failed 'bytes=1000-1999', received length was %d" % len(body[1]) + assert len(body[0]) == 1000, \ + "failed 'bytes=0-999', received %d bytes" % len(body[0]) + assert len(body[1]) == 1000, \ + "failed 'bytes=1000-1999', received %d bytes" % len(body[1]) responses[0].close() responses[1].close() responses[2].close() From 0a2c642b2fb95d7f551ce0aeb09f8f2c60e4da54 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 14:57:13 +1100 Subject: [PATCH 18/19] Improve the strictness of range header pattern match and increase test coverage. --- aiohttp/file_sender.py | 9 +++------ aiohttp/web_reqrep.py | 4 ++-- tests/test_web_sendfile_functional.py | 12 +++++++++++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/aiohttp/file_sender.py b/aiohttp/file_sender.py index dc5a2be5e6e..c8eea1b5d51 100644 --- a/aiohttp/file_sender.py +++ b/aiohttp/file_sender.py @@ -167,12 +167,9 @@ def send(self, request, filepath): # into file pointer offset and count if start is not None or end is not None: status = HTTPPartialContent.status_code - if start is None: # return tail of file - if end < 0: - start = file_size + end - count = -end - else: - raise HTTPRequestRangeNotSatisfiable + if start is None and end < 0: # return tail of file + start = file_size + end + count = -end else: count = (end or file_size) - start diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index c7543c85e50..c0dcc7eae56 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -322,7 +322,7 @@ def http_range(self, *, _RANGE=hdrs.RANGE): start, end = None, None if rng is not None: try: - pattern = r'bytes=(\d*)-(\d*)' + pattern = r'^bytes=(\d*)-(\d*)$' start, end = re.findall(pattern, rng)[0] except IndexError: # pattern was not found in header raise ValueError("range not in acceptible format") @@ -338,7 +338,7 @@ def http_range(self, *, _RANGE=hdrs.RANGE): # end is inclusive in range header, exclusive for slice end += 1 - if start > end: + if start >= end: raise ValueError('start cannot be after end') if start is end is None: # No valid range supplied diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index f02c1ad5938..05bc569952b 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -424,7 +424,12 @@ def handler(request): resp.close() # start > end - resp = yield from client.get('/', headers={'Range': 'bytes=10-5'}) + resp = yield from client.get('/', headers={'Range': 'bytes=100-0'}) + assert resp.status == 416, "Range start can't be greater than end" + resp.close() + + # start > end + resp = yield from client.get('/', headers={'Range': 'bytes=10-9'}) assert resp.status == 416, "Range start can't be greater than end" resp.close() @@ -432,3 +437,8 @@ def handler(request): resp = yield from client.get('/', headers={'Range': 'bytes=a-f'}) assert resp.status == 416, 'Range must be integers' resp.close() + + # double dash range + resp = yield from client.get('/', headers={'Range': 'bytes=0--10'}) + assert resp.status == 416, 'double dash in range' + resp.close() From c88482525fc4978fcbc53758c3f6c00574d4d759 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Fri, 11 Nov 2016 15:02:35 +1100 Subject: [PATCH 19/19] Add test to cover invalid case of blank start and end time given --- tests/test_web_sendfile_functional.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 05bc569952b..50f1e5ab7ea 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -442,3 +442,8 @@ def handler(request): resp = yield from client.get('/', headers={'Range': 'bytes=0--10'}) assert resp.status == 416, 'double dash in range' resp.close() + + # no range + resp = yield from client.get('/', headers={'Range': 'bytes=-'}) + assert resp.status == 416, 'no range given' + resp.close()