From f9bd33311a1e794169091643811b6f2546c2a2c1 Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 12:32:17 -0700 Subject: [PATCH 01/11] [StaticRoute] Add preliminary support for sendfile system call. --- aiohttp/web_urldispatcher.py | 82 ++++++++++++++++++++++++++++++------ tests/test_web_functional.py | 11 ++++- 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 10fbf6ebf52..1cf751883a8 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -145,7 +145,7 @@ class StaticRoute(Route): def __init__(self, name, prefix, directory, *, expect_handler=None, chunk_size=256*1024, - response_factory=None): + response_factory=None, sendfile_fallback=False): assert prefix.startswith('/'), prefix assert prefix.endswith('/'), prefix super().__init__( @@ -163,6 +163,11 @@ def __init__(self, name, prefix, directory, *, raise ValueError( "No directory exists at '{}'".format(self._directory)) + if sendfile_fallback or not hasattr(os, "sendfile"): + self.sendfile = self.sendfile_fallback + else: + self.sendfile = self.sendfile_system + def match(self, path): if not path.startswith(self._prefix): return None @@ -174,6 +179,63 @@ def url(self, *, filename, query=None): url = self._prefix + filename return self._append_query(url, query) + def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): + try: + n = os.sendfile(out_fd, in_fd, offset, count) + except (BlockingIOError, InterruptedError): + return + except Exception as exc: + loop.remove_writer(out_fd) + fut.set_exception(exc) + return + else: + loop.remove_writer(out_fd) + + if n < count: + loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, + offset + n, count - n, loop) + else: + fut.set_result(None) + + @asyncio.coroutine + def _sendfile(self, out_fd, in_fd, offset, count, loop): + fut = asyncio.Future(loop=loop) + loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, offset, + count, loop) + return fut + + @asyncio.coroutine + def sendfile_system(self, resp, fobj): + """ + Write the content of `fobj` to `resp` using the ``sendfile`` system + call. + + `fobj` should be an open file object. + + `resp` should be a :obj:`aiohttp.web.StreamResponse` instance. + """ + yield from resp.drain() + req = resp._req + loop = req.app.loop + out_fd = req._transport._sock_fd + in_fd = fobj.fileno() + yield from self._sendfile(out_fd, in_fd, 0, os.fstat(in_fd).st_size, + loop) + + @asyncio.coroutine + def sendfile_fallback(self, resp, fobj): + """ + Mimic the :meth:`sendfile` method, but without using the ``sendfile`` + system call. This should be used on systems that don't support + ``sendfile``. + """ + yield from resp.drain() + chunk = fobj.read(self._chunk_size) + while chunk: + resp.write(chunk) + yield from resp.drain() + chunk = fobj.read(self._chunk_size) + @asyncio.coroutine def handle(self, request): filename = request.match_info['filename'] @@ -200,20 +262,12 @@ def handle(self, request): resp.last_modified = st.st_mtime file_size = st.st_size - single_chunk = file_size < self._chunk_size - if single_chunk: - resp.content_length = file_size + resp.content_length = file_size resp.start(request) with open(filepath, 'rb') as f: - chunk = f.read(self._chunk_size) - if single_chunk: - resp.write(chunk) - else: - while chunk: - resp.write(chunk) - chunk = f.read(self._chunk_size) + yield from self.sendfile(resp, f) return resp @@ -413,7 +467,8 @@ def add_route(self, method, path, handler, return route def add_static(self, prefix, path, *, name=None, expect_handler=None, - chunk_size=256*1024, response_factory=None): + chunk_size=256*1024, response_factory=None, + sendfile_fallback=False): """ Adds static files view :param prefix - url prefix @@ -425,6 +480,7 @@ def add_static(self, prefix, path, *, name=None, expect_handler=None, route = StaticRoute(name, prefix, path, expect_handler=expect_handler, chunk_size=chunk_size, - response_factory=response_factory) + response_factory=response_factory, + sendfile_fallback=sendfile_fallback) self.register_route(route) return route diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 8493bdeec2c..3a66aa68b14 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -1,6 +1,7 @@ import asyncio import gc import json +import os import os.path import socket import unittest @@ -312,14 +313,20 @@ def go(): self.loop.run_until_complete(go()) - def test_static_file(self): + @unittest.skipUnless(hasattr(os, "sendfile"), + "system does not support 'sendfile' system call") + def test_static_file_sendfile(self): + self.test_static_file(False) + + def test_static_file(self, sendfile_fallback=True): @asyncio.coroutine def go(dirname, filename): app, _, url = yield from self.create_server( 'GET', '/static/' + filename ) - app.router.add_static('/static', dirname) + app.router.add_static('/static', dirname, + sendfile_fallback=sendfile_fallback) resp = yield from request('GET', url, loop=self.loop) self.assertEqual(200, resp.status) From a993a97d18d595c9c68e144249f6a53dbd8756cb Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 14:07:09 -0700 Subject: [PATCH 02/11] [StaticRoute] Remove redundant `_sendfile` coroutine & future proof `sendfile` for RANGE support. --- aiohttp/web_urldispatcher.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 1cf751883a8..25706010c61 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -198,14 +198,7 @@ def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): fut.set_result(None) @asyncio.coroutine - def _sendfile(self, out_fd, in_fd, offset, count, loop): - fut = asyncio.Future(loop=loop) - loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, offset, - count, loop) - return fut - - @asyncio.coroutine - def sendfile_system(self, resp, fobj): + def sendfile_system(self, resp, fobj, offset, count): """ Write the content of `fobj` to `resp` using the ``sendfile`` system call. @@ -215,15 +208,21 @@ def sendfile_system(self, resp, fobj): `resp` should be a :obj:`aiohttp.web.StreamResponse` instance. """ yield from resp.drain() + req = resp._req + loop = req.app.loop out_fd = req._transport._sock_fd in_fd = fobj.fileno() - yield from self._sendfile(out_fd, in_fd, 0, os.fstat(in_fd).st_size, - loop) + fut = asyncio.Future(loop=loop) + + loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, offset, + count, loop) + + yield from fut @asyncio.coroutine - def sendfile_fallback(self, resp, fobj): + def sendfile_fallback(self, resp, fobj, offset, count): """ Mimic the :meth:`sendfile` method, but without using the ``sendfile`` system call. This should be used on systems that don't support @@ -267,7 +266,7 @@ def handle(self, request): resp.start(request) with open(filepath, 'rb') as f: - yield from self.sendfile(resp, f) + yield from self.sendfile(resp, f, 0, file_size) return resp From ec4ef9d1f329a61149ee8844307510e701dded7a Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 14:34:44 -0700 Subject: [PATCH 03/11] [StaticRoute] Refactor `_sendfile_cb` to avoid repeated `remove_writer` calls. --- aiohttp/web_urldispatcher.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 25706010c61..dfc50e0e47e 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -180,16 +180,14 @@ def url(self, *, filename, query=None): return self._append_query(url, query) def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): + loop.remove_writer(out_fd) try: n = os.sendfile(out_fd, in_fd, offset, count) except (BlockingIOError, InterruptedError): - return + n = 0 except Exception as exc: - loop.remove_writer(out_fd) fut.set_exception(exc) return - else: - loop.remove_writer(out_fd) if n < count: loop.add_writer(out_fd, self._sendfile_cb, fut, out_fd, in_fd, From 657fb92c0c7019f0f0f531288ff91a755bf128dd Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 14:54:25 -0700 Subject: [PATCH 04/11] [StaticRoute] Use public API to access socket fd in sendfile. --- aiohttp/web_urldispatcher.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index dfc50e0e47e..d116ac38f29 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -196,7 +196,7 @@ def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): fut.set_result(None) @asyncio.coroutine - def sendfile_system(self, resp, fobj, offset, count): + def sendfile_system(self, req, resp, fobj, offset, count): """ Write the content of `fobj` to `resp` using the ``sendfile`` system call. @@ -207,10 +207,8 @@ def sendfile_system(self, resp, fobj, offset, count): """ yield from resp.drain() - req = resp._req - loop = req.app.loop - out_fd = req._transport._sock_fd + out_fd = req.transport.get_extra_info("socket").fileno() in_fd = fobj.fileno() fut = asyncio.Future(loop=loop) @@ -220,7 +218,7 @@ def sendfile_system(self, resp, fobj, offset, count): yield from fut @asyncio.coroutine - def sendfile_fallback(self, resp, fobj, offset, count): + def sendfile_fallback(self, req, resp, fobj, offset, count): """ Mimic the :meth:`sendfile` method, but without using the ``sendfile`` system call. This should be used on systems that don't support @@ -264,7 +262,7 @@ def handle(self, request): resp.start(request) with open(filepath, 'rb') as f: - yield from self.sendfile(resp, f, 0, file_size) + yield from self.sendfile(request, resp, f, 0, file_size) return resp From 07546fdb0c3820e3a7d0f0129d54796da0209d08 Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 16:40:35 -0700 Subject: [PATCH 05/11] [StaticRoute] Remove `sendfile_fallback` flag & refactor unittests. --- aiohttp/web_urldispatcher.py | 18 +- tests/test_web_functional.py | 446 ++++++++++++++++++----------------- 2 files changed, 244 insertions(+), 220 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index d116ac38f29..3b939a0bde5 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -145,7 +145,7 @@ class StaticRoute(Route): def __init__(self, name, prefix, directory, *, expect_handler=None, chunk_size=256*1024, - response_factory=None, sendfile_fallback=False): + response_factory=None): assert prefix.startswith('/'), prefix assert prefix.endswith('/'), prefix super().__init__( @@ -163,11 +163,6 @@ def __init__(self, name, prefix, directory, *, raise ValueError( "No directory exists at '{}'".format(self._directory)) - if sendfile_fallback or not hasattr(os, "sendfile"): - self.sendfile = self.sendfile_fallback - else: - self.sendfile = self.sendfile_system - def match(self, path): if not path.startswith(self._prefix): return None @@ -231,6 +226,11 @@ def sendfile_fallback(self, req, resp, fobj, offset, count): yield from resp.drain() chunk = fobj.read(self._chunk_size) + if hasattr(os, "sendfile"): + sendfile = sendfile_system + else: + sendfile = sendfile_fallback + @asyncio.coroutine def handle(self, request): filename = request.match_info['filename'] @@ -462,8 +462,7 @@ def add_route(self, method, path, handler, return route def add_static(self, prefix, path, *, name=None, expect_handler=None, - chunk_size=256*1024, response_factory=None, - sendfile_fallback=False): + chunk_size=256*1024, response_factory=None): """ Adds static files view :param prefix - url prefix @@ -475,7 +474,6 @@ def add_static(self, prefix, path, *, name=None, expect_handler=None, route = StaticRoute(name, prefix, path, expect_handler=expect_handler, chunk_size=chunk_size, - response_factory=response_factory, - sendfile_fallback=sendfile_fallback) + response_factory=response_factory) self.register_route(route) return route diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 3a66aa68b14..0919cedd91a 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -11,7 +11,7 @@ from aiohttp.streams import EOF_MARKER -class TestWebFunctional(unittest.TestCase): +class WebFunctionalSetupMixin(unittest.TestCase): def setUp(self): self.handler = None @@ -49,6 +49,9 @@ def create_server(self, method, path, handler=None): self.addCleanup(srv.close) return app, srv, url + +class TestWebFunctional(WebFunctionalSetupMixin): + def test_simple_get(self): @asyncio.coroutine @@ -313,215 +316,6 @@ def go(): self.loop.run_until_complete(go()) - @unittest.skipUnless(hasattr(os, "sendfile"), - "system does not support 'sendfile' system call") - def test_static_file_sendfile(self): - self.test_static_file(False) - - def test_static_file(self, sendfile_fallback=True): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname, - sendfile_fallback=sendfile_fallback) - - resp = yield from request('GET', url, loop=self.loop) - self.assertEqual(200, resp.status) - txt = yield from resp.text() - self.assertEqual('file content', txt.rstrip()) - ct = resp.headers['CONTENT-TYPE'] - self.assertEqual('application/octet-stream', ct) - self.assertEqual(resp.headers.get('CONTENT-ENCODING'), None) - resp.close() - - resp = yield from request('GET', url + 'fake', loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - resp = yield from request('GET', url + '/../../', loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'data.unknown_mime_type' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_with_content_type(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname, chunk_size=16) - - resp = yield from request('GET', url, loop=self.loop) - self.assertEqual(200, resp.status) - body = yield from resp.read() - with open(os.path.join(dirname, filename), 'rb') as f: - content = f.read() - self.assertEqual(content, body) - ct = resp.headers['CONTENT-TYPE'] - self.assertEqual('image/jpeg', ct) - self.assertEqual(resp.headers.get('CONTENT-ENCODING'), None) - resp.close() - - resp = yield from request('GET', url + 'fake', loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - resp = yield from request('GET', url + '/../../', loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'software_development_in_picture.jpg' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_with_content_encoding(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname) - - resp = yield from request('GET', url, loop=self.loop) - self.assertEqual(200, resp.status) - body = yield from resp.read() - self.assertEqual(b'hello aiohttp\n', body) - ct = resp.headers['CONTENT-TYPE'] - self.assertEqual('text/plain', ct) - encoding = resp.headers['CONTENT-ENCODING'] - self.assertEqual('gzip', encoding) - resp.close() - - here = os.path.dirname(__file__) - filename = 'hello.txt.gz' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_directory_traversal_attack(self): - - @asyncio.coroutine - def go(dirname, relpath): - self.assertTrue(os.path.isfile(os.path.join(dirname, relpath))) - - app, _, url = yield from self.create_server('GET', '/static/') - app.router.add_static('/static', dirname) - - url_relpath = url + relpath - resp = yield from request('GET', url_relpath, loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - url_relpath2 = url + 'dir/../' + filename - resp = yield from request('GET', url_relpath2, loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - url_abspath = \ - url + os.path.abspath(os.path.join(dirname, filename)) - resp = yield from request('GET', url_abspath, loop=self.loop) - self.assertEqual(404, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = '../README.rst' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_if_modified_since(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname) - - resp = yield from request('GET', url, loop=self.loop) - self.assertEqual(200, resp.status) - lastmod = resp.headers.get('Last-Modified') - self.assertIsNotNone(lastmod) - resp.close() - - resp = yield from request('GET', url, loop=self.loop, - headers={'If-Modified-Since': lastmod}) - self.assertEqual(304, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'data.unknown_mime_type' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_if_modified_since_past_date(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname) - - lastmod = 'Mon, 1 Jan 1990 01:01:01 GMT' - resp = yield from request('GET', url, loop=self.loop, - headers={'If-Modified-Since': lastmod}) - self.assertEqual(200, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'data.unknown_mime_type' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_if_modified_since_future_date(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname) - - lastmod = 'Fri, 31 Dec 9999 23:59:59 GMT' - resp = yield from request('GET', url, loop=self.loop, - headers={'If-Modified-Since': lastmod}) - self.assertEqual(304, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'data.unknown_mime_type' - self.loop.run_until_complete(go(here, filename)) - - def test_static_file_if_modified_since_invalid_date(self): - - @asyncio.coroutine - def go(dirname, filename): - app, _, url = yield from self.create_server( - 'GET', '/static/' + filename - ) - app.router.add_static('/static', dirname) - - lastmod = 'not a valid HTTP-date' - resp = yield from request('GET', url, loop=self.loop, - headers={'If-Modified-Since': lastmod}) - self.assertEqual(200, resp.status) - resp.close() - - here = os.path.dirname(__file__) - filename = 'data.unknown_mime_type' - self.loop.run_until_complete(go(here, filename)) - - def test_static_route_path_existence_check(self): - directory = os.path.dirname(__file__) - web.StaticRoute(None, "/", directory) - - nodirectory = os.path.join(directory, "nonexistent-uPNiOEAg5d") - with self.assertRaises(ValueError): - web.StaticRoute(None, "/", nodirectory) - def test_post_form_with_duplicate_keys(self): @asyncio.coroutine @@ -933,3 +727,235 @@ def go(): client.close() self.loop.run_until_complete(go()) + + +class TestStaticFileSendfileFallback(WebFunctionalSetupMixin): + + def patch_sendfile(self, add_static): + def f(*args, **kwargs): + route = add_static(*args, **kwargs) + route.sendfile = route.sendfile_fallback + return route + return f + + @asyncio.coroutine + def create_server(self, method, path): + app, srv, url = yield from super().create_server(method, path) + app.router.add_static = self.patch_sendfile(app.router.add_static) + + return app, srv, url + + def test_static_file(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + resp = yield from request('GET', url, loop=self.loop) + self.assertEqual(200, resp.status) + txt = yield from resp.text() + self.assertEqual('file content', txt.rstrip()) + ct = resp.headers['CONTENT-TYPE'] + self.assertEqual('application/octet-stream', ct) + self.assertEqual(resp.headers.get('CONTENT-ENCODING'), None) + resp.close() + + resp = yield from request('GET', url + 'fake', loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + resp = yield from request('GET', url + '/../../', loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_with_content_type(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname, chunk_size=16) + + resp = yield from request('GET', url, loop=self.loop) + self.assertEqual(200, resp.status) + body = yield from resp.read() + with open(os.path.join(dirname, filename), 'rb') as f: + content = f.read() + self.assertEqual(content, body) + ct = resp.headers['CONTENT-TYPE'] + self.assertEqual('image/jpeg', ct) + self.assertEqual(resp.headers.get('CONTENT-ENCODING'), None) + resp.close() + + resp = yield from request('GET', url + 'fake', loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + resp = yield from request('GET', url + '/../../', loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'software_development_in_picture.jpg' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_with_content_encoding(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + resp = yield from request('GET', url, loop=self.loop) + self.assertEqual(200, resp.status) + body = yield from resp.read() + self.assertEqual(b'hello aiohttp\n', body) + ct = resp.headers['CONTENT-TYPE'] + self.assertEqual('text/plain', ct) + encoding = resp.headers['CONTENT-ENCODING'] + self.assertEqual('gzip', encoding) + resp.close() + + here = os.path.dirname(__file__) + filename = 'hello.txt.gz' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_directory_traversal_attack(self): + + @asyncio.coroutine + def go(dirname, relpath): + self.assertTrue(os.path.isfile(os.path.join(dirname, relpath))) + + app, _, url = yield from self.create_server('GET', '/static/') + app.router.add_static('/static', dirname) + + url_relpath = url + relpath + resp = yield from request('GET', url_relpath, loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + url_relpath2 = url + 'dir/../' + filename + resp = yield from request('GET', url_relpath2, loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + url_abspath = \ + url + os.path.abspath(os.path.join(dirname, filename)) + resp = yield from request('GET', url_abspath, loop=self.loop) + self.assertEqual(404, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = '../README.rst' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_if_modified_since(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + resp = yield from request('GET', url, loop=self.loop) + self.assertEqual(200, resp.status) + lastmod = resp.headers.get('Last-Modified') + self.assertIsNotNone(lastmod) + resp.close() + + resp = yield from request('GET', url, loop=self.loop, + headers={'If-Modified-Since': lastmod}) + self.assertEqual(304, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_if_modified_since_past_date(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + lastmod = 'Mon, 1 Jan 1990 01:01:01 GMT' + resp = yield from request('GET', url, loop=self.loop, + headers={'If-Modified-Since': lastmod}) + self.assertEqual(200, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_if_modified_since_future_date(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + lastmod = 'Fri, 31 Dec 9999 23:59:59 GMT' + resp = yield from request('GET', url, loop=self.loop, + headers={'If-Modified-Since': lastmod}) + self.assertEqual(304, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_if_modified_since_invalid_date(self): + + @asyncio.coroutine + def go(dirname, filename): + app, _, url = yield from self.create_server( + 'GET', '/static/' + filename + ) + app.router.add_static('/static', dirname) + + lastmod = 'not a valid HTTP-date' + resp = yield from request('GET', url, loop=self.loop, + headers={'If-Modified-Since': lastmod}) + self.assertEqual(200, resp.status) + resp.close() + + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_route_path_existence_check(self): + directory = os.path.dirname(__file__) + web.StaticRoute(None, "/", directory) + + nodirectory = os.path.join(directory, "nonexistent-uPNiOEAg5d") + with self.assertRaises(ValueError): + web.StaticRoute(None, "/", nodirectory) + + +@unittest.skipUnless(hasattr(os, "sendfile"), + "sendfile system call not supported") +class TestStaticFileSendfile(TestStaticFileSendfileFallback): + + def patch_sendfile(self, add_static): + def f(*args, **kwargs): + route = add_static(*args, **kwargs) + route.sendfile = route.sendfile_system + return route + return f From 332c7565bb02b1a8bb75e4d908e87f5c3cf45dac Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Sun, 13 Sep 2015 16:46:46 -0700 Subject: [PATCH 06/11] [StaticRoute] Remove pointless initial `resp.drain()` in `sendfile_fallback`. --- aiohttp/web_urldispatcher.py | 1 - 1 file changed, 1 deletion(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 3b939a0bde5..924e8ebc9ba 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -219,7 +219,6 @@ def sendfile_fallback(self, req, resp, fobj, offset, count): system call. This should be used on systems that don't support ``sendfile``. """ - yield from resp.drain() chunk = fobj.read(self._chunk_size) while chunk: resp.write(chunk) From 1e221cba711f0d85addc410fcb7bfc3f22419e8e Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Mon, 14 Sep 2015 11:58:47 -0700 Subject: [PATCH 07/11] [StaticRoute] Update `sendfile` docstrings. --- aiohttp/web_urldispatcher.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 924e8ebc9ba..c1bfdd7bce4 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -196,9 +196,13 @@ def sendfile_system(self, req, resp, fobj, offset, count): Write the content of `fobj` to `resp` using the ``sendfile`` system call. - `fobj` should be an open file object. + `req` should be a :obj:`aiohttp.web.Request` instance. `resp` should be a :obj:`aiohttp.web.StreamResponse` instance. + + `fobj` should be an open file object. + + `offset` & `count` allow for partial writes. """ yield from resp.drain() From 33ce05550a26cd53d8535bbec53ea26e22b39ab6 Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Mon, 14 Sep 2015 15:13:02 -0700 Subject: [PATCH 08/11] [StaticRoute] Implement offset & count support in `sendfile_fallback`. --- aiohttp/web_urldispatcher.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index c1bfdd7bce4..7ed8e24277f 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -223,11 +223,19 @@ def sendfile_fallback(self, req, resp, fobj, offset, count): system call. This should be used on systems that don't support ``sendfile``. """ - chunk = fobj.read(self._chunk_size) - while chunk: + fobj.seek(offset) + chunk_size = self._chunk_size + + chunk = fobj.read(chunk_size) + while chunk and count > chunk_size: resp.write(chunk) yield from resp.drain() - chunk = fobj.read(self._chunk_size) + count = count - chunk_size + chunk = fobj.read(chunk_size) + + if chunk: + resp.write(chunk[:count]) + yield from resp.drain() if hasattr(os, "sendfile"): sendfile = sendfile_system From 0cad8c1ed4101a71d5c825874f8d0a110000773f Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Mon, 14 Sep 2015 15:27:29 -0700 Subject: [PATCH 09/11] [StaticRoute] Update `sendfile_system` & `sendfile_fallback` docstrings. --- aiohttp/web_urldispatcher.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 7ed8e24277f..b5578a6a51d 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -193,8 +193,8 @@ def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): @asyncio.coroutine def sendfile_system(self, req, resp, fobj, offset, count): """ - Write the content of `fobj` to `resp` using the ``sendfile`` system - call. + Write `count` bytes of `fobj` to `resp` starting from `offset` using + the ``sendfile`` system call. `req` should be a :obj:`aiohttp.web.Request` instance. @@ -202,7 +202,9 @@ def sendfile_system(self, req, resp, fobj, offset, count): `fobj` should be an open file object. - `offset` & `count` allow for partial writes. + `offset` should be an integer >= 0. + + `count` should be an integer > 0. """ yield from resp.drain() @@ -220,8 +222,12 @@ def sendfile_system(self, req, resp, fobj, offset, count): def sendfile_fallback(self, req, resp, fobj, offset, count): """ Mimic the :meth:`sendfile` method, but without using the ``sendfile`` - system call. This should be used on systems that don't support - ``sendfile``. + system call. This should be used on systems that don't support the + ``sendfile`` system call. + + To avoid blocking the event loop & to keep memory usage low, `fobj` is + transferred in chunks controlled by the `chunk_size` argument to + :class:`StaticRoute`. """ fobj.seek(offset) chunk_size = self._chunk_size From a5e61f6115053b6e6bf8ed9b2de94ac31a75977f Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Mon, 14 Sep 2015 15:33:39 -0700 Subject: [PATCH 10/11] [StaticRoute] Make `sendfile_system` & `sendfile_fallback` private. --- aiohttp/web_urldispatcher.py | 8 ++++---- tests/test_web_functional.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index b5578a6a51d..f47d7b1f460 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -191,7 +191,7 @@ def _sendfile_cb(self, fut, out_fd, in_fd, offset, count, loop): fut.set_result(None) @asyncio.coroutine - def sendfile_system(self, req, resp, fobj, offset, count): + def _sendfile_system(self, req, resp, fobj, offset, count): """ Write `count` bytes of `fobj` to `resp` starting from `offset` using the ``sendfile`` system call. @@ -219,7 +219,7 @@ def sendfile_system(self, req, resp, fobj, offset, count): yield from fut @asyncio.coroutine - def sendfile_fallback(self, req, resp, fobj, offset, count): + def _sendfile_fallback(self, req, resp, fobj, offset, count): """ Mimic the :meth:`sendfile` method, but without using the ``sendfile`` system call. This should be used on systems that don't support the @@ -244,9 +244,9 @@ def sendfile_fallback(self, req, resp, fobj, offset, count): yield from resp.drain() if hasattr(os, "sendfile"): - sendfile = sendfile_system + sendfile = _sendfile_system else: - sendfile = sendfile_fallback + sendfile = _sendfile_fallback @asyncio.coroutine def handle(self, request): diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 0919cedd91a..3340ffe6b48 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -734,7 +734,7 @@ class TestStaticFileSendfileFallback(WebFunctionalSetupMixin): def patch_sendfile(self, add_static): def f(*args, **kwargs): route = add_static(*args, **kwargs) - route.sendfile = route.sendfile_fallback + route.sendfile = route._sendfile_fallback return route return f @@ -956,6 +956,6 @@ class TestStaticFileSendfile(TestStaticFileSendfileFallback): def patch_sendfile(self, add_static): def f(*args, **kwargs): route = add_static(*args, **kwargs) - route.sendfile = route.sendfile_system + route.sendfile = route._sendfile_system return route return f From 6ad6d32de3e62342bd0f6ea34955a484dc32bc7a Mon Sep 17 00:00:00 2001 From: Jashandeep Sohi Date: Mon, 14 Sep 2015 15:45:08 -0700 Subject: [PATCH 11/11] [StaticRoute] Fix `_sendfile_fallback` docstring. --- aiohttp/web_urldispatcher.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index f47d7b1f460..6fe5bfc183c 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -221,9 +221,9 @@ def _sendfile_system(self, req, resp, fobj, offset, count): @asyncio.coroutine def _sendfile_fallback(self, req, resp, fobj, offset, count): """ - Mimic the :meth:`sendfile` method, but without using the ``sendfile`` - system call. This should be used on systems that don't support the - ``sendfile`` system call. + Mimic the :meth:`_sendfile_system` method, but without using the + ``sendfile`` system call. This should be used on systems that don't + support the ``sendfile`` system call. To avoid blocking the event loop & to keep memory usage low, `fobj` is transferred in chunks controlled by the `chunk_size` argument to