From e84757ad3caa8d9daec4ed93cf7d28e1b50b040a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 22 Feb 2016 20:16:17 +0100 Subject: [PATCH 01/27] Implemented simple sending cookie domain filter --- aiohttp/client.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index f18d9644f8e..57075a857a2 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -7,6 +7,7 @@ import sys import traceback import warnings +import re import http.cookies import urllib.parse @@ -24,6 +25,8 @@ PY_35 = sys.version_info >= (3, 5) +IPV4_PATTERN = re.compile(r"(\d{1,3}\.){3}\d{1,3}") + class ClientSession: """First-class interface for making HTTP requests.""" @@ -172,10 +175,13 @@ def _request(self, method, url, *, skip_headers.add(upstr(i)) while True: + + cookies = self._filter_cookies(url) + req = self._request_class( method, url, params=params, headers=headers, skip_auto_headers=skip_headers, data=data, - cookies=self.cookies, encoding=encoding, + cookies=cookies, encoding=encoding, auth=auth, version=version, compress=compress, chunked=chunked, expect100=expect100, loop=self._loop, response_class=self._response_class) @@ -353,10 +359,58 @@ def _update_cookies(self, cookies): if isinstance(value, http.cookies.Morsel): # use dict method because SimpleCookie class modifies value # before Python 3.4 + domain = value["domain"] + if domain.startswith("."): + value["domain"] = domain[1:] dict.__setitem__(self.cookies, name, value) else: self.cookies[name] = value + def _filter_cookies(self, url): + """Returns this session's cookies filtered by their attributes""" + # TODO: filter by 'expires', 'path', ... + netloc = urllib.parse.urlsplit(url).netloc + is_ip = IPV4_PATTERN.match(netloc) is not None + + filtered = http.cookies.SimpleCookie() + + for name, morsel in self.cookies.items(): + morsel_domain = morsel["domain"] + + if is_ip and morsel_domain: + # not requesting from a domain, don't send cookies that aren't shared + continue + + # Copy cookies with matching or empty (shared) domain + if not morsel_domain or self._is_domain_match(morsel_domain, netloc): + filtered[name] = morsel + + return filtered + + @staticmethod + def _is_domain_match(domain_string, string): + """Implements domain matching according to RFC 6265""" + if domain_string == string: + return True + + if not string.endswith(domain_string): + return False + + rest = string[:-len(domain_string)] + + if not rest: + return False + + if rest[-1] != ".": + return False + + netloc = urllib.parse.urlsplit(string).netloc + is_ip = IPV4_PATTERN.match(netloc) is not None + if is_ip: + return False + + return True + def _prepare_headers(self, headers): """ Add default headers and transform it to CIMultiDict """ From 67f278d9a11669ca97a96b33cb643f069269bfff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 22 Feb 2016 20:16:42 +0100 Subject: [PATCH 02/27] Added tests for simple sending cookie domain filter --- tests/test_client_session.py | 96 ++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/tests/test_client_session.py b/tests/test_client_session.py index e6188f9be30..e67cf58d522 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -3,10 +3,12 @@ import gc import re import types +import http.cookies from unittest import mock import aiohttp import pytest +from aiohttp import test_utils from aiohttp.client import ClientSession from aiohttp.connector import BaseConnector, TCPConnector from aiohttp.multidict import CIMultiDict, MultiDict @@ -368,3 +370,97 @@ def test_request_ctx_manager_props(loop): assert isinstance(ctx_mgr.gi_frame, types.FrameType) assert not ctx_mgr.gi_running assert isinstance(ctx_mgr.gi_code, types.CodeType) + + +@pytest.fixture +def get_with_cookies(create_session): + cookie_string = """ + shared-cookie=first; + domain-cookie=second; domain=example.com; + subdomain1-cookie=third; domain=test1.example.com; + subdomain2-cookie=fourth; domain=test2.example.com; + dotted-domain-cookie=fifth; domain=.example.com; + different-domain-cookie=sixth; domain=different.org; + """ + + err = NotImplementedError() + + req = mock.Mock() + req_factory = mock.Mock(return_value=req) + + req.send = mock.Mock(side_effect=err) + + session = create_session( + request_class=req_factory, + cookies=http.cookies.SimpleCookie(cookie_string)) + + @asyncio.coroutine + def create_connection(req): + # return self.transport, self.protocol + return mock.Mock(), mock.Mock() + session._connector._create_connection = create_connection + + return session.get, req_factory + + +@pytest.fixture +def send_cookie_request(get_with_cookies): + get, req = get_with_cookies + + @asyncio.coroutine + def maker(*args, **kwargs): + with pytest.raises(NotImplementedError): + yield from get(*args, **kwargs) + + cookies = req.call_args[1]["cookies"] + return cookies + return maker + + +@pytest.mark.run_loop +def test_cookie_domain_filter_ip(send_cookie_request): + cookies = yield from send_cookie_request("http://1.2.3.4/") + assert set(cookies.keys()) == { + "shared-cookie" + } + + +@pytest.mark.run_loop +def test_cookie_domain_filter_same_host(send_cookie_request): + cookies = yield from send_cookie_request("http://example.com/") + assert set(cookies.keys()) == { + "shared-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + +@pytest.mark.run_loop +def test_cookie_domain_filter_same_host_and_subdomain(send_cookie_request): + cookies = yield from send_cookie_request("http://test1.example.com/") + assert set(cookies.keys()) == { + "shared-cookie", + "domain-cookie", + "subdomain1-cookie", + "dotted-domain-cookie" + } + + +@pytest.mark.run_loop +def test_cookie_domain_filter_same_host_different_subdomain(send_cookie_request): + cookies = yield from send_cookie_request("http://different.example.com/") + assert set(cookies.keys()) == { + "shared-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + +@pytest.mark.run_loop +def test_cookie_domain_filter_different_host(send_cookie_request): + cookies = yield from send_cookie_request("http://different.org/") + assert set(cookies.keys()) == { + "shared-cookie", + "different-domain-cookie" + } + From eb8ff5c13b1186f2fc58c8a482746dd3b5057596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 22 Feb 2016 21:09:24 +0100 Subject: [PATCH 03/27] Fixed flake8 errors --- aiohttp/client.py | 7 +++++-- tests/test_client_session.py | 6 ++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 57075a857a2..9dda5b85a40 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -378,11 +378,14 @@ def _filter_cookies(self, url): morsel_domain = morsel["domain"] if is_ip and morsel_domain: - # not requesting from a domain, don't send cookies that aren't shared + # not requesting from a domain, + # don't send cookies that aren't shared continue # Copy cookies with matching or empty (shared) domain - if not morsel_domain or self._is_domain_match(morsel_domain, netloc): + if ( + not morsel_domain or + self._is_domain_match(morsel_domain, netloc)): filtered[name] = morsel return filtered diff --git a/tests/test_client_session.py b/tests/test_client_session.py index e67cf58d522..a780a2a99d7 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -8,7 +8,6 @@ import aiohttp import pytest -from aiohttp import test_utils from aiohttp.client import ClientSession from aiohttp.connector import BaseConnector, TCPConnector from aiohttp.multidict import CIMultiDict, MultiDict @@ -447,7 +446,7 @@ def test_cookie_domain_filter_same_host_and_subdomain(send_cookie_request): @pytest.mark.run_loop -def test_cookie_domain_filter_same_host_different_subdomain(send_cookie_request): +def test_cookie_domain_filter_same_host_diff_subdomain(send_cookie_request): cookies = yield from send_cookie_request("http://different.example.com/") assert set(cookies.keys()) == { "shared-cookie", @@ -457,10 +456,9 @@ def test_cookie_domain_filter_same_host_different_subdomain(send_cookie_request) @pytest.mark.run_loop -def test_cookie_domain_filter_different_host(send_cookie_request): +def test_cookie_domain_filter_diff_host(send_cookie_request): cookies = yield from send_cookie_request("http://different.org/") assert set(cookies.keys()) == { "shared-cookie", "different-domain-cookie" } - From 1def5b0939fbc8a75b0ca953389866d543dc7033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 22 Feb 2016 21:15:05 +0100 Subject: [PATCH 04/27] Using ipaddress module to determine if a netloc is an IP address --- aiohttp/client.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 9dda5b85a40..6200ee41148 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -3,11 +3,11 @@ import asyncio import base64 import hashlib +import ipaddress import os import sys import traceback import warnings -import re import http.cookies import urllib.parse @@ -25,8 +25,6 @@ PY_35 = sys.version_info >= (3, 5) -IPV4_PATTERN = re.compile(r"(\d{1,3}\.){3}\d{1,3}") - class ClientSession: """First-class interface for making HTTP requests.""" @@ -370,7 +368,11 @@ def _filter_cookies(self, url): """Returns this session's cookies filtered by their attributes""" # TODO: filter by 'expires', 'path', ... netloc = urllib.parse.urlsplit(url).netloc - is_ip = IPV4_PATTERN.match(netloc) is not None + is_ip = True + try: + ipaddress.ip_address(netloc) + except ValueError: + is_ip = False filtered = http.cookies.SimpleCookie() @@ -408,7 +410,12 @@ def _is_domain_match(domain_string, string): return False netloc = urllib.parse.urlsplit(string).netloc - is_ip = IPV4_PATTERN.match(netloc) is not None + is_ip = True + try: + ipaddress.ip_address(netloc) + except ValueError: + is_ip = False + if is_ip: return False From fbe82c4bc78ec70572655652631320353d527451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 22 Feb 2016 22:02:29 +0100 Subject: [PATCH 05/27] Added workaround somewhere else commented "use dict method because SimpleCookie class modifies value before Python 3.4" --- aiohttp/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 6200ee41148..6cbf97f5ba2 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -388,7 +388,7 @@ def _filter_cookies(self, url): if ( not morsel_domain or self._is_domain_match(morsel_domain, netloc)): - filtered[name] = morsel + dict.__setitem__(filtered, name, morsel) return filtered From 4bd4a4e0436507bbeb6e55a22682934e4acf0597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 19:44:36 +0100 Subject: [PATCH 06/27] Fixed errors with ports in URLs by using hostname instead of netloc; Fixed handling starting dot in cookie domains --- aiohttp/client.py | 50 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 6cbf97f5ba2..f778a9e097d 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -199,7 +199,7 @@ def _request(self, method, url, *, except OSError as exc: raise aiohttp.ClientOSError(*exc.args) from exc - self._update_cookies(resp.cookies) + self._update_cookies(resp.cookies, resp.url) # For Backward compatability with `share_cookie` connectors if self._connector._share_cookies: self._connector.update_cookies(resp.cookies) @@ -348,29 +348,51 @@ def _ws_connect(self, url, *, autoping, self._loop) - def _update_cookies(self, cookies): + def _update_cookies(self, cookies, url=None): """Update shared cookies.""" + hostname = "" + if url is not None: + hostname = urllib.parse.urlsplit(url).hostname or "" + if isinstance(cookies, dict): cookies = cookies.items() for name, value in cookies: if isinstance(value, http.cookies.Morsel): + + cookie_domain = value["domain"] + if cookie_domain: + + if cookie_domain.startswith("."): + # Remove leading dot + cookie_domain = cookie_domain[1:] + value["domain"] = cookie_domain + + if ( + url is not None and + not self._is_domain_match( + cookie_domain, hostname)): + # Setting cookies for different domains is not allowed + continue + # use dict method because SimpleCookie class modifies value # before Python 3.4 - domain = value["domain"] - if domain.startswith("."): - value["domain"] = domain[1:] dict.__setitem__(self.cookies, name, value) + else: self.cookies[name] = value + if not self.cookies[name]["domain"] and url is not None: + # Set the cookie's domain to the response hostname + self.cookies[name]["domain"] = hostname + def _filter_cookies(self, url): """Returns this session's cookies filtered by their attributes""" # TODO: filter by 'expires', 'path', ... - netloc = urllib.parse.urlsplit(url).netloc + hostname = urllib.parse.urlsplit(url).hostname or "" is_ip = True try: - ipaddress.ip_address(netloc) + ipaddress.ip_address(hostname) except ValueError: is_ip = False @@ -387,21 +409,21 @@ def _filter_cookies(self, url): # Copy cookies with matching or empty (shared) domain if ( not morsel_domain or - self._is_domain_match(morsel_domain, netloc)): + self._is_domain_match(morsel_domain, hostname)): dict.__setitem__(filtered, name, morsel) return filtered @staticmethod - def _is_domain_match(domain_string, string): + def _is_domain_match(domain, hostname): """Implements domain matching according to RFC 6265""" - if domain_string == string: + if hostname == domain: return True - if not string.endswith(domain_string): + if not hostname.endswith(domain): return False - rest = string[:-len(domain_string)] + rest = hostname[:-len(domain)] if not rest: return False @@ -409,10 +431,8 @@ def _is_domain_match(domain_string, string): if rest[-1] != ".": return False - netloc = urllib.parse.urlsplit(string).netloc - is_ip = True try: - ipaddress.ip_address(netloc) + ipaddress.ip_address(hostname) except ValueError: is_ip = False From 87bad1effc04c4aa17cc1a2a877d88b1aa511ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 14:34:03 +0100 Subject: [PATCH 07/27] Fixed not accepting cookies from IPs --- aiohttp/client.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/aiohttp/client.py b/aiohttp/client.py index f778a9e097d..1618508cbd5 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -354,6 +354,14 @@ def _update_cookies(self, cookies, url=None): if url is not None: hostname = urllib.parse.urlsplit(url).hostname or "" + try: + ipaddress.ip_address(hostname) + except ValueError: + pass + else: + # Don't take cookies from IPs + return + if isinstance(cookies, dict): cookies = cookies.items() From 3088e093652b5d376f1452128aaa4cb5272ff883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 08:47:28 +0100 Subject: [PATCH 08/27] Removed impossible condition --- aiohttp/client.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 1618508cbd5..ede31467341 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -433,9 +433,6 @@ def _is_domain_match(domain, hostname): rest = hostname[:-len(domain)] - if not rest: - return False - if rest[-1] != ".": return False From c7a9efe3bd314b784a00f970fb0e8c504d387b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 15:33:29 +0100 Subject: [PATCH 09/27] Changed an obsolete test, where explicit cookie sharing is now required --- tests/test_client_functional_oldstyle.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_client_functional_oldstyle.py b/tests/test_client_functional_oldstyle.py index 4cadfd1b790..68835d88cad 100644 --- a/tests/test_client_functional_oldstyle.py +++ b/tests/test_client_functional_oldstyle.py @@ -1144,9 +1144,12 @@ def test_session_cookies(self, m_log): session.request('get', httpd.url('cookies'))) self.assertEqual(resp.cookies['c1'].value, 'cookie1') self.assertEqual(resp.cookies['c2'].value, 'cookie2') - self.assertEqual(session.cookies, resp.cookies) resp.close() + # Add the received cookies as shared for sending them to the test + # server, which is only accessible via IP + session.cookies.update(resp.cookies) + # Assert, that we send those cookies in next requests r = self.loop.run_until_complete( session.request('get', httpd.url('method', 'get'))) From 8720b8eec107dcff4d6452259bdfc3f7f6096e9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 14:33:34 +0100 Subject: [PATCH 10/27] Added tests for filtering of received cookies --- tests/test_client_session.py | 115 ++++++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 28 deletions(-) diff --git a/tests/test_client_session.py b/tests/test_client_session.py index a780a2a99d7..1a97a93d292 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -373,25 +373,48 @@ def test_request_ctx_manager_props(loop): @pytest.fixture def get_with_cookies(create_session): - cookie_string = """ - shared-cookie=first; - domain-cookie=second; domain=example.com; - subdomain1-cookie=third; domain=test1.example.com; - subdomain2-cookie=fourth; domain=test2.example.com; - dotted-domain-cookie=fifth; domain=.example.com; - different-domain-cookie=sixth; domain=different.org; - """ - - err = NotImplementedError() + # Cookies to send from client to server as "Cookie" header + cookies_to_send = ( + "shared-cookie=first; " + "domain-cookie=second; Domain=example.com; " + "subdomain1-cookie=third; Domain=test1.example.com; " + "subdomain2-cookie=fourth; Domain=test2.example.com; " + "dotted-domain-cookie=fifth; Domain=.example.com; " + "different-domain-cookie=sixth; Domain=different.org; " + ) + + # Cookies received from the server as "Set-Cookie" header + cookies_to_receive = ( + "unconstrained-cookie=first; " + "domain-cookie=second; Domain=example.com; " + "subdomain1-cookie=third; Domain=test1.example.com; " + "subdomain2-cookie=fourth; Domain=test2.example.com; " + "dotted-domain-cookie=fifth; Domain=.example.com; " + "different-domain-cookie=sixth; Domain=different.org; " + ) req = mock.Mock() req_factory = mock.Mock(return_value=req) + resp = mock.Mock() + resp.cookies = http.cookies.SimpleCookie(cookies_to_receive) - req.send = mock.Mock(side_effect=err) + def send(writer, reader): + # Clear the cookies between send and receive + session.cookies.clear() + # Reply with the requested URL + resp.url = req_factory.call_args[0][1] + return resp + req.send = send + + @asyncio.coroutine + def start(connection, read_until_eof=False): + connection.close() + return resp + resp.start = start session = create_session( request_class=req_factory, - cookies=http.cookies.SimpleCookie(cookie_string)) + cookies=http.cookies.SimpleCookie(cookies_to_send)) @asyncio.coroutine def create_connection(req): @@ -399,66 +422,102 @@ def create_connection(req): return mock.Mock(), mock.Mock() session._connector._create_connection = create_connection - return session.get, req_factory + return session, req_factory @pytest.fixture def send_cookie_request(get_with_cookies): - get, req = get_with_cookies + session, req = get_with_cookies @asyncio.coroutine def maker(*args, **kwargs): - with pytest.raises(NotImplementedError): - yield from get(*args, **kwargs) + yield from session.get(*args, **kwargs) - cookies = req.call_args[1]["cookies"] - return cookies + cookies_sent = req.call_args[1]["cookies"] + cookies_received = session.cookies + return cookies_sent, cookies_received return maker @pytest.mark.run_loop def test_cookie_domain_filter_ip(send_cookie_request): - cookies = yield from send_cookie_request("http://1.2.3.4/") - assert set(cookies.keys()) == { + cookies_sent, cookies_received = ( + yield from send_cookie_request("http://1.2.3.4/")) + + assert set(cookies_sent.keys()) == { "shared-cookie" } + assert set(cookies_received.keys()) == set() + @pytest.mark.run_loop def test_cookie_domain_filter_same_host(send_cookie_request): - cookies = yield from send_cookie_request("http://example.com/") - assert set(cookies.keys()) == { + cookies_sent, cookies_received = ( + yield from send_cookie_request("http://example.com/")) + + assert set(cookies_sent.keys()) == { "shared-cookie", "domain-cookie", "dotted-domain-cookie" } + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + @pytest.mark.run_loop def test_cookie_domain_filter_same_host_and_subdomain(send_cookie_request): - cookies = yield from send_cookie_request("http://test1.example.com/") - assert set(cookies.keys()) == { + cookies_sent, cookies_received = ( + yield from send_cookie_request("http://test1.example.com/")) + + assert set(cookies_sent.keys()) == { "shared-cookie", "domain-cookie", "subdomain1-cookie", "dotted-domain-cookie" } + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "subdomain1-cookie", + "dotted-domain-cookie" + } + @pytest.mark.run_loop def test_cookie_domain_filter_same_host_diff_subdomain(send_cookie_request): - cookies = yield from send_cookie_request("http://different.example.com/") - assert set(cookies.keys()) == { + cookies_sent, cookies_received = ( + yield from send_cookie_request("http://different.example.com/")) + + assert set(cookies_sent.keys()) == { "shared-cookie", "domain-cookie", "dotted-domain-cookie" } + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + @pytest.mark.run_loop def test_cookie_domain_filter_diff_host(send_cookie_request): - cookies = yield from send_cookie_request("http://different.org/") - assert set(cookies.keys()) == { + cookies_sent, cookies_received = ( + yield from send_cookie_request("http://different.org/")) + + assert set(cookies_sent.keys()) == { "shared-cookie", "different-domain-cookie" } + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "different-domain-cookie" + } From f8812d513ff81d9d997ffe0f61f0b0e8693b47d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Tue, 23 Feb 2016 16:15:15 +0100 Subject: [PATCH 11/27] Implemented host-only-flag --- aiohttp/client.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index ede31467341..9c00d5d7341 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -53,6 +53,7 @@ def __init__(self, *, connector=None, loop=None, cookies=None, self._source_traceback = traceback.extract_stack(sys._getframe(1)) self._cookies = http.cookies.SimpleCookie() + self._host_only_cookies = set() # For Backward compatability with `share_cookies` connectors if connector._share_cookies: @@ -390,9 +391,13 @@ def _update_cookies(self, cookies, url=None): else: self.cookies[name] = value - if not self.cookies[name]["domain"] and url is not None: + cookie = self.cookies[name] + + if not cookie["domain"] and url is not None: # Set the cookie's domain to the response hostname - self.cookies[name]["domain"] = hostname + # and set its host-only-flag + self._host_only_cookies.add(name) + cookie["domain"] = hostname def _filter_cookies(self, url): """Returns this session's cookies filtered by their attributes""" @@ -406,19 +411,24 @@ def _filter_cookies(self, url): filtered = http.cookies.SimpleCookie() - for name, morsel in self.cookies.items(): - morsel_domain = morsel["domain"] + for name, cookie in self.cookies.items(): + cookie_domain = cookie["domain"] + + # Send shared cookies + if not cookie_domain: + dict.__setitem__(filtered, name, cookie) + continue + + if is_ip: + continue - if is_ip and morsel_domain: - # not requesting from a domain, - # don't send cookies that aren't shared + if name in self._host_only_cookies: + if cookie_domain != hostname: + continue + elif not self._is_domain_match(cookie_domain, hostname): continue - # Copy cookies with matching or empty (shared) domain - if ( - not morsel_domain or - self._is_domain_match(morsel_domain, hostname)): - dict.__setitem__(filtered, name, morsel) + dict.__setitem__(filtered, name, cookie) return filtered From 90bb61be16e3807c8a4b0f3f279abadd4b646983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Wed, 24 Feb 2016 19:04:20 +0100 Subject: [PATCH 12/27] Refactoring; Moved new code over to the new class helpers.SessionCookieStore, which ClientSession refers to --- aiohttp/client.py | 124 +++------------------------------------------ aiohttp/helpers.py | 124 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 117 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 9c00d5d7341..da7fd0dc412 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -3,17 +3,16 @@ import asyncio import base64 import hashlib -import ipaddress import os import sys import traceback import warnings -import http.cookies import urllib.parse import aiohttp from .client_reqrep import ClientRequest, ClientResponse from .errors import WSServerHandshakeError +from .helpers import SessionCookieStore from .multidict import MultiDictProxy, MultiDict, CIMultiDict, upstr from .websocket import WS_KEY, WebSocketParser, WebSocketWriter from .websocket_client import ClientWebSocketResponse @@ -52,14 +51,13 @@ def __init__(self, *, connector=None, loop=None, cookies=None, if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) - self._cookies = http.cookies.SimpleCookie() - self._host_only_cookies = set() + self._cookie_store = SessionCookieStore() # For Backward compatability with `share_cookies` connectors if connector._share_cookies: - self._update_cookies(connector.cookies) + self._cookie_store.update_cookies(connector.cookies) if cookies is not None: - self._update_cookies(cookies) + self._cookie_store.update_cookies(cookies) self._connector = connector self._default_auth = auth self._version = version @@ -175,7 +173,7 @@ def _request(self, method, url, *, while True: - cookies = self._filter_cookies(url) + cookies = self._cookie_store.filter_cookies(url) req = self._request_class( method, url, params=params, headers=headers, @@ -200,7 +198,8 @@ def _request(self, method, url, *, except OSError as exc: raise aiohttp.ClientOSError(*exc.args) from exc - self._update_cookies(resp.cookies, resp.url) + self._cookie_store.update_cookies(resp.cookies, resp.url) + # For Backward compatability with `share_cookie` connectors if self._connector._share_cookies: self._connector.update_cookies(resp.cookies) @@ -349,113 +348,6 @@ def _ws_connect(self, url, *, autoping, self._loop) - def _update_cookies(self, cookies, url=None): - """Update shared cookies.""" - hostname = "" - if url is not None: - hostname = urllib.parse.urlsplit(url).hostname or "" - - try: - ipaddress.ip_address(hostname) - except ValueError: - pass - else: - # Don't take cookies from IPs - return - - if isinstance(cookies, dict): - cookies = cookies.items() - - for name, value in cookies: - if isinstance(value, http.cookies.Morsel): - - cookie_domain = value["domain"] - if cookie_domain: - - if cookie_domain.startswith("."): - # Remove leading dot - cookie_domain = cookie_domain[1:] - value["domain"] = cookie_domain - - if ( - url is not None and - not self._is_domain_match( - cookie_domain, hostname)): - # Setting cookies for different domains is not allowed - continue - - # use dict method because SimpleCookie class modifies value - # before Python 3.4 - dict.__setitem__(self.cookies, name, value) - - else: - self.cookies[name] = value - - cookie = self.cookies[name] - - if not cookie["domain"] and url is not None: - # Set the cookie's domain to the response hostname - # and set its host-only-flag - self._host_only_cookies.add(name) - cookie["domain"] = hostname - - def _filter_cookies(self, url): - """Returns this session's cookies filtered by their attributes""" - # TODO: filter by 'expires', 'path', ... - hostname = urllib.parse.urlsplit(url).hostname or "" - is_ip = True - try: - ipaddress.ip_address(hostname) - except ValueError: - is_ip = False - - filtered = http.cookies.SimpleCookie() - - for name, cookie in self.cookies.items(): - cookie_domain = cookie["domain"] - - # Send shared cookies - if not cookie_domain: - dict.__setitem__(filtered, name, cookie) - continue - - if is_ip: - continue - - if name in self._host_only_cookies: - if cookie_domain != hostname: - continue - elif not self._is_domain_match(cookie_domain, hostname): - continue - - dict.__setitem__(filtered, name, cookie) - - return filtered - - @staticmethod - def _is_domain_match(domain, hostname): - """Implements domain matching according to RFC 6265""" - if hostname == domain: - return True - - if not hostname.endswith(domain): - return False - - rest = hostname[:-len(domain)] - - if rest[-1] != ".": - return False - - try: - ipaddress.ip_address(hostname) - except ValueError: - is_ip = False - - if is_ip: - return False - - return True - def _prepare_headers(self, headers): """ Add default headers and transform it to CIMultiDict """ @@ -549,7 +441,7 @@ def connector(self): @property def cookies(self): """The session cookies.""" - return self._cookies + return self._cookie_store.cookies @property def version(self): diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index be03975a145..4650abfa20e 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -5,9 +5,11 @@ import datetime import functools import io +import ipaddress import os import re -from urllib.parse import quote, urlencode +from urllib.parse import quote, urlencode, urlsplit +from http.cookies import SimpleCookie, Morsel from collections import namedtuple from pathlib import Path @@ -449,6 +451,14 @@ def requote_uri(uri): return quote(uri, safe=safe_without_percent) +def is_ip_address(hostname): + try: + ipaddress.ip_address(hostname) + except ValueError: + return False + return True + + class Timeout: """Timeout context manager. @@ -492,3 +502,115 @@ def __exit__(self, exc_type, exc_val, exc_tb): def _cancel_task(self): self._cancelled = self._task.cancel() + + +class SessionCookieStore: + """Implements cookie storage adhering to RFC 6265.""" + + def __init__(self, cookies=None): + self._cookies = SimpleCookie() + self._host_only_cookies = set() + + if cookies is not None: + self.update_cookies(cookies) + + @property + def cookies(self): + """The session cookies.""" + return self._cookies + + def update_cookies(self, cookies, response_url=None): + """Update cookies.""" + hostname = urlsplit(response_url).hostname + + if is_ip_address(hostname): + # Don't accept cookies from IPs + return + + if isinstance(cookies, dict): + cookies = cookies.items() + + for name, value in cookies: + if isinstance(value, Morsel): + + if not self._add_morsel(name, value, hostname): + continue + + else: + self._cookies[name] = value + + cookie = self._cookies[name] + + if not cookie["domain"] and hostname is not None: + # Set the cookie's domain to the response hostname + # and set its host-only-flag + self._host_only_cookies.add(name) + cookie["domain"] = hostname + + # Remove the host-only flags of nonexistent cookies + self._host_only_cookies -= ( + self._host_only_cookies.difference(self._cookies.keys())) + + def _add_morsel(self, name, value, hostname): + """Add a Morsel to the cookie store.""" + cookie_domain = value["domain"] + if cookie_domain.startswith("."): + # Remove leading dot + cookie_domain = cookie_domain[1:] + value["domain"] = cookie_domain + + if not cookie_domain or not hostname: + dict.__setitem__(self._cookies, name, value) + return True + + if not self._is_domain_match(cookie_domain, hostname): + # Setting cookies for different domains is not allowed + return False + + # use dict method because SimpleCookie class modifies value + # before Python 3.4 + dict.__setitem__(self._cookies, name, value) + return True + + def filter_cookies(self, request_url): + """Returns this store's cookies filtered by their attributes.""" + # TODO: filter by 'expires', 'path', ... + hostname = urlsplit(request_url).hostname or "" + filtered = SimpleCookie() + + for name, cookie in self._cookies.items(): + cookie_domain = cookie["domain"] + + # Send shared cookies + if not cookie_domain: + dict.__setitem__(filtered, name, cookie) + continue + + if is_ip_address(hostname): + continue + + if name in self._host_only_cookies: + if cookie_domain != hostname: + continue + elif not self._is_domain_match(cookie_domain, hostname): + continue + + dict.__setitem__(filtered, name, cookie) + + return filtered + + @staticmethod + def _is_domain_match(domain, hostname): + """Implements domain matching adhering to RFC 6265.""" + if hostname == domain: + return True + + if not hostname.endswith(domain): + return False + + rest = hostname[:-len(domain)] + + if rest[-1] != ".": + return False + + return not is_ip_address(hostname) From 7a19859b30a4c41691d31f7c57b212cec70fd05b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Wed, 24 Feb 2016 19:13:34 +0100 Subject: [PATCH 13/27] Split the tests into testing calls to SessionCookieStore and the filtering methods inside it --- tests/test_client_session.py | 154 +++++------------------------------ tests/test_helpers.py | 117 +++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 133 deletions(-) diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 1a97a93d292..301871990d5 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -371,50 +371,35 @@ def test_request_ctx_manager_props(loop): assert isinstance(ctx_mgr.gi_code, types.CodeType) -@pytest.fixture -def get_with_cookies(create_session): - # Cookies to send from client to server as "Cookie" header - cookies_to_send = ( - "shared-cookie=first; " - "domain-cookie=second; Domain=example.com; " - "subdomain1-cookie=third; Domain=test1.example.com; " - "subdomain2-cookie=fourth; Domain=test2.example.com; " - "dotted-domain-cookie=fifth; Domain=.example.com; " - "different-domain-cookie=sixth; Domain=different.org; " - ) - - # Cookies received from the server as "Set-Cookie" header - cookies_to_receive = ( - "unconstrained-cookie=first; " - "domain-cookie=second; Domain=example.com; " - "subdomain1-cookie=third; Domain=test1.example.com; " - "subdomain2-cookie=fourth; Domain=test2.example.com; " - "dotted-domain-cookie=fifth; Domain=.example.com; " - "different-domain-cookie=sixth; Domain=different.org; " - ) +@pytest.mark.run_loop +def test_cookie_store_usage(create_session): + url = "http://example.com/" req = mock.Mock() req_factory = mock.Mock(return_value=req) resp = mock.Mock() - resp.cookies = http.cookies.SimpleCookie(cookies_to_receive) + resp.url = url + resp.cookies = http.cookies.SimpleCookie("reply=value") + req_cookies = http.cookies.SimpleCookie("request=value") + + store = mock.Mock() + + def req_send(writer, reader): + assert store.filter_cookies.called + assert store.filter_cookies.call_args[0] == (url,) + store.reset_mock() - def send(writer, reader): - # Clear the cookies between send and receive - session.cookies.clear() - # Reply with the requested URL - resp.url = req_factory.call_args[0][1] return resp - req.send = send + req.send = req_send @asyncio.coroutine - def start(connection, read_until_eof=False): + def resp_start(connection, read_until_eof=False): connection.close() return resp - resp.start = start + resp.start = resp_start - session = create_session( - request_class=req_factory, - cookies=http.cookies.SimpleCookie(cookies_to_send)) + session = create_session(request_class=req_factory, cookies=req_cookies) + assert set(session.cookies.keys()) == {"request"} @asyncio.coroutine def create_connection(req): @@ -422,102 +407,7 @@ def create_connection(req): return mock.Mock(), mock.Mock() session._connector._create_connection = create_connection - return session, req_factory - - -@pytest.fixture -def send_cookie_request(get_with_cookies): - session, req = get_with_cookies - - @asyncio.coroutine - def maker(*args, **kwargs): - yield from session.get(*args, **kwargs) - - cookies_sent = req.call_args[1]["cookies"] - cookies_received = session.cookies - return cookies_sent, cookies_received - return maker - - -@pytest.mark.run_loop -def test_cookie_domain_filter_ip(send_cookie_request): - cookies_sent, cookies_received = ( - yield from send_cookie_request("http://1.2.3.4/")) - - assert set(cookies_sent.keys()) == { - "shared-cookie" - } - - assert set(cookies_received.keys()) == set() - - -@pytest.mark.run_loop -def test_cookie_domain_filter_same_host(send_cookie_request): - cookies_sent, cookies_received = ( - yield from send_cookie_request("http://example.com/")) - - assert set(cookies_sent.keys()) == { - "shared-cookie", - "domain-cookie", - "dotted-domain-cookie" - } - - assert set(cookies_received.keys()) == { - "unconstrained-cookie", - "domain-cookie", - "dotted-domain-cookie" - } - - -@pytest.mark.run_loop -def test_cookie_domain_filter_same_host_and_subdomain(send_cookie_request): - cookies_sent, cookies_received = ( - yield from send_cookie_request("http://test1.example.com/")) - - assert set(cookies_sent.keys()) == { - "shared-cookie", - "domain-cookie", - "subdomain1-cookie", - "dotted-domain-cookie" - } - - assert set(cookies_received.keys()) == { - "unconstrained-cookie", - "domain-cookie", - "subdomain1-cookie", - "dotted-domain-cookie" - } - - -@pytest.mark.run_loop -def test_cookie_domain_filter_same_host_diff_subdomain(send_cookie_request): - cookies_sent, cookies_received = ( - yield from send_cookie_request("http://different.example.com/")) - - assert set(cookies_sent.keys()) == { - "shared-cookie", - "domain-cookie", - "dotted-domain-cookie" - } - - assert set(cookies_received.keys()) == { - "unconstrained-cookie", - "domain-cookie", - "dotted-domain-cookie" - } - - -@pytest.mark.run_loop -def test_cookie_domain_filter_diff_host(send_cookie_request): - cookies_sent, cookies_received = ( - yield from send_cookie_request("http://different.org/")) - - assert set(cookies_sent.keys()) == { - "shared-cookie", - "different-domain-cookie" - } - - assert set(cookies_received.keys()) == { - "unconstrained-cookie", - "different-domain-cookie" - } + session._cookie_store = store + yield from session.get(url) + assert store.update_cookies.called + assert store.update_cookies.call_args[0] == (resp.cookies, url) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index dbf954bdb43..81dea0f2477 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,7 +1,9 @@ import pytest +import unittest +import datetime +import http.cookies from unittest import mock from aiohttp import helpers -import datetime def test_parse_mimetype_1(): @@ -205,3 +207,116 @@ def test_requote_uri_properly_requotes(): # Ensure requoting doesn't break expectations. quoted = 'http://example.com/fiz?buz=%25ppicture' assert quoted == helpers.requote_uri(quoted) + + +class TestSessionCookieStore(unittest.TestCase): + + def setUp(self): + # Cookies to send from client to server as "Cookie" header + self.cookies_to_send = http.cookies.SimpleCookie( + "shared-cookie=first; " + "domain-cookie=second; Domain=example.com; " + "subdomain1-cookie=third; Domain=test1.example.com; " + "subdomain2-cookie=fourth; Domain=test2.example.com; " + "dotted-domain-cookie=fifth; Domain=.example.com; " + "different-domain-cookie=sixth; Domain=different.org; " + ) + + # Cookies received from the server as "Set-Cookie" header + self.cookies_to_receive = http.cookies.SimpleCookie( + "unconstrained-cookie=first; " + "domain-cookie=second; Domain=example.com; " + "subdomain1-cookie=third; Domain=test1.example.com; " + "subdomain2-cookie=fourth; Domain=test2.example.com; " + "dotted-domain-cookie=fifth; Domain=.example.com; " + "different-domain-cookie=sixth; Domain=different.org; " + ) + + self.store = helpers.SessionCookieStore(self.cookies_to_send) + + def request_reply_with_same_url(self, url): + cookies_sent = self.store.filter_cookies(url) + + self.store.cookies.clear() + + self.store.update_cookies(self.cookies_to_receive, url) + cookies_received = self.store.cookies + + return cookies_sent, cookies_received + + def test_constructor(self): + self.assertEqual(self.store.cookies, self.cookies_to_send) + + def test_domain_filter_ip(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://1.2.3.4/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie" + } + + assert set(cookies_received.keys()) == set() + + def test_domain_filter_same_host(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://example.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + def test_domain_filter_same_host_and_subdomain(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://test1.example.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "domain-cookie", + "subdomain1-cookie", + "dotted-domain-cookie" + } + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "subdomain1-cookie", + "dotted-domain-cookie" + } + + def test_domain_filter_same_host_diff_subdomain(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://different.example.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "domain-cookie", + "dotted-domain-cookie" + } + + def test_cookie_domain_filter_diff_host(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://different.org/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "different-domain-cookie" + } + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "different-domain-cookie" + } From 5d5dd5e7a4743cfd257511fca37667631ccad0d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 29 Feb 2016 20:35:11 +0100 Subject: [PATCH 14/27] Modified test_cookie_store_usage, now using mock.patch --- tests/test_client_session.py | 78 +++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 301871990d5..22951b1eac8 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -8,6 +8,7 @@ import aiohttp import pytest +from aiohttp import web from aiohttp.client import ClientSession from aiohttp.connector import BaseConnector, TCPConnector from aiohttp.multidict import CIMultiDict, MultiDict @@ -372,42 +373,53 @@ def test_request_ctx_manager_props(loop): @pytest.mark.run_loop -def test_cookie_store_usage(create_session): - url = "http://example.com/" +def test_cookie_store_usage(create_app_and_client): + req_url = None - req = mock.Mock() - req_factory = mock.Mock(return_value=req) - resp = mock.Mock() - resp.url = url - resp.cookies = http.cookies.SimpleCookie("reply=value") - req_cookies = http.cookies.SimpleCookie("request=value") + init_mock = mock.Mock(return_value=None) + update_mock = mock.Mock(return_value=None) + filter_mock = mock.Mock(return_value=None) - store = mock.Mock() - - def req_send(writer, reader): - assert store.filter_cookies.called - assert store.filter_cookies.call_args[0] == (url,) - store.reset_mock() - - return resp - req.send = req_send + patches = mock.patch.multiple( + "aiohttp.helpers.SessionCookieStore", + __init__=init_mock, + update_cookies=update_mock, + filter_cookies=filter_mock, + ) @asyncio.coroutine - def resp_start(connection, read_until_eof=False): - connection.close() - return resp - resp.start = resp_start - - session = create_session(request_class=req_factory, cookies=req_cookies) - assert set(session.cookies.keys()) == {"request"} + def handler(request): + nonlocal req_url + req_url = "http://%s/" % request.host - @asyncio.coroutine - def create_connection(req): - # return self.transport, self.protocol - return mock.Mock(), mock.Mock() - session._connector._create_connection = create_connection + resp = web.Response() + resp.set_cookie("response", "resp_value") + return resp - session._cookie_store = store - yield from session.get(url) - assert store.update_cookies.called - assert store.update_cookies.call_args[0] == (resp.cookies, url) + with patches: + app, client = yield from create_app_and_client( + client_params={"cookies": {"request": "req_value"}} + ) + app.router.add_route('GET', '/', handler) + + # Updating the cookie store with initial user defined cookies + assert init_mock.called + assert update_mock.called + assert update_mock.call_args[0] == ( + {"request": "req_value"}, + ) + + update_mock.reset_mock() + yield from client.get("/") + + # Filtering the cookie store before sending the request, + # getting the request URL as only parameter + assert filter_mock.called + assert filter_mock.call_args[0] == (req_url,) + + # Updating the cookie store with the response cookies + assert update_mock.called + resp_cookies = update_mock.call_args[0][0] + assert isinstance(resp_cookies, http.cookies.SimpleCookie) + assert "response" in resp_cookies + assert resp_cookies["response"].value == "resp_value" From 0d3c2be57abd449db4d68daf5a8427060bec1ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 29 Feb 2016 20:37:53 +0100 Subject: [PATCH 15/27] Renamed SessionCookieStore to CookieJar --- aiohttp/client.py | 14 +++++++------- aiohttp/helpers.py | 2 +- tests/test_client_session.py | 10 +++++----- tests/test_helpers.py | 17 +++++++++-------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index da7fd0dc412..6320be84a93 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -12,7 +12,7 @@ import aiohttp from .client_reqrep import ClientRequest, ClientResponse from .errors import WSServerHandshakeError -from .helpers import SessionCookieStore +from .helpers import CookieJar from .multidict import MultiDictProxy, MultiDict, CIMultiDict, upstr from .websocket import WS_KEY, WebSocketParser, WebSocketWriter from .websocket_client import ClientWebSocketResponse @@ -51,13 +51,13 @@ def __init__(self, *, connector=None, loop=None, cookies=None, if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) - self._cookie_store = SessionCookieStore() + self._cookie_jar = CookieJar() # For Backward compatability with `share_cookies` connectors if connector._share_cookies: - self._cookie_store.update_cookies(connector.cookies) + self._cookie_jar.update_cookies(connector.cookies) if cookies is not None: - self._cookie_store.update_cookies(cookies) + self._cookie_jar.update_cookies(cookies) self._connector = connector self._default_auth = auth self._version = version @@ -173,7 +173,7 @@ def _request(self, method, url, *, while True: - cookies = self._cookie_store.filter_cookies(url) + cookies = self._cookie_jar.filter_cookies(url) req = self._request_class( method, url, params=params, headers=headers, @@ -198,7 +198,7 @@ def _request(self, method, url, *, except OSError as exc: raise aiohttp.ClientOSError(*exc.args) from exc - self._cookie_store.update_cookies(resp.cookies, resp.url) + self._cookie_jar.update_cookies(resp.cookies, resp.url) # For Backward compatability with `share_cookie` connectors if self._connector._share_cookies: @@ -441,7 +441,7 @@ def connector(self): @property def cookies(self): """The session cookies.""" - return self._cookie_store.cookies + return self._cookie_jar.cookies @property def version(self): diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 4650abfa20e..9af3780e325 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -504,7 +504,7 @@ def _cancel_task(self): self._cancelled = self._task.cancel() -class SessionCookieStore: +class CookieJar: """Implements cookie storage adhering to RFC 6265.""" def __init__(self, cookies=None): diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 22951b1eac8..8510cfc5bb7 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -373,7 +373,7 @@ def test_request_ctx_manager_props(loop): @pytest.mark.run_loop -def test_cookie_store_usage(create_app_and_client): +def test_cookie_jar_usage(create_app_and_client): req_url = None init_mock = mock.Mock(return_value=None) @@ -381,7 +381,7 @@ def test_cookie_store_usage(create_app_and_client): filter_mock = mock.Mock(return_value=None) patches = mock.patch.multiple( - "aiohttp.helpers.SessionCookieStore", + "aiohttp.helpers.CookieJar", __init__=init_mock, update_cookies=update_mock, filter_cookies=filter_mock, @@ -402,7 +402,7 @@ def handler(request): ) app.router.add_route('GET', '/', handler) - # Updating the cookie store with initial user defined cookies + # Updating the cookie jar with initial user defined cookies assert init_mock.called assert update_mock.called assert update_mock.call_args[0] == ( @@ -412,12 +412,12 @@ def handler(request): update_mock.reset_mock() yield from client.get("/") - # Filtering the cookie store before sending the request, + # Filtering the cookie jar before sending the request, # getting the request URL as only parameter assert filter_mock.called assert filter_mock.call_args[0] == (req_url,) - # Updating the cookie store with the response cookies + # Updating the cookie jar with the response cookies assert update_mock.called resp_cookies = update_mock.call_args[0][0] assert isinstance(resp_cookies, http.cookies.SimpleCookie) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 81dea0f2477..ca7d503be76 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -209,7 +209,7 @@ def test_requote_uri_properly_requotes(): assert quoted == helpers.requote_uri(quoted) -class TestSessionCookieStore(unittest.TestCase): +class TestCookieJar(unittest.TestCase): def setUp(self): # Cookies to send from client to server as "Cookie" header @@ -232,20 +232,21 @@ def setUp(self): "different-domain-cookie=sixth; Domain=different.org; " ) - self.store = helpers.SessionCookieStore(self.cookies_to_send) - def request_reply_with_same_url(self, url): - cookies_sent = self.store.filter_cookies(url) + self.jar = helpers.CookieJar(self.cookies_to_send) + + cookies_sent = self.jar.filter_cookies(url) - self.store.cookies.clear() + self.jar.cookies.clear() - self.store.update_cookies(self.cookies_to_receive, url) - cookies_received = self.store.cookies + self.jar.update_cookies(self.cookies_to_receive, url) + cookies_received = self.jar.cookies.copy() return cookies_sent, cookies_received def test_constructor(self): - self.assertEqual(self.store.cookies, self.cookies_to_send) + jar = helpers.CookieJar(self.cookies_to_send) + self.assertEqual(jar.cookies, self.cookies_to_send) def test_domain_filter_ip(self): cookies_sent, cookies_received = ( From 1d28248c4b98b67e6c4a1c9bfcbdbbcbd322ca8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 29 Feb 2016 20:38:16 +0100 Subject: [PATCH 16/27] Implemented filtering by secure-flag --- aiohttp/helpers.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 9af3780e325..0895a42fb5c 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -575,7 +575,7 @@ def _add_morsel(self, name, value, hostname): def filter_cookies(self, request_url): """Returns this store's cookies filtered by their attributes.""" # TODO: filter by 'expires', 'path', ... - hostname = urlsplit(request_url).hostname or "" + url_parsed = urlsplit(request_url) filtered = SimpleCookie() for name, cookie in self._cookies.items(): @@ -586,6 +586,8 @@ def filter_cookies(self, request_url): dict.__setitem__(filtered, name, cookie) continue + hostname = url_parsed.hostname or "" + if is_ip_address(hostname): continue @@ -595,6 +597,11 @@ def filter_cookies(self, request_url): elif not self._is_domain_match(cookie_domain, hostname): continue + is_secure = url_parsed.scheme in ("https", "wss") + + if cookie["secure"] and not is_secure: + continue + dict.__setitem__(filtered, name, cookie) return filtered From a648d148339611ef223b5a1d385c2bdd037510e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 29 Feb 2016 20:38:36 +0100 Subject: [PATCH 17/27] Added test for secure-flag --- tests/test_helpers.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index ca7d503be76..f3057c1566f 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -220,6 +220,7 @@ def setUp(self): "subdomain2-cookie=fourth; Domain=test2.example.com; " "dotted-domain-cookie=fifth; Domain=.example.com; " "different-domain-cookie=sixth; Domain=different.org; " + "secure-cookie=seventh: Domain=secure.com; Secure;" ) # Cookies received from the server as "Set-Cookie" header @@ -321,3 +322,19 @@ def test_cookie_domain_filter_diff_host(self): "unconstrained-cookie", "different-domain-cookie" } + + def test_cookie_secure_filter(self): + cookies_sent, _ = ( + self.request_reply_with_same_url("http://secure.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie" + } + + cookies_sent, _ = ( + self.request_reply_with_same_url("https://secure.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "secure-cookie" + } From bb559cd4bcb21e839ad859b2dad5566b56dfeabb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Mon, 29 Feb 2016 20:39:40 +0100 Subject: [PATCH 18/27] Added rudimentary test for is_ip_address() --- tests/test_helpers.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index f3057c1566f..5d280d0320b 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -209,6 +209,24 @@ def test_requote_uri_properly_requotes(): assert quoted == helpers.requote_uri(quoted) +def test_is_ip_address(): + assert helpers.is_ip_address("127.0.0.1") + assert helpers.is_ip_address("::1") + assert helpers.is_ip_address("FE80:0000:0000:0000:0202:B3FF:FE1E:8329") + + # Hostnames + assert not helpers.is_ip_address("localhost") + assert not helpers.is_ip_address("www.example.com") + + # Out of range + assert not helpers.is_ip_address("999.999.999.999") + # Contain a port + assert not helpers.is_ip_address("127.0.0.1:80") + assert not helpers.is_ip_address("[2001:db8:0:1]:80") + # Too many "::" + assert not helpers.is_ip_address("1200::AB00:1234::2552:7777:1313") + + class TestCookieJar(unittest.TestCase): def setUp(self): From cdf703f3e92ddea822da6057b6252f6c8ace674b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Fri, 18 Mar 2016 19:13:47 +0100 Subject: [PATCH 19/27] Implemented filtering by path-attribute --- aiohttp/helpers.py | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 0895a42fb5c..d91ac087153 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -521,7 +521,8 @@ def cookies(self): def update_cookies(self, cookies, response_url=None): """Update cookies.""" - hostname = urlsplit(response_url).hostname + url_parsed = urlsplit(response_url or "") + hostname = url_parsed.hostname if is_ip_address(hostname): # Don't accept cookies from IPs @@ -547,6 +548,16 @@ def update_cookies(self, cookies, response_url=None): self._host_only_cookies.add(name) cookie["domain"] = hostname + if not cookie["path"] or not cookie["path"].startswith("/"): + # Set the cookie's path to the response path + path = url_parsed.path + if not path.startswith("/"): + path = "/" + else: + # Cut everything from the last slash to the end + path = "/" + path[1:path.rfind("/")] + cookie["path"] = path + # Remove the host-only flags of nonexistent cookies self._host_only_cookies -= ( self._host_only_cookies.difference(self._cookies.keys())) @@ -597,6 +608,9 @@ def filter_cookies(self, request_url): elif not self._is_domain_match(cookie_domain, hostname): continue + if not self._is_path_match(url_parsed.path, cookie["path"]): + continue + is_secure = url_parsed.scheme in ("https", "wss") if cookie["secure"] and not is_secure: @@ -615,9 +629,25 @@ def _is_domain_match(domain, hostname): if not hostname.endswith(domain): return False - rest = hostname[:-len(domain)] + non_matching = hostname[:-len(domain)] - if rest[-1] != ".": + if not non_matching.endswith("."): return False return not is_ip_address(hostname) + + @staticmethod + def _is_path_match(req_path, cookie_path): + """Implements path matching adhering to RFC 6265.""" + if req_path == cookie_path: + return True + + if not req_path.startswith(cookie_path): + return False + + if cookie_path.endswith("/"): + return True + + non_matching = req_path[len(cookie_path):] + + return non_matching.startswith("/") From 1a409eaa93da11e0b867624dfc3c02ca567728c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Fri, 18 Mar 2016 19:14:33 +0100 Subject: [PATCH 20/27] Added tests for path-attribute --- tests/test_helpers.py | 103 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 91 insertions(+), 12 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 5d280d0320b..b112ebf1b53 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -238,28 +238,33 @@ def setUp(self): "subdomain2-cookie=fourth; Domain=test2.example.com; " "dotted-domain-cookie=fifth; Domain=.example.com; " "different-domain-cookie=sixth; Domain=different.org; " - "secure-cookie=seventh: Domain=secure.com; Secure;" + "secure-cookie=seventh; Domain=secure.com; Secure; " + "no-path-cookie=eighth; Domain=pathtest.com; " + "path1-cookie=nineth; Domain=pathtest.com; Path=/ " + "path2-cookie=tenth; Domain=pathtest.com; Path=/one " + "path3-cookie=eleventh; Domain=pathtest.com; Path=/one/two " + "path4-cookie=twelfth; Domain=pathtest.com; Path=/one/two/" ) # Cookies received from the server as "Set-Cookie" header self.cookies_to_receive = http.cookies.SimpleCookie( - "unconstrained-cookie=first; " - "domain-cookie=second; Domain=example.com; " - "subdomain1-cookie=third; Domain=test1.example.com; " - "subdomain2-cookie=fourth; Domain=test2.example.com; " - "dotted-domain-cookie=fifth; Domain=.example.com; " - "different-domain-cookie=sixth; Domain=different.org; " + "unconstrained-cookie=first; Path=/ " + "domain-cookie=second; Domain=example.com; Path=/ " + "subdomain1-cookie=third; Domain=test1.example.com; Path=/ " + "subdomain2-cookie=fourth; Domain=test2.example.com; Path=/ " + "dotted-domain-cookie=fifth; Domain=.example.com; Path=/ " + "different-domain-cookie=sixth; Domain=different.org; Path=/" ) def request_reply_with_same_url(self, url): - self.jar = helpers.CookieJar(self.cookies_to_send) + jar = helpers.CookieJar(self.cookies_to_send) - cookies_sent = self.jar.filter_cookies(url) + cookies_sent = jar.filter_cookies(url) - self.jar.cookies.clear() + jar.cookies.clear() - self.jar.update_cookies(self.cookies_to_receive, url) - cookies_received = self.jar.cookies.copy() + jar.update_cookies(self.cookies_to_receive, url) + cookies_received = jar.cookies.copy() return cookies_sent, cookies_received @@ -356,3 +361,77 @@ def test_cookie_secure_filter(self): "shared-cookie", "secure-cookie" } + + def test_cookie_path_filter_root(self): + cookies_sent, _ = ( + self.request_reply_with_same_url("http://pathtest.com/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie" + } + + def test_cookie_path_filter_folder(self): + + cookies_sent, _ = ( + self.request_reply_with_same_url("http://pathtest.com/one/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie", + "path2-cookie" + } + + def test_cookie_path_filter_file(self): + + cookies_sent, _ = self.request_reply_with_same_url( + "http://pathtest.com/one/two") + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie", + "path2-cookie", + "path3-cookie" + } + + def test_cookie_path_filter_subfolder(self): + + cookies_sent, _ = self.request_reply_with_same_url( + "http://pathtest.com/one/two/") + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie", + "path2-cookie", + "path3-cookie", + "path4-cookie" + } + + def test_cookie_path_filter_subsubfolder(self): + + cookies_sent, _ = self.request_reply_with_same_url( + "http://pathtest.com/one/two/three/") + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie", + "path2-cookie", + "path3-cookie", + "path4-cookie" + } + + def test_cookie_path_filter_different_folder(self): + + cookies_sent, _ = ( + self.request_reply_with_same_url("http://pathtest.com/hundred/")) + + assert set(cookies_sent.keys()) == { + "shared-cookie", + "no-path-cookie", + "path1-cookie" + } From 3be84c2626445ae8668fb7f79177af780df0505b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Fri, 18 Mar 2016 19:51:48 +0100 Subject: [PATCH 21/27] Added test for path-attributes of received cookies --- tests/test_helpers.py | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index b112ebf1b53..ca5654f9b50 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -240,20 +240,23 @@ def setUp(self): "different-domain-cookie=sixth; Domain=different.org; " "secure-cookie=seventh; Domain=secure.com; Secure; " "no-path-cookie=eighth; Domain=pathtest.com; " - "path1-cookie=nineth; Domain=pathtest.com; Path=/ " - "path2-cookie=tenth; Domain=pathtest.com; Path=/one " - "path3-cookie=eleventh; Domain=pathtest.com; Path=/one/two " - "path4-cookie=twelfth; Domain=pathtest.com; Path=/one/two/" + "path1-cookie=nineth; Domain=pathtest.com; Path=/; " + "path2-cookie=tenth; Domain=pathtest.com; Path=/one; " + "path3-cookie=eleventh; Domain=pathtest.com; Path=/one/two; " + "path4-cookie=twelfth; Domain=pathtest.com; Path=/one/two/;" ) # Cookies received from the server as "Set-Cookie" header self.cookies_to_receive = http.cookies.SimpleCookie( - "unconstrained-cookie=first; Path=/ " - "domain-cookie=second; Domain=example.com; Path=/ " - "subdomain1-cookie=third; Domain=test1.example.com; Path=/ " - "subdomain2-cookie=fourth; Domain=test2.example.com; Path=/ " - "dotted-domain-cookie=fifth; Domain=.example.com; Path=/ " - "different-domain-cookie=sixth; Domain=different.org; Path=/" + "unconstrained-cookie=first; Path=/; " + "domain-cookie=second; Domain=example.com; Path=/; " + "subdomain1-cookie=third; Domain=test1.example.com; Path=/; " + "subdomain2-cookie=fourth; Domain=test2.example.com; Path=/; " + "dotted-domain-cookie=fifth; Domain=.example.com; Path=/; " + "different-domain-cookie=sixth; Domain=different.org; Path=/; " + "no-path-cookie=seventh; Domain=pathtest.com; " + "path-cookie=eighth; Domain=pathtest.com; Path=/somepath;" + "wrong-path-cookie=nineth; Domain=pathtest.com; Path=somepath;" ) def request_reply_with_same_url(self, url): @@ -435,3 +438,18 @@ def test_cookie_path_filter_different_folder(self): "no-path-cookie", "path1-cookie" } + + def test_cookie_path_value(self): + _, cookies_received = ( + self.request_reply_with_same_url("http://pathtest.com/")) + + assert set(cookies_received.keys()) == { + "unconstrained-cookie", + "no-path-cookie", + "path-cookie", + "wrong-path-cookie" + } + + assert cookies_received["no-path-cookie"]["path"] == "/" + assert cookies_received["path-cookie"]["path"] == "/somepath" + assert cookies_received["wrong-path-cookie"]["path"] == "/" From 49592340263ef36031066a1f52c65468372e0ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Fri, 18 Mar 2016 19:55:58 +0100 Subject: [PATCH 22/27] Using TestCase's assertEqual() --- tests/test_helpers.py | 80 +++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index ca5654f9b50..ad6b1015eaa 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -279,177 +279,177 @@ def test_domain_filter_ip(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://1.2.3.4/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie" - } + }) - assert set(cookies_received.keys()) == set() + self.assertEqual(set(cookies_received.keys()), set()) def test_domain_filter_same_host(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://example.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "domain-cookie", "dotted-domain-cookie" - } + }) - assert set(cookies_received.keys()) == { + self.assertEqual(set(cookies_received.keys()), { "unconstrained-cookie", "domain-cookie", "dotted-domain-cookie" - } + }) def test_domain_filter_same_host_and_subdomain(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://test1.example.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "domain-cookie", "subdomain1-cookie", "dotted-domain-cookie" - } + }) - assert set(cookies_received.keys()) == { + self.assertEqual(set(cookies_received.keys()), { "unconstrained-cookie", "domain-cookie", "subdomain1-cookie", "dotted-domain-cookie" - } + }) def test_domain_filter_same_host_diff_subdomain(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://different.example.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "domain-cookie", "dotted-domain-cookie" - } + }) - assert set(cookies_received.keys()) == { + self.assertEqual(set(cookies_received.keys()), { "unconstrained-cookie", "domain-cookie", "dotted-domain-cookie" - } + }) def test_cookie_domain_filter_diff_host(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://different.org/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "different-domain-cookie" - } + }) - assert set(cookies_received.keys()) == { + self.assertEqual(set(cookies_received.keys()), { "unconstrained-cookie", "different-domain-cookie" - } + }) def test_cookie_secure_filter(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://secure.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie" - } + }) cookies_sent, _ = ( self.request_reply_with_same_url("https://secure.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "secure-cookie" - } + }) def test_cookie_path_filter_root(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie" - } + }) def test_cookie_path_filter_folder(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/one/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie", "path2-cookie" - } + }) def test_cookie_path_filter_file(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two") - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie", "path2-cookie", "path3-cookie" - } + }) def test_cookie_path_filter_subfolder(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two/") - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie", "path2-cookie", "path3-cookie", "path4-cookie" - } + }) def test_cookie_path_filter_subsubfolder(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two/three/") - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie", "path2-cookie", "path3-cookie", "path4-cookie" - } + }) def test_cookie_path_filter_different_folder(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/hundred/")) - assert set(cookies_sent.keys()) == { + self.assertEqual(set(cookies_sent.keys()), { "shared-cookie", "no-path-cookie", "path1-cookie" - } + }) def test_cookie_path_value(self): _, cookies_received = ( self.request_reply_with_same_url("http://pathtest.com/")) - assert set(cookies_received.keys()) == { + self.assertEqual(set(cookies_received.keys()), { "unconstrained-cookie", "no-path-cookie", "path-cookie", "wrong-path-cookie" - } + }) - assert cookies_received["no-path-cookie"]["path"] == "/" - assert cookies_received["path-cookie"]["path"] == "/somepath" - assert cookies_received["wrong-path-cookie"]["path"] == "/" + self.assertEqual(cookies_received["no-path-cookie"]["path"], "/") + self.assertEqual(cookies_received["path-cookie"]["path"], "/somepath") + self.assertEqual(cookies_received["wrong-path-cookie"]["path"], "/") From 2d21ff46655f313ad72adfd4fd78e156bd606c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Sun, 20 Mar 2016 00:21:06 +0100 Subject: [PATCH 23/27] Implemented expires- and max-age-attribute --- aiohttp/helpers.py | 113 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 4 deletions(-) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index d91ac087153..a0d2d074846 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -507,8 +507,22 @@ def _cancel_task(self): class CookieJar: """Implements cookie storage adhering to RFC 6265.""" - def __init__(self, cookies=None): + DATE_TOKENS_RE = re.compile( + "[\x09\x20-\x2F\x3B-\x40\x5B-\x60\x7B-\x7E]*" + "(?P[\x00-\x08\x0A-\x1F\d:a-zA-Z\x7F-\xFF]+)") + + DATE_HMS_TIME_RE = re.compile("(\d{1,2}):(\d{1,2}):(\d{1,2})") + + DATE_DAY_OF_MONTH_RE = re.compile("(\d{1,2})") + + DATE_MONTH_RE = re.compile( + "(jan|feb|mar|apr|may|jun|jul|aug|sep|oct|nov|dec)", re.I) + + DATE_YEAR_RE = re.compile("(\d{2,4})") + + def __init__(self, cookies=None, loop=None): self._cookies = SimpleCookie() + self._loop = loop or asyncio.get_event_loop() self._host_only_cookies = set() if cookies is not None: @@ -519,6 +533,10 @@ def cookies(self): """The session cookies.""" return self._cookies + def _expire_cookie(self, name): + if name in self._cookies: + del self._cookies[name] + def update_cookies(self, cookies, response_url=None): """Update cookies.""" url_parsed = urlsplit(response_url or "") @@ -558,12 +576,31 @@ def update_cookies(self, cookies, response_url=None): path = "/" + path[1:path.rfind("/")] cookie["path"] = path + max_age = cookie["max-age"] + if max_age: + try: + delta_seconds = int(max_age) + self._loop.call_later( + delta_seconds, self._expire_cookie, name) + except ValueError: + cookie["max-age"] = "" + + expires = cookie["expires"] + if not cookie["max-age"] and expires: + expire_time = self._parse_date(expires) + if expire_time: + self._loop.call_at( + expire_time.timestamp(), + self._expire_cookie, name) + else: + cookie["expires"] = "" + # Remove the host-only flags of nonexistent cookies self._host_only_cookies -= ( self._host_only_cookies.difference(self._cookies.keys())) def _add_morsel(self, name, value, hostname): - """Add a Morsel to the cookie store.""" + """Add a Morsel to the cookie jar.""" cookie_domain = value["domain"] if cookie_domain.startswith("."): # Remove leading dot @@ -584,8 +621,7 @@ def _add_morsel(self, name, value, hostname): return True def filter_cookies(self, request_url): - """Returns this store's cookies filtered by their attributes.""" - # TODO: filter by 'expires', 'path', ... + """Returns this jar's cookies filtered by their attributes.""" url_parsed = urlsplit(request_url) filtered = SimpleCookie() @@ -651,3 +687,72 @@ def _is_path_match(req_path, cookie_path): non_matching = req_path[len(cookie_path):] return non_matching.startswith("/") + + @classmethod + def _parse_date(cls, date_str): + """Implements date string parsing adhering to RFC 6265.""" + if not date_str: + return + + found_time = False + found_day_of_month = False + found_month = False + found_year = False + + hour = minute = second = 0 + day_of_month = 0 + month = "" + year = 0 + + for token_match in cls.DATE_TOKENS_RE.finditer(date_str): + + token = token_match.group("token") + + if not found_time: + time_match = cls.DATE_HMS_TIME_RE.match(token) + if time_match: + found_time = True + hour, minute, second = [ + int(s) for s in time_match.groups()] + continue + + if not found_day_of_month: + day_of_month_match = cls.DATE_DAY_OF_MONTH_RE.match(token) + if day_of_month_match: + found_day_of_month = True + day_of_month = int(day_of_month_match.group()) + continue + + if not found_month: + month_match = cls.DATE_MONTH_RE.match(token) + if month_match: + found_month = True + month = month_match.group() + continue + + if not found_year: + year_match = cls.DATE_YEAR_RE.match(token) + if year_match: + found_year = True + year = int(year_match.group()) + + if 70 <= year <= 99: + year += 1900 + elif 0 <= year <= 69: + year += 2000 + + if False in (found_day_of_month, found_month, found_year, found_time): + return + + if not 1 <= day_of_month <= 31: + return + + if year < 1601 or hour > 23 or minute > 59 or second > 59: + return + + dt = datetime.datetime.strptime( + "%s %d %d:%d:%d %d" % ( + month, day_of_month, hour, minute, second, year + ), "%b %d %H:%M:%S %Y") + + return dt.replace(tzinfo=datetime.timezone.utc) From 2832ea48a3f1c68b3e313bff1388413ea786404d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Sun, 20 Mar 2016 00:21:59 +0100 Subject: [PATCH 24/27] Added tests for expires- and max-age-attribute --- tests/test_helpers.py | 87 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 9 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index ad6b1015eaa..a1de2d002b5 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,9 +1,10 @@ +import asyncio import pytest import unittest import datetime import http.cookies from unittest import mock -from aiohttp import helpers +from aiohttp import helpers, test_utils def test_parse_mimetype_1(): @@ -243,7 +244,11 @@ def setUp(self): "path1-cookie=nineth; Domain=pathtest.com; Path=/; " "path2-cookie=tenth; Domain=pathtest.com; Path=/one; " "path3-cookie=eleventh; Domain=pathtest.com; Path=/one/two; " - "path4-cookie=twelfth; Domain=pathtest.com; Path=/one/two/;" + "path4-cookie=twelfth; Domain=pathtest.com; Path=/one/two/; " + "expires-cookie=thirteenth; Domain=expirestest.com; Path=/;" + " Expires=Tue, 1 Jan 1980 12:00:00 GMT; " + "max-age-cookie=fourteenth; Domain=maxagetest.com; Path=/;" + " Max-Age=60;" ) # Cookies received from the server as "Set-Cookie" header @@ -255,25 +260,51 @@ def setUp(self): "dotted-domain-cookie=fifth; Domain=.example.com; Path=/; " "different-domain-cookie=sixth; Domain=different.org; Path=/; " "no-path-cookie=seventh; Domain=pathtest.com; " - "path-cookie=eighth; Domain=pathtest.com; Path=/somepath;" + "path-cookie=eighth; Domain=pathtest.com; Path=/somepath; " "wrong-path-cookie=nineth; Domain=pathtest.com; Path=somepath;" ) + self.loop = asyncio.new_event_loop() + asyncio.set_event_loop(None) + + self.jar = helpers.CookieJar(loop=self.loop) + + def tearDown(self): + self.loop.close() + def request_reply_with_same_url(self, url): - jar = helpers.CookieJar(self.cookies_to_send) + self.jar.update_cookies(self.cookies_to_send) + cookies_sent = self.jar.filter_cookies(url) - cookies_sent = jar.filter_cookies(url) + self.jar.cookies.clear() - jar.cookies.clear() + self.jar.update_cookies(self.cookies_to_receive, url) + cookies_received = self.jar.cookies.copy() - jar.update_cookies(self.cookies_to_receive, url) - cookies_received = jar.cookies.copy() + self.jar.cookies.clear() return cookies_sent, cookies_received + def timed_request( + self, url, update_time, send_time): + time_func = "time.monotonic" + + with mock.patch(time_func, return_value=update_time): + self.jar.update_cookies(self.cookies_to_send) + + with mock.patch(time_func, return_value=send_time): + test_utils.run_briefly(self.loop) + + cookies_sent = self.jar.filter_cookies(url) + + self.jar.cookies.clear() + + return cookies_sent + def test_constructor(self): - jar = helpers.CookieJar(self.cookies_to_send) + jar = helpers.CookieJar(self.cookies_to_send, self.loop) self.assertEqual(jar.cookies, self.cookies_to_send) + self.assertEqual(jar._loop, self.loop) def test_domain_filter_ip(self): cookies_sent, cookies_received = ( @@ -453,3 +484,41 @@ def test_cookie_path_value(self): self.assertEqual(cookies_received["no-path-cookie"]["path"], "/") self.assertEqual(cookies_received["path-cookie"]["path"], "/somepath") self.assertEqual(cookies_received["wrong-path-cookie"]["path"], "/") + + def test_cookie_expires(self): + ts_before = datetime.datetime( + 1975, 1, 1, tzinfo=datetime.timezone.utc).timestamp() + + ts_after = datetime.datetime( + 1985, 1, 1, tzinfo=datetime.timezone.utc).timestamp() + + cookies_sent = self.timed_request( + "http://expirestest.com/", ts_before, ts_before) + + self.assertEqual(set(cookies_sent.keys()), { + "shared-cookie", + "expires-cookie" + }) + + cookies_sent = self.timed_request( + "http://expirestest.com/", ts_before, ts_after) + + self.assertEqual(set(cookies_sent.keys()), { + "shared-cookie" + }) + + def test_cookie_max_age(self): + cookies_sent = self.timed_request( + "http://maxagetest.com/", 1000, 1000) + + self.assertEqual(set(cookies_sent.keys()), { + "shared-cookie", + "max-age-cookie" + }) + + cookies_sent = self.timed_request( + "http://maxagetest.com/", 1000, 2000) + + self.assertEqual(set(cookies_sent.keys()), { + "shared-cookie" + }) From 1cfb2c67efd2852b10106a86112ac8e7675ca6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Sun, 20 Mar 2016 00:35:31 +0100 Subject: [PATCH 25/27] Passing the ClientSession's loop through to its CookieJar --- aiohttp/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 6320be84a93..a4a4a54ca30 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -51,7 +51,7 @@ def __init__(self, *, connector=None, loop=None, cookies=None, if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) - self._cookie_jar = CookieJar() + self._cookie_jar = CookieJar(loop=loop) # For Backward compatability with `share_cookies` connectors if connector._share_cookies: From 791713202ebe17f095bb7d81893d187af5bf5bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Wed, 13 Apr 2016 21:52:35 +0200 Subject: [PATCH 26/27] Added more tests (invalid values, host only cookie, domain matching, path matching, date parsing) --- tests/test_helpers.py | 132 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 12 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index a1de2d002b5..c8d860ceeed 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -248,7 +248,11 @@ def setUp(self): "expires-cookie=thirteenth; Domain=expirestest.com; Path=/;" " Expires=Tue, 1 Jan 1980 12:00:00 GMT; " "max-age-cookie=fourteenth; Domain=maxagetest.com; Path=/;" - " Max-Age=60;" + " Max-Age=60; " + "invalid-max-age-cookie=fifteenth; Domain=invalid-values.com; " + " Max-Age=string; " + "invalid-expires-cookie=sixteenth; Domain=invalid-values.com; " + " Expires=string;" ) # Cookies received from the server as "Set-Cookie" header @@ -366,7 +370,7 @@ def test_domain_filter_same_host_diff_subdomain(self): "dotted-domain-cookie" }) - def test_cookie_domain_filter_diff_host(self): + def test_domain_filter_diff_host(self): cookies_sent, cookies_received = ( self.request_reply_with_same_url("http://different.org/")) @@ -380,7 +384,16 @@ def test_cookie_domain_filter_diff_host(self): "different-domain-cookie" }) - def test_cookie_secure_filter(self): + def test_domain_filter_host_only(self): + self.jar.update_cookies(self.cookies_to_receive, "http://example.com/") + + cookies_sent = self.jar.filter_cookies("http://example.com/") + self.assertIn("unconstrained-cookie", set(cookies_sent.keys())) + + cookies_sent = self.jar.filter_cookies("http://different.org/") + self.assertNotIn("unconstrained-cookie", set(cookies_sent.keys())) + + def test_secure_filter(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://secure.com/")) @@ -396,7 +409,7 @@ def test_cookie_secure_filter(self): "secure-cookie" }) - def test_cookie_path_filter_root(self): + def test_path_filter_root(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/")) @@ -406,7 +419,7 @@ def test_cookie_path_filter_root(self): "path1-cookie" }) - def test_cookie_path_filter_folder(self): + def test_path_filter_folder(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/one/")) @@ -418,7 +431,7 @@ def test_cookie_path_filter_folder(self): "path2-cookie" }) - def test_cookie_path_filter_file(self): + def test_path_filter_file(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two") @@ -431,7 +444,7 @@ def test_cookie_path_filter_file(self): "path3-cookie" }) - def test_cookie_path_filter_subfolder(self): + def test_path_filter_subfolder(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two/") @@ -445,7 +458,7 @@ def test_cookie_path_filter_subfolder(self): "path4-cookie" }) - def test_cookie_path_filter_subsubfolder(self): + def test_path_filter_subsubfolder(self): cookies_sent, _ = self.request_reply_with_same_url( "http://pathtest.com/one/two/three/") @@ -459,7 +472,7 @@ def test_cookie_path_filter_subsubfolder(self): "path4-cookie" }) - def test_cookie_path_filter_different_folder(self): + def test_path_filter_different_folder(self): cookies_sent, _ = ( self.request_reply_with_same_url("http://pathtest.com/hundred/")) @@ -470,7 +483,7 @@ def test_cookie_path_filter_different_folder(self): "path1-cookie" }) - def test_cookie_path_value(self): + def test_path_value(self): _, cookies_received = ( self.request_reply_with_same_url("http://pathtest.com/")) @@ -485,7 +498,7 @@ def test_cookie_path_value(self): self.assertEqual(cookies_received["path-cookie"]["path"], "/somepath") self.assertEqual(cookies_received["wrong-path-cookie"]["path"], "/") - def test_cookie_expires(self): + def test_expires(self): ts_before = datetime.datetime( 1975, 1, 1, tzinfo=datetime.timezone.utc).timestamp() @@ -507,7 +520,7 @@ def test_cookie_expires(self): "shared-cookie" }) - def test_cookie_max_age(self): + def test_max_age(self): cookies_sent = self.timed_request( "http://maxagetest.com/", 1000, 1000) @@ -522,3 +535,98 @@ def test_cookie_max_age(self): self.assertEqual(set(cookies_sent.keys()), { "shared-cookie" }) + + def test_invalid_values(self): + cookies_sent, cookies_received = ( + self.request_reply_with_same_url("http://invalid-values.com/")) + + self.assertEqual(set(cookies_sent.keys()), { + "shared-cookie", + "invalid-max-age-cookie", + "invalid-expires-cookie" + }) + + cookie = cookies_sent["invalid-max-age-cookie"] + self.assertEqual(cookie["max-age"], "") + + cookie = cookies_sent["invalid-expires-cookie"] + self.assertEqual(cookie["expires"], "") + + def test_domain_matching(self): + test_func = helpers.CookieJar._is_domain_match + + self.assertTrue(test_func("test.com", "test.com")) + self.assertTrue(test_func("test.com", "sub.test.com")) + + self.assertFalse(test_func("test.com", "")) + self.assertFalse(test_func("test.com", "test.org")) + self.assertFalse(test_func("diff-test.com", "test.com")) + self.assertFalse(test_func("test.com", "diff-test.com")) + self.assertFalse(test_func("test.com", "127.0.0.1")) + + def test_path_matching(self): + test_func = helpers.CookieJar._is_path_match + + self.assertTrue(test_func("/", "")) + self.assertTrue(test_func("/file", "")) + self.assertTrue(test_func("/folder/file", "")) + self.assertTrue(test_func("/", "/")) + self.assertTrue(test_func("/file", "/")) + self.assertTrue(test_func("/file", "/file")) + self.assertTrue(test_func("/folder/", "/folder/")) + self.assertTrue(test_func("/folder/", "/")) + self.assertTrue(test_func("/folder/file", "/")) + + self.assertFalse(test_func("/", "/file")) + self.assertFalse(test_func("/", "/folder/")) + self.assertFalse(test_func("/file", "/folder/file")) + self.assertFalse(test_func("/folder/", "/folder/file")) + self.assertFalse(test_func("/different-file", "/file")) + self.assertFalse(test_func("/different-folder/", "/folder/")) + + def test_date_parsing(self): + parse_func = helpers.CookieJar._parse_date + utc=datetime.timezone.utc + + self.assertEqual(parse_func(""), None) + + # 70 -> 1970 + self.assertEqual( + parse_func("Tue, 1 Jan 70 00:00:00 GMT"), + datetime.datetime(1970, 1, 1, tzinfo=utc)) + + # 10 -> 2010 + self.assertEqual( + parse_func("Tue, 1 Jan 10 00:00:00 GMT"), + datetime.datetime(2010, 1, 1, tzinfo=utc)) + + # No day of week string + self.assertEqual( + parse_func("1 Jan 1970 00:00:00 GMT"), + datetime.datetime(1970, 1, 1, tzinfo=utc)) + + # No timezone string + self.assertEqual( + parse_func("Tue, 1 Jan 1970 00:00:00"), + datetime.datetime(1970, 1, 1, tzinfo=utc)) + + # No year + self.assertEqual(parse_func("Tue, 1 Jan 00:00:00 GMT"), None) + + # No month + self.assertEqual(parse_func("Tue, 1 1970 00:00:00 GMT"), None) + + # No day of month + self.assertEqual(parse_func("Tue, Jan 1970 00:00:00 GMT"), None) + + # No time + self.assertEqual(parse_func("Tue, 1 Jan 1970 GMT"), None) + + # Invalid day of month + self.assertEqual(parse_func("Tue, 0 Jan 1970 00:00:00 GMT"), None) + + # Invalid year + self.assertEqual(parse_func("Tue, 1 Jan 1500 00:00:00 GMT"), None) + + # Invalid time + self.assertEqual(parse_func("Tue, 1 Jan 1970 77:88:99 GMT"), None) From f6a5da49d996946fd4ba66bc0a076f81190c750e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20H=C3=BCther?= Date: Sun, 24 Apr 2016 14:49:25 +0200 Subject: [PATCH 27/27] Fixed flake8 "Missing whitespace around operator" --- tests/test_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c8d860ceeed..c7712ec7784 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -586,7 +586,7 @@ def test_path_matching(self): def test_date_parsing(self): parse_func = helpers.CookieJar._parse_date - utc=datetime.timezone.utc + utc = datetime.timezone.utc self.assertEqual(parse_func(""), None)