From 3a8da86577e84499f64c09a10bb4c70fb6dff6e9 Mon Sep 17 00:00:00 2001 From: Vitaly Magerya Date: Sat, 30 May 2015 23:51:42 +0300 Subject: [PATCH 1/4] Make StaticRoute respect If-Modified-Since header --- aiohttp/web_urldispatcher.py | 17 +++++++++++---- tests/test_web_functional.py | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 02aeed35d20..9efff24b640 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -11,11 +11,12 @@ import inspect from urllib.parse import urlencode +from wsgiref.handlers import format_date_time from . import hdrs from .abc import AbstractRouter, AbstractMatchInfo from .protocol import HttpVersion11 -from .web_exceptions import HTTPMethodNotAllowed, HTTPNotFound +from .web_exceptions import HTTPMethodNotAllowed, HTTPNotFound, HTTPNotModified from .web_reqrep import StreamResponse @@ -169,7 +170,6 @@ def url(self, *, filename, query=None): @asyncio.coroutine def handle(self, request): - resp = StreamResponse() filename = request.match_info['filename'] filepath = os.path.abspath(os.path.join(self._directory, filename)) if not filepath.startswith(self._directory): @@ -177,14 +177,23 @@ def handle(self, request): if not os.path.exists(filepath) or not os.path.isfile(filepath): raise HTTPNotFound() + st = os.stat(filepath) + mtime = format_date_time(st.st_mtime) + + if request.headers.get(hdrs.IF_MODIFIED_SINCE) == mtime: + raise HTTPNotModified() + ct, encoding = mimetypes.guess_type(filepath) if not ct: ct = 'application/octet-stream' + + resp = StreamResponse() resp.content_type = ct if encoding: - resp.headers['content-encoding'] = encoding + resp.headers[hdrs.CONTENT_ENCODING] = encoding + resp.headers[hdrs.LAST_MODIFIED] = mtime - file_size = os.stat(filepath).st_size + file_size = st.st_size single_chunk = file_size < self._chunk_size if single_chunk: diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index fa975d66e30..1bb409c5a2e 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -374,6 +374,48 @@ def go(dirname, relpath): 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() + + lastmod = 'Mon, 1 Jan 1990 01:01:01 GMT' + resp = yield from request('GET', url, loop=self.loop, + headers={'If-Modified-Since': lastmod}) + self.assertIn(resp.status, (200, 304)) + resp.close() + + 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(200, resp.status) + resp.close() + + 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) From 748860b0928fd5c9eaa7650f9b5bc3fea99c2d0a Mon Sep 17 00:00:00 2001 From: Vitaly Magerya Date: Sun, 31 May 2015 10:57:22 +0300 Subject: [PATCH 2/4] Split If-Modified-Since test into multiple --- tests/test_web_functional.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 1bb409c5a2e..7074dd19113 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -394,12 +394,38 @@ def go(dirname, filename): 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.assertIn(resp.status, (200, 304)) resp.close() + here = os.path.dirname(__file__) + filename = 'data.unknown_mime_type' + self.loop.run_until_complete(go(here, filename)) + + def test_static_file_not_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) + lastmod = 'Fri, 31 Dec 9999 23:59:59 GMT' resp = yield from request('GET', url, loop=self.loop, headers={'If-Modified-Since': lastmod}) From 903726b4369f929e2d89cb89edaf31c0244bca67 Mon Sep 17 00:00:00 2001 From: Vitaly Magerya Date: Tue, 2 Jun 2015 13:18:27 +0300 Subject: [PATCH 3/4] Add 'last_modified' and 'if_modified_since' properties --- aiohttp/web_reqrep.py | 52 +++++++++++++++++++++++++++++++++++- aiohttp/web_urldispatcher.py | 7 +++-- docs/web_reference.rst | 18 +++++++++++++ tests/test_web_functional.py | 19 ++++++++++--- tests/test_web_response.py | 37 +++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 8 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index c63cc74ff1c..31af226b2d6 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -4,13 +4,17 @@ import binascii import cgi import collections +import datetime import http.cookies import io import json +import math +import time import warnings -from urllib.parse import urlsplit, parse_qsl, unquote +from email.utils import parsedate from types import MappingProxyType +from urllib.parse import urlsplit, parse_qsl, unquote from . import hdrs from .helpers import reify @@ -65,6 +69,33 @@ def content_length(self, _CONTENT_LENGTH=hdrs.CONTENT_LENGTH): else: return int(l) + @property + def if_modified_since(self, _IF_MODIFIED_SINCE=hdrs.IF_MODIFIED_SINCE): + """The value of If-Modified-Since HTTP header, or None. + + This header is represented as a `datetime` object. + """ + httpdate = self.headers.get(_IF_MODIFIED_SINCE) + if httpdate is not None: + timetuple = parsedate(httpdate) + if timetuple is not None: + return datetime.datetime(*timetuple[:6], + tzinfo=datetime.timezone.utc) + return None + + @property + def last_modified(self, _LAST_MODIFIED=hdrs.LAST_MODIFIED): + """The value of Last-Modified HTTP header, or None. + + This header is represented as a `datetime` object. + """ + httpdate = self.headers.get(_LAST_MODIFIED) + if httpdate is not None: + timetuple = parsedate(httpdate) + if timetuple is not None: + return datetime.datetime(*timetuple[:6], + tzinfo=datetime.timezone.utc) + return None FileField = collections.namedtuple('Field', 'name filename file content_type') @@ -513,6 +544,25 @@ def charset(self, value): self._content_dict['charset'] = str(value).lower() self._generate_content_type_header() + @property + def last_modified(self): + # Just a placeholder for adding setter + return super().last_modified + + @last_modified.setter + def last_modified(self, value): + if value is None: + if hdrs.LAST_MODIFIED in self.headers: + del self.headers[hdrs.LAST_MODIFIED] + elif isinstance(value, (int, float)): + self.headers[hdrs.LAST_MODIFIED] = time.strftime( + "%a, %d %b %Y %H:%M:%S GMT", time.gmtime(math.ceil(value))) + elif isinstance(value, datetime.datetime): + self.headers[hdrs.LAST_MODIFIED] = time.strftime( + "%a, %d %b %Y %H:%M:%S GMT", value.utctimetuple()) + elif isinstance(value, str): + self.headers[hdrs.LAST_MODIFIED] = value + def _generate_content_type_header(self, CONTENT_TYPE=hdrs.CONTENT_TYPE): params = '; '.join("%s=%s" % i for i in self._content_dict.items()) if params: diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 9efff24b640..aba3e920c9c 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -11,7 +11,6 @@ import inspect from urllib.parse import urlencode -from wsgiref.handlers import format_date_time from . import hdrs from .abc import AbstractRouter, AbstractMatchInfo @@ -178,9 +177,9 @@ def handle(self, request): raise HTTPNotFound() st = os.stat(filepath) - mtime = format_date_time(st.st_mtime) - if request.headers.get(hdrs.IF_MODIFIED_SINCE) == mtime: + modsince = request.if_modified_since + if modsince is not None and st.st_mtime <= modsince.timestamp(): raise HTTPNotModified() ct, encoding = mimetypes.guess_type(filepath) @@ -191,7 +190,7 @@ def handle(self, request): resp.content_type = ct if encoding: resp.headers[hdrs.CONTENT_ENCODING] = encoding - resp.headers[hdrs.LAST_MODIFIED] = mtime + resp.last_modified = st.st_mtime file_size = st.st_size single_chunk = file_size < self._chunk_size diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 8ef1dc3a51b..e1ff207568f 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -198,6 +198,15 @@ first positional parameter. Returns :class:`int` or ``None`` if *Content-Length* is absent. + .. attribute:: if_modified_since + + Read-only property that returns the date specified in the + *If-Modified-Since* header. + + Returns :class:`datetime.datetime` or ``None`` if + *If-Modified-Since* header is absent or is not a valid + HTTP date. + .. coroutinemethod:: read() Read request body, returns :class:`bytes` object with body content. @@ -503,6 +512,15 @@ StreamResponse The value converted to lower-case on attribute assigning. + .. attribute:: last_modified + + *Last-Modified* header for outgoing response. + + This property accepts raw :class:`str` values, + :class:`datetime.datetime` objects, Unix timestamps specified + as an :class:`int` or a :class:`float` object, and the + value ``None`` to unset the header. + .. method:: start(request) :param aiohttp.web.Request request: HTTP request object, that the diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 7074dd19113..eb057bb1d8d 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -410,14 +410,14 @@ def go(dirname, filename): lastmod = 'Mon, 1 Jan 1990 01:01:01 GMT' resp = yield from request('GET', url, loop=self.loop, headers={'If-Modified-Since': lastmod}) - self.assertIn(resp.status, (200, 304)) + 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_not_modified_since(self): + def test_static_file_if_modified_since_future_date(self): @asyncio.coroutine def go(dirname, filename): @@ -429,9 +429,22 @@ def go(dirname, filename): 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(200, resp.status) + 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}) diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 38a079016d9..44537a135fa 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -1,4 +1,5 @@ import asyncio +import datetime import unittest from unittest import mock from aiohttp import hdrs @@ -103,6 +104,42 @@ def test_charset_without_content_type(self): with self.assertRaises(RuntimeError): resp.charset = 'koi8-r' + def test_last_modified_initial(self): + resp = StreamResponse() + self.assertIsNone(resp.last_modified) + + def test_last_modified_string(self): + resp = StreamResponse() + + dt = datetime.datetime(1990, 1, 2, 3, 4, 5, 0, datetime.timezone.utc) + resp.last_modified = 'Mon, 2 Jan 1990 03:04:05 GMT' + self.assertEqual(resp.last_modified, dt) + + def test_last_modified_timestamp(self): + resp = StreamResponse() + + dt = datetime.datetime(1970, 1, 1, 0, 0, 0, 0, datetime.timezone.utc) + + resp.last_modified = 0 + self.assertEqual(resp.last_modified, dt) + + resp.last_modified = 0.0 + self.assertEqual(resp.last_modified, dt) + + def test_last_modified_datetime(self): + resp = StreamResponse() + + dt = datetime.datetime(2001, 2, 3, 4, 5, 6, 0, datetime.timezone.utc) + resp.last_modified = dt + self.assertEqual(resp.last_modified, dt) + + def test_last_modified_reset(self): + resp = StreamResponse() + + resp.last_modified = 0 + resp.last_modified = None + self.assertEqual(resp.last_modified, None) + @mock.patch('aiohttp.web_reqrep.ResponseImpl') def test_start(self, ResponseImpl): req = self.make_request('GET', '/') From 7823f61b13e5cff59dabe5df97031e516cf3faaf Mon Sep 17 00:00:00 2001 From: Vitaly Magerya Date: Tue, 2 Jun 2015 13:44:23 +0300 Subject: [PATCH 4/4] Move 'last_modified' and 'if_modified_since' properties --- aiohttp/web_reqrep.py | 57 ++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index 31af226b2d6..a7dba236624 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -69,34 +69,6 @@ def content_length(self, _CONTENT_LENGTH=hdrs.CONTENT_LENGTH): else: return int(l) - @property - def if_modified_since(self, _IF_MODIFIED_SINCE=hdrs.IF_MODIFIED_SINCE): - """The value of If-Modified-Since HTTP header, or None. - - This header is represented as a `datetime` object. - """ - httpdate = self.headers.get(_IF_MODIFIED_SINCE) - if httpdate is not None: - timetuple = parsedate(httpdate) - if timetuple is not None: - return datetime.datetime(*timetuple[:6], - tzinfo=datetime.timezone.utc) - return None - - @property - def last_modified(self, _LAST_MODIFIED=hdrs.LAST_MODIFIED): - """The value of Last-Modified HTTP header, or None. - - This header is represented as a `datetime` object. - """ - httpdate = self.headers.get(_LAST_MODIFIED) - if httpdate is not None: - timetuple = parsedate(httpdate) - if timetuple is not None: - return datetime.datetime(*timetuple[:6], - tzinfo=datetime.timezone.utc) - return None - FileField = collections.namedtuple('Field', 'name filename file content_type') @@ -229,6 +201,20 @@ def headers(self): """A case-insensitive multidict proxy with all headers.""" return self._headers + @property + def if_modified_since(self, _IF_MODIFIED_SINCE=hdrs.IF_MODIFIED_SINCE): + """The value of If-Modified-Since HTTP header, or None. + + This header is represented as a `datetime` object. + """ + httpdate = self.headers.get(_IF_MODIFIED_SINCE) + if httpdate is not None: + timetuple = parsedate(httpdate) + if timetuple is not None: + return datetime.datetime(*timetuple[:6], + tzinfo=datetime.timezone.utc) + return None + @property def keep_alive(self): """Is keepalive enabled by client?""" @@ -545,9 +531,18 @@ def charset(self, value): self._generate_content_type_header() @property - def last_modified(self): - # Just a placeholder for adding setter - return super().last_modified + def last_modified(self, _LAST_MODIFIED=hdrs.LAST_MODIFIED): + """The value of Last-Modified HTTP header, or None. + + This header is represented as a `datetime` object. + """ + httpdate = self.headers.get(_LAST_MODIFIED) + if httpdate is not None: + timetuple = parsedate(httpdate) + if timetuple is not None: + return datetime.datetime(*timetuple[:6], + tzinfo=datetime.timezone.utc) + return None @last_modified.setter def last_modified(self, value):