From 3afd52d2bfc0032b614b6dd6c0a8088e2317b77e Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 00:00:38 +0530 Subject: [PATCH 1/8] Fix Response docs --- docs/web_reference.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 00a10cf200b..16a2b31d9ea 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -680,14 +680,14 @@ Response .. attribute:: text - Read-write attribute for storing response's content, represented as str, - :class:`str`. + Read-write attribute for storing response's content, represented as + string, :class:`str`. - Setting :attr:`str` also recalculates + Setting :attr:`text` also recalculates :attr:`~StreamResponse.content_length` value and :attr:`~StreamResponse.body` value - Resetting :attr:`body` (assigning ``None``) sets + Resetting :attr:`text` (assigning ``None``) sets :attr:`~StreamResponse.content_length` to ``None`` too, dropping *Content-Length* HTTP header. From f86db670d8863c8c4b8b1c88b30bd1db77d3ca8d Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 00:06:03 +0530 Subject: [PATCH 2/8] Fix #944 --- aiohttp/web_reqrep.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index 83cc6ec4004..0431eba19b6 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -774,7 +774,7 @@ def __init__(self, *, body=None, status=200, self.set_tcp_cork(True) if body is not None and text is not None: - raise ValueError("body and text are not allowed together.") + raise ValueError('body and text are not allowed together') if text is not None: if hdrs.CONTENT_TYPE not in self.headers: @@ -794,25 +794,23 @@ def __init__(self, *, body=None, status=200, self._content_dict = {'charset': charset} self.body = text.encode(charset) else: - self.text = text if content_type or charset: - raise ValueError("Passing both Content-Type header and " - "content_type or charset params " - "is forbidden") + raise ValueError('passing both Content-Type header and ' + 'content_type or charset params ' + 'is forbidden') + self.text = text else: if hdrs.CONTENT_TYPE in self.headers: if content_type or charset: - raise ValueError("Passing both Content-Type header and " - "content_type or charset params " - "is forbidden") - if content_type: - self.content_type = content_type + raise ValueError('passing both Content-Type header and ' + 'content_type or charset params ' + 'is forbidden') + if content_type is None: + content_type = 'application/octet-stream' + self.content_type = content_type if charset: self.charset = charset - if body is not None: - self.body = body - else: - self.body = None + self.body = body @property def body(self): From 04842cabef87d77ac061b88b5c9e0d80d4517389 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 00:22:48 +0530 Subject: [PATCH 3/8] Add tests for default content_type --- tests/test_web_response.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_web_response.py b/tests/test_web_response.py index d89cc658e09..7dce54ad57a 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -887,6 +887,16 @@ def test_set_text_with_charset(): assert "koi8-r" == resp.charset +def test_content_type_with_set_text(): + resp = Response(text='text') + assert resp.content_type == 'text/plain' + + +def test_content_type_with_set_body(): + resp = Response(body=b'body') + assert resp.content_type == 'application/octet-stream' + + def test_started_when_not_started(): resp = StreamResponse() assert not resp.prepared From 5b35a7a4ee36fd704f1761524f36169a8c72b741 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 16:36:16 +0530 Subject: [PATCH 4/8] Use `"` for exception messages --- aiohttp/web_reqrep.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index 0431eba19b6..a73ba2bd6e1 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -649,7 +649,7 @@ def _start_pre_check(self, request): if self._resp_impl is not None: if self._req is not request: raise RuntimeError( - 'Response has been started with different request.') + "Response has been started with different request.") else: return self._resp_impl else: @@ -774,19 +774,19 @@ def __init__(self, *, body=None, status=200, self.set_tcp_cork(True) if body is not None and text is not None: - raise ValueError('body and text are not allowed together') + raise ValueError("body and text are not allowed together") if text is not None: if hdrs.CONTENT_TYPE not in self.headers: # fast path for filling headers if not isinstance(text, str): - raise TypeError('text argument must be str (%r)' % + raise TypeError("text argument must be str (%r)" % type(text)) if content_type is None: content_type = 'text/plain' elif ";" in content_type: - raise ValueError('charset must not be in content_type ' - 'argument') + raise ValueError("charset must not be in content_type " + "argument") charset = charset or 'utf-8' self.headers[hdrs.CONTENT_TYPE] = ( content_type + '; charset=%s' % charset) @@ -795,16 +795,16 @@ def __init__(self, *, body=None, status=200, self.body = text.encode(charset) else: if content_type or charset: - raise ValueError('passing both Content-Type header and ' - 'content_type or charset params ' - 'is forbidden') + raise ValueError("passing both Content-Type header and " + "content_type or charset params " + "is forbidden") self.text = text else: if hdrs.CONTENT_TYPE in self.headers: if content_type or charset: - raise ValueError('passing both Content-Type header and ' - 'content_type or charset params ' - 'is forbidden') + raise ValueError("passing both Content-Type header and " + "content_type or charset params " + "is forbidden") if content_type is None: content_type = 'application/octet-stream' self.content_type = content_type @@ -819,7 +819,7 @@ def body(self): @body.setter def body(self, body): if body is not None and not isinstance(body, bytes): - raise TypeError('body argument must be bytes (%r)' % type(body)) + raise TypeError("body argument must be bytes (%r)" % type(body)) self._body = body if body is not None: self.content_length = len(body) @@ -835,7 +835,7 @@ def text(self): @text.setter def text(self, text): if text is not None and not isinstance(text, str): - raise TypeError('text argument must be str (%r)' % type(text)) + raise TypeError("text argument must be str (%r)" % type(text)) if self.content_type == 'application/octet-stream': self.content_type = 'text/plain' @@ -863,7 +863,7 @@ def json_response(data=_sentinel, *, text=None, body=None, status=200, if data is not _sentinel: if text or body: raise ValueError( - 'only one of data, text, or body should be specified' + "only one of data, text, or body should be specified" ) else: text = dumps(data) From 841e0aa76ef8d8566930b6871382224329bfb362 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 19:17:56 +0530 Subject: [PATCH 5/8] Update headers in StreamResponse constructor --- aiohttp/web_reqrep.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index a73ba2bd6e1..d5339d81534 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -417,6 +417,8 @@ def __init__(self, *, status=200, reason=None, headers=None): if headers is not None: self._headers.extend(headers) + self._parse_content_type(self._headers.get(hdrs.CONTENT_TYPE)) + self._generate_content_type_header() def _copy_cookies(self): for cookie in self._cookies.values(): @@ -728,7 +730,7 @@ def _start(self, request): def write(self, data): assert isinstance(data, (bytes, bytearray, memoryview)), \ - 'data argument must be byte-ish (%r)' % type(data) + "data argument must be byte-ish (%r)" % type(data) if self._eof_sent: raise RuntimeError("Cannot call write() after write_eof()") @@ -805,9 +807,8 @@ def __init__(self, *, body=None, status=200, raise ValueError("passing both Content-Type header and " "content_type or charset params " "is forbidden") - if content_type is None: - content_type = 'application/octet-stream' - self.content_type = content_type + if content_type: + self.content_type = content_type if charset: self.charset = charset self.body = body From dbe75ac7be31fd441f66d5ac148e4133a58c1535 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 20:05:54 +0530 Subject: [PATCH 6/8] Update affected tests and add more --- tests/test_client_functional.py | 3 ++- tests/test_web_response.py | 40 ++++++++++++++++++++++++++------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index 5e5a67e8e8d..cdbe0f96dce 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -450,7 +450,8 @@ def handler(request): app.router.add_route('GET', '/', handler) resp = yield from client.get('/') assert resp.status == 200 - assert resp.raw_headers == ((b'CONTENT-LENGTH', b'0'), + assert resp.raw_headers == ((b'CONTENT-TYPE', b'application/octet-stream'), + (b'CONTENT-LENGTH', b'0'), (b'DATE', mock.ANY), (b'SERVER', mock.ANY)) resp.close() diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 7dce54ad57a..9cb42b6386f 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -691,7 +691,9 @@ def test_response_ctor(): assert 'OK' == resp.reason assert resp.body is None assert 0 == resp.content_length - assert CIMultiDict([('CONTENT-LENGTH', '0')]) == resp.headers + assert (CIMultiDict([('CONTENT-TYPE', 'application/octet-stream'), + ('CONTENT-LENGTH', '0')]) == + resp.headers) def test_ctor_with_headers_and_status(): @@ -700,7 +702,9 @@ def test_ctor_with_headers_and_status(): assert 201 == resp.status assert b'body' == resp.body assert 4 == resp.content_length - assert (CIMultiDict([('AGE', '12'), ('CONTENT-LENGTH', '4')]) == + assert (CIMultiDict([('CONTENT-TYPE', 'application/octet-stream'), + ('AGE', '12'), + ('CONTENT-LENGTH', '4')]) == resp.headers) @@ -816,8 +820,11 @@ def append(data): yield from resp.prepare(req) yield from resp.write_eof() txt = buf.decode('utf8') - assert re.match('HTTP/1.1 200 OK\r\nContent-Length: 0\r\n' - 'Date: .+\r\nServer: .+\r\n\r\n', txt) + assert re.match('HTTP/1.1 200 OK\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 0\r\n' + 'Date: .+\r\n' + 'Server: .+\r\n\r\n', txt) @asyncio.coroutine @@ -838,8 +845,12 @@ def append(data): yield from resp.prepare(req) yield from resp.write_eof() txt = buf.decode('utf8') - assert re.match('HTTP/1.1 200 OK\r\nContent-Length: 4\r\n' - 'Date: .+\r\nServer: .+\r\n\r\ndata', txt) + assert re.match('HTTP/1.1 200 OK\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 4\r\n' + 'Date: .+\r\n' + 'Server: .+\r\n\r\n' + 'data', txt) @asyncio.coroutine @@ -861,9 +872,12 @@ def append(data): yield from resp.prepare(req) yield from resp.write_eof() txt = buf.decode('utf8') - assert re.match('HTTP/1.1 200 OK\r\nContent-Length: 0\r\n' + assert re.match('HTTP/1.1 200 OK\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 0\r\n' 'Set-Cookie: name=value\r\n' - 'Date: .+\r\nServer: .+\r\n\r\n', txt) + 'Date: .+\r\n' + 'Server: .+\r\n\r\n', txt) def test_set_text_with_content_type(): @@ -887,6 +901,16 @@ def test_set_text_with_charset(): assert "koi8-r" == resp.charset +def test_default_content_type_in_stream_response(): + resp = StreamResponse() + assert resp.content_type == 'application/octet-stream' + + +def test_default_content_type_in_response(): + resp = Response() + assert resp.content_type == 'application/octet-stream' + + def test_content_type_with_set_text(): resp = Response(text='text') assert resp.content_type == 'text/plain' From bcbf144bc5bb196dcff8bdb5c01873a7a15a1ad7 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 20:46:37 +0530 Subject: [PATCH 7/8] Fix order of headers --- tests/test_web_response.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 9cb42b6386f..7a3d59024fe 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -702,8 +702,8 @@ def test_ctor_with_headers_and_status(): assert 201 == resp.status assert b'body' == resp.body assert 4 == resp.content_length - assert (CIMultiDict([('CONTENT-TYPE', 'application/octet-stream'), - ('AGE', '12'), + assert (CIMultiDict([('AGE', '12'), + ('CONTENT-TYPE', 'application/octet-stream'), ('CONTENT-LENGTH', '4')]) == resp.headers) From ce207aa02d1ca68a2ebbcf07f1cf3df989644e7e Mon Sep 17 00:00:00 2001 From: Vamsi Krishna Avula Date: Fri, 26 Aug 2016 21:52:46 +0530 Subject: [PATCH 8/8] Check for Content-Type in local headers variable instead of self.headers --- aiohttp/web_reqrep.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/aiohttp/web_reqrep.py b/aiohttp/web_reqrep.py index d5339d81534..0e8260db860 100644 --- a/aiohttp/web_reqrep.py +++ b/aiohttp/web_reqrep.py @@ -778,8 +778,14 @@ def __init__(self, *, body=None, status=200, if body is not None and text is not None: raise ValueError("body and text are not allowed together") + content_type_header = False + if headers is not None: + search_key = hdrs.CONTENT_TYPE.lower() + content_type_header = any(search_key == key.lower() + for key in headers) + if text is not None: - if hdrs.CONTENT_TYPE not in self.headers: + if not content_type_header: # fast path for filling headers if not isinstance(text, str): raise TypeError("text argument must be str (%r)" % @@ -802,7 +808,7 @@ def __init__(self, *, body=None, status=200, "is forbidden") self.text = text else: - if hdrs.CONTENT_TYPE in self.headers: + if content_type_header: if content_type or charset: raise ValueError("passing both Content-Type header and " "content_type or charset params "