Bug 1634183 [wpt PR 23327] - [wptserve] Make cookies always binary strings, a=testonly
authorRobert Ma <robertma@chromium.org>
Wed, 13 May 2020 09:41:43 +0000
changeset 531081 226966b93b0795d750065f6a6fedf78f166c86f9
parent 531080 e8667097102c2f7f282425dd436c61ec841d87e6
child 531082 b0e0161b9de7300553a8ff1e8a33f715f3d0a12a
push id37435
push userapavel@mozilla.com
push dateWed, 20 May 2020 15:28:23 +0000
treeherdermozilla-central@5415da14ec9a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1634183, 23327
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1634183 [wpt PR 23327] - [wptserve] Make cookies always binary strings, a=testonly Automatic update from web-platform-tests [wptserve] Make cookies always binary strings (#23327) Change cookies on both Request and Response to always accept and return binary strings. Affected APIs: * Request.cookies * Response.{set,unset,delete}_cookie (Note that only name and value are converted at the moment.) Fix unit tests and infrastructure/ tests in Python 3. -- wpt-commits: e030609933ab7f98b1f757e41e80e96f5457e404 wpt-pr: 23327
testing/web-platform/tests/tools/wptserve/tests/functional/test_cookies.py
testing/web-platform/tests/tools/wptserve/wptserve/request.py
testing/web-platform/tests/tools/wptserve/wptserve/response.py
--- a/testing/web-platform/tests/tools/wptserve/tests/functional/test_cookies.py
+++ b/testing/web-platform/tests/tools/wptserve/tests/functional/test_cookies.py
@@ -5,60 +5,62 @@ import pytest
 wptserve = pytest.importorskip("wptserve")
 from .base import TestUsingServer
 
 
 class TestResponseSetCookie(TestUsingServer):
     def test_name_value(self):
         @wptserve.handlers.handler
         def handler(request, response):
-            response.set_cookie("name", "value")
+            response.set_cookie(b"name", b"value")
             return "Test"
 
         route = ("GET", "/test/name_value", handler)
         self.server.router.register(*route)
         resp = self.request(route[1])
 
         self.assertEqual(resp.info()["Set-Cookie"], "name=value; Path=/")
 
     def test_unset(self):
         @wptserve.handlers.handler
         def handler(request, response):
-            response.set_cookie("name", "value")
-            response.unset_cookie("name")
+            response.set_cookie(b"name", b"value")
+            response.unset_cookie(b"name")
             return "Test"
 
         route = ("GET", "/test/unset", handler)
         self.server.router.register(*route)
         resp = self.request(route[1])
 
         self.assertTrue("Set-Cookie" not in resp.info())
 
     def test_delete(self):
         @wptserve.handlers.handler
         def handler(request, response):
-            response.delete_cookie("name")
+            response.delete_cookie(b"name")
             return "Test"
 
         route = ("GET", "/test/delete", handler)
         self.server.router.register(*route)
         resp = self.request(route[1])
 
         parts = dict(item.split("=") for
                      item in resp.info()["Set-Cookie"].split("; ") if item)
 
         self.assertEqual(parts["name"], "")
         self.assertEqual(parts["Path"], "/")
-        #Should also check that expires is in the past
+        # TODO: Should also check that expires is in the past
+
 
 class TestRequestCookies(TestUsingServer):
     def test_set_cookie(self):
         @wptserve.handlers.handler
         def handler(request, response):
-            return request.cookies["name"].value
+            return request.cookies[b"name"].value
 
         route = ("GET", "/test/set_cookie", handler)
         self.server.router.register(*route)
         resp = self.request(route[1], headers={"Cookie": "name=value"})
         self.assertEqual(resp.read(), b"value")
 
+
 if __name__ == '__main__':
     unittest.main()
--- a/testing/web-platform/tests/tools/wptserve/wptserve/request.py
+++ b/testing/web-platform/tests/tools/wptserve/wptserve/request.py
@@ -2,17 +2,17 @@ import base64
 import cgi
 import tempfile
 
 from six import BytesIO, binary_type, iteritems, PY3
 from six.moves.http_cookies import BaseCookie
 from six.moves.urllib.parse import parse_qsl, urlsplit
 
 from . import stash
-from .utils import HTTPException, isomorphic_encode
+from .utils import HTTPException, isomorphic_encode, isomorphic_decode
 
 missing = object()
 
 
 class Server(object):
     """Data about the server environment
 
     .. attribute:: config
@@ -224,23 +224,23 @@ class Request(object):
     .. attribute:: POST
 
     MultiDict representing the request body parameters. Most parameters
     are present as string values, but file uploads have file-like
     values. All string values (including keys) have binary type.
 
     .. attribute:: cookies
 
-    Cookies object representing cookies sent with the request with a
+    A Cookies object representing cookies sent with the request with a
     dictionary-like interface.
 
     .. attribute:: auth
 
-    Object with username and password properties representing any
-    credentials supplied using HTTP authentication.
+    An instance of Authentication with username and password properties
+    representing any credentials supplied using HTTP authentication.
 
     .. attribute:: server
 
     Server object containing information about the server environment.
     """
 
     def __init__(self, request_handler):
         self.doc_root = request_handler.server.router.doc_root
@@ -317,24 +317,22 @@ class Request(object):
             fs = cgi.FieldStorage(**kwargs)
             self._POST = MultiDict.from_field_storage(fs)
             self.raw_input.seek(pos)
         return self._POST
 
     @property
     def cookies(self):
         if self._cookies is None:
-            parser = BaseCookie()
+            parser = BinaryCookieParser()
             cookie_headers = self.headers.get("cookie", b"")
-            if PY3:
-                cookie_headers = cookie_headers.decode("iso-8859-1")
             parser.load(cookie_headers)
             cookies = Cookies()
             for key, value in iteritems(parser):
-                cookies[key] = CookieValue(value)
+                cookies[isomorphic_encode(key)] = CookieValue(value)
             self._cookies = cookies
         return self._cookies
 
     @property
     def headers(self):
         if self._headers is None:
             self._headers = RequestHeaders(self.raw_headers)
         return self._headers
@@ -585,18 +583,49 @@ class MultiDict(dict):
                 if not value.filename:
                     value = isomorphic_encode(value.value)
                 else:
                     assert isinstance(value, cgi.FieldStorage)
                 self.add(isomorphic_encode(key), value)
         return self
 
 
+class BinaryCookieParser(BaseCookie):
+    """A subclass of BaseCookie that returns values in binary strings
+
+    This is not intended to store the cookies; use Cookies instead.
+    """
+    def value_decode(self, val):
+        """Decode value from network to (real_value, coded_value).
+
+        Override BaseCookie.value_decode.
+        """
+        return isomorphic_encode(val), val
+
+    def value_encode(self, val):
+        raise NotImplementedError('BinaryCookieParser is not for setting cookies')
+
+    def load(self, rawdata):
+        """Load cookies from a binary string.
+
+        This overrides and calls BaseCookie.load. Unlike BaseCookie.load, it
+        does not accept dictionaries.
+        """
+        assert isinstance(rawdata, binary_type)
+        if PY3:
+            # BaseCookie.load expects a native string, which in Python 3 is text.
+            rawdata = isomorphic_decode(rawdata)
+        super(BinaryCookieParser, self).load(rawdata)
+
+
 class Cookies(MultiDict):
-    """MultiDict specialised for Cookie values"""
+    """MultiDict specialised for Cookie values
+
+    Keys and values are binary strings.
+    """
     def __init__(self):
         pass
 
     def __getitem__(self, key):
         return self.last(key)
 
 
 class Authentication(object):
--- a/testing/web-platform/tests/tools/wptserve/wptserve/response.py
+++ b/testing/web-platform/tests/tools/wptserve/wptserve/response.py
@@ -96,38 +96,44 @@ class Response(object):
         else:
             self._status = (int(value), None)
 
     def set_cookie(self, name, value, path="/", domain=None, max_age=None,
                    expires=None, secure=False, httponly=False, comment=None):
         """Set a cookie to be sent with a Set-Cookie header in the
         response
 
-        :param name: String name of the cookie
-        :param value: String value of the cookie
+        :param name: name of the cookie (a binary string)
+        :param value: value of the cookie (a binary string, or None)
         :param max_age: datetime.timedelta int representing the time (in seconds)
                         until the cookie expires
         :param path: String path to which the cookie applies
         :param domain: String domain to which the cookie applies
         :param secure: Boolean indicating whether the cookie is marked as secure
         :param httponly: Boolean indicating whether the cookie is marked as
                          HTTP Only
         :param comment: String comment
         :param expires: datetime.datetime or datetime.timedelta indicating a
                         time or interval from now when the cookie expires
 
         """
+        # TODO(Python 3): Convert other parameters (e.g. path) to bytes, too.
+        if value is None:
+            value = b''
+            max_age = 0
+            expires = timedelta(days=-1)
+
+        if PY3:
+            name = isomorphic_decode(name)
+            value = isomorphic_decode(value)
+
         days = {i+1: name for i, name in enumerate(["jan", "feb", "mar",
                                                     "apr", "may", "jun",
                                                     "jul", "aug", "sep",
                                                     "oct", "nov", "dec"])}
-        if value is None:
-            value = ''
-            max_age = 0
-            expires = timedelta(days=-1)
 
         if isinstance(expires, timedelta):
             expires = datetime.utcnow() + expires
 
         if expires is not None:
             expires_str = expires.strftime("%d %%s %Y %H:%M:%S GMT")
             expires_str = expires_str % days[expires.month]
             expires = expires_str
@@ -151,16 +157,18 @@ class Response(object):
         maybe_set("max-age", max_age)
         maybe_set("secure", secure)
         maybe_set("httponly", httponly)
 
         self.headers.append("Set-Cookie", m.OutputString())
 
     def unset_cookie(self, name):
         """Remove a cookie from those that are being sent with the response"""
+        if PY3:
+            name = isomorphic_decode(name)
         cookies = self.headers.get("Set-Cookie")
         parser = BaseCookie()
         for cookie in cookies:
             if PY3:
                 # BaseCookie.load expects a text string.
                 cookie = isomorphic_decode(cookie)
             parser.load(cookie)