Refactor cookie jar

CHANGES.rst

@@ -133,9 +133,12 @@ CHANGES
 
 - Conform to RFC3986 - do not include url fragments in client requests #1174
 
--
+- Drop `ClientSession.cookies` (BACKWARD INCOMPATIBLE CHANGE) #1173
 
--
+- Refactor `AbstractCookieJar` public API (BACKWARD INCOMPATIBLE) #1173
+
+- Fix clashing cookies with have the same name but belong to different
+  domains #1125
 
 -
 

aiohttp/abc.py

@@ -1,7 +1,7 @@
 import asyncio
 import sys
 from abc import ABC, abstractmethod
-from http.cookies import SimpleCookie
+from collections.abc import Iterable, Sized
 
 PY_35 = sys.version_info >= (3, 5)
 
@@ -70,21 +70,19 @@ def close(self):
         """Release resolver"""
 
 
-class AbstractCookieJar(ABC):
+class AbstractCookieJar(Sized, Iterable):
 
     def __init__(self, *, loop=None):
-        self._cookies = SimpleCookie()
         self._loop = loop or asyncio.get_event_loop()
 
-    @property
-    def cookies(self):
-        """The session cookies."""
-        return self._cookies
+    @abstractmethod
+    def clear(self):
+        """Clear all cookies."""
 
     @abstractmethod
     def update_cookies(self, cookies, response_url=None):
         """Update cookies."""
 
     @abstractmethod
     def filter_cookies(self, request_url):
-        """Returns this jar's cookies filtered by their attributes."""
+        """Return the jar's cookies filtered by their attributes."""

aiohttp/client.py

@@ -469,9 +469,9 @@ def connector(self):
         return self._connector
 
     @property
-    def cookies(self):
+    def cookie_jar(self):
         """The session cookies."""
-        return self._cookie_jar.cookies
+        return self._cookie_jar
 
     @property
     def version(self):

aiohttp/cookiejar.py

@@ -1,6 +1,8 @@
 import datetime
 import re
 import time
+from collections import defaultdict
+from collections.abc import Mapping
 from http.cookies import Morsel, SimpleCookie
 from math import ceil
 from urllib.parse import urlsplit
@@ -29,15 +31,25 @@ class CookieJar(AbstractCookieJar):
 
     def __init__(self, *, unsafe=False, loop=None):
         super().__init__(loop=loop)
+        self._cookies = defaultdict(SimpleCookie)
         self._host_only_cookies = set()
         self._unsafe = unsafe
         self._next_expiration = ceil(loop.time())
         self._expirations = {}
 
-    @property
-    def cookies(self):
+    def clear(self):
+        self._cookies.clear()
+        self._host_only_cookies.clear()
+        self._next_expiration = ceil(self._loop.time())
+        self._expirations.clear()
+
+    def __iter__(self):
         self._do_expiration()
-        return super().cookies
+        for val in self._cookies.values():
+            yield from val.values()
+
+    def __len__(self):
+        return sum(1 for i in self)
 
     def _do_expiration(self):
         now = self._loop.time()
@@ -49,51 +61,64 @@ def _do_expiration(self):
         to_del = []
         cookies = self._cookies
         expirations = self._expirations
-        for name, when in expirations.items():
+        for (domain, name), when in expirations.items():
             if when < now:
-                cookies.pop(name, None)
-                to_del.append(name)
+                cookies[domain].pop(name, None)
+                to_del.append((domain, name))
+                self._host_only_cookies.discard((domain, name))
             else:
                 next_expiration = min(next_expiration, when)
-        for name in to_del:
-            del expirations[name]
+        for key in to_del:
+            del expirations[key]
+
         self._next_expiration = ceil(next_expiration)
 
-    def _expire_cookie(self, when, name):
+    def _expire_cookie(self, when, domain, name):
         self._next_expiration = min(self._next_expiration, when)
-        self._expirations[name] = when
+        self._expirations[(domain, name)] = when
 
     def update_cookies(self, cookies, response_url=None):
         """Update cookies."""
-        self._do_expiration()
         url_parsed = urlsplit(response_url or "")
         hostname = url_parsed.hostname
 
         if not self._unsafe and is_ip_address(hostname):
             # Don't accept cookies from IPs
             return
 
-        if isinstance(cookies, dict):
+        if isinstance(cookies, Mapping):
             cookies = cookies.items()
 
-        for name, value in cookies:
-            if isinstance(value, Morsel):
+        for name, cookie in cookies:
+            if not isinstance(cookie, Morsel):
+                tmp = SimpleCookie()
+                tmp[name] = cookie
+                cookie = tmp[name]
 
-                if not self._add_morsel(name, value, hostname):
-                    continue
+            domain = cookie["domain"]
 
-            else:
-                self._cookies[name] = value
-
-            cookie = self._cookies[name]
+            # ignore domains with trailing dots
+            if domain.endswith('.'):
+                domain = ""
+                del cookie["domain"]
 
-            if not cookie["domain"] and hostname is not None:
+            if not 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
+                self._host_only_cookies.add((hostname, name))
+                domain = cookie["domain"] = hostname
+
+            if domain.startswith("."):
+                # Remove leading dot
+                domain = domain[1:]
+                cookie["domain"] = domain
+
+            if hostname and not self._is_domain_match(domain, hostname):
+                # Setting cookies for different domains is not allowed
+                continue
 
-            if not cookie["path"] or not cookie["path"].startswith("/"):
+            path = cookie["path"]
+            if not path or not path.startswith("/"):
                 # Set the cookie's path to the response path
                 path = url_parsed.path
                 if not path.startswith("/"):
@@ -108,45 +133,25 @@ def update_cookies(self, cookies, response_url=None):
                 try:
                     delta_seconds = int(max_age)
                     self._expire_cookie(self._loop.time() + delta_seconds,
-                                        name)
+                                        domain, 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._expire_cookie(expire_time.timestamp(),
-                                        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 jar."""
-        cookie_domain = value["domain"]
-        if cookie_domain.startswith("."):
-            # Remove leading dot
-            cookie_domain = cookie_domain[1:]
-            value["domain"] = cookie_domain
+            else:
+                expires = cookie["expires"]
+                if expires:
+                    expire_time = self._parse_date(expires)
+                    if expire_time:
+                        self._expire_cookie(expire_time.timestamp(),
+                                            domain, name)
+                    else:
+                        cookie["expires"] = ""
 
-        if not cookie_domain or not hostname:
             # use dict method because SimpleCookie class modifies value
             # before Python 3.4.3
-            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
+            dict.__setitem__(self._cookies[domain], name, cookie)
 
-        # use dict method because SimpleCookie class modifies value
-        # before Python 3.4.3
-        dict.__setitem__(self._cookies, name, value)
-        return True
+        self._do_expiration()
 
     def filter_cookies(self, request_url):
         """Returns this jar's cookies filtered by their attributes."""
@@ -156,21 +161,22 @@ def filter_cookies(self, request_url):
         hostname = url_parsed.hostname or ""
         is_not_secure = url_parsed.scheme not in ("https", "wss")
 
-        for name, cookie in self._cookies.items():
-            cookie_domain = cookie["domain"]
+        for cookie in self:
+            name = cookie.key
+            domain = cookie["domain"]
 
             # Send shared cookies
-            if not cookie_domain:
-                dict.__setitem__(filtered, name, cookie)
+            if not domain:
+                filtered[name] = cookie.value
                 continue
 
             if not self._unsafe and is_ip_address(hostname):
                 continue
 
-            if name in self._host_only_cookies:
-                if cookie_domain != hostname:
+            if (domain, name) in self._host_only_cookies:
+                if domain != hostname:
                     continue
-            elif not self._is_domain_match(cookie_domain, hostname):
+            elif not self._is_domain_match(domain, hostname):
                 continue
 
             if not self._is_path_match(url_parsed.path, cookie["path"]):
@@ -179,7 +185,7 @@ def filter_cookies(self, request_url):
             if is_not_secure and cookie["secure"]:
                 continue
 
-            dict.__setitem__(filtered, name, cookie)
+            filtered[name] = cookie.value
 
         return filtered
 

docs/abc.rst

@@ -3,9 +3,9 @@
 Abstract Base Classes
 =====================
 
-.. module:: aiohttp.abc
+.. module:: aiohttp
 
-.. currentmodule:: aiohttp.abc
+.. currentmodule:: aiohttp
 
 Abstract routing
 ----------------
@@ -102,25 +102,47 @@ attribute.
 Abstract Cookie Jar
 -------------------
 
-.. class:: AbstractCookieJar(*, loop=None)
+.. class:: AbstractCookieJar
 
-   An abstract class for cookie storage.
+   The cookie jar instance is available as :attr:`ClientSession.cookie_jar`.
 
-   :param loop: an :ref:`event loop<asyncio-event-loop>` instance.
+   The jar contains :class:`~http.cookies.Morsel` items for storing
+   internal cookie data.
 
-                If param is ``None`` :func:`asyncio.get_event_loop`
-                used for getting default event loop, but we strongly
-                recommend to use explicit loops everywhere.
+   API provides a count of saved cookies::
 
+       len(session.cookie_jar)
 
-   .. attribute:: cookies
+   These cookies may be iterated over::
 
-      :class:`http.cookies.SimpleCookie` instance for storing cookies info.
+       for cookie in session.cookie_jar:
+           print(cookie.key)
+           print(cookie["domain"])
+
+   An abstract class for cookie storage. Implements
+   :class:`collections.abc.Iterable` and
+   :class:`collections.abc.Sized`.
 
    .. method:: update_cookies(cookies, response_url=None)
 
-      Update cookies.
+      Update cookies returned by server in ``Set-Cookie`` header.
+
+      :param cookies: a :class:`collections.abc.Mapping`
+         (e.g. :class:`dict`, :class:`~http.cookies.SimpleCookie`) or
+         *iterable* of *pairs* with cookies returned by server's
+         response.
+
+      :param str response_url: URL of response, ``None`` for *shared
+         cookies*.  Regular cookies are coupled with server's URL and
+         are sent only to this server, shared ones are sent in every
+         client request.
 
    .. method:: filter_cookies(request_url)
 
-      Returns this jar's cookies filtered by their attributes.
+      Return jar's cookies acceptable for URL and available in
+      ``Cookie`` header for sending client requests for given URL.
+
+      :param str response_url: request's URL for which cookies are asked.
+
+      :return: :class:`http.cookies.SimpleCookie` with filtered
+         cookies for given URL.

docs/client.rst

@@ -220,7 +220,7 @@ parameter of :class:`ClientSession` constructor::
 .. note::
    ``httpbin.org/cookies`` endpoint returns request cookies
    in JSON-encoded body.
-   To access session cookies see :attr:`ClientSession.cookies`.
+   To access session cookies see :attr:`ClientSession.cookie_jar`.
 
 
 More complicated POST requests
@@ -378,7 +378,8 @@ between multiple requests::
     async with aiohttp.ClientSession() as session:
         await session.get(
             'http://httpbin.org/cookies/set?my_cookie=my_value')
-        assert session.cookies['my_cookie'].value == 'my_value'
+        filtered = session.cookies_jar.filter_cookies('http://httpbin.org')
+        assert filtered['my_cookie'].value == 'my_value'
         async with session.get('http://httpbin.org/cookies') as r:
             json_body = await r.json()
             assert json_body['cookies']['my_cookie'] == 'my_value'
@@ -613,7 +614,7 @@ If a response contains some Cookies, you can quickly access them::
 
    Response cookies contain only values, that were in ``Set-Cookie`` headers
    of the **last** request in redirection chain. To gather cookies between all
-   redirection requests you can use :ref:`aiohttp.ClientSession
+   redirection requests please use :ref:`aiohttp.ClientSession
    <aiohttp-client-session>` object.