Repository: pallets/werkzeug
Pull request #1458 (2019-02-14 14:00:09)

Multiple cookies can have the same name
Due to my own errors I ended up in a situation where two cookies had the same name. With a little bit of googling I realised this was something that is part of the RFC (linked in this [stackoverflow answer](https://stackoverflow.com/a/24214538)).

But the default in Flask (from the defaults of werkzeug) is to support only one cookie with the same name. It is fairly easy to fix in the use code (subclassing the response object), but it feels like the default should follow the RFC.

Just one caveat. The `ImmutableMultiDict` and the `ImmutableTypeConversionDict` have different defaults on how they handle update to the same key. In the first case, the first value is returned, while in the second case the last value is returned. 

```.py
>>> ImmutableMultiDict([['foo', 'bar'], ['foo', 'baz']]).get('foo')
'bar'

>>> ImmutableTypeConversionDict([['foo', 'bar'], ['foo', 'baz']]).get('foo')
'baz'
```

This might break some people's code (even if they should not have relied on the order of the cookies, according to the RFC). This could be changed easily, but make the code slightly more complex (and arcane).

davidism: I've applied some global code style cleanup and changes, and as a result this PR will no longer merge. Would you mind recreating it against the current mater? Sorry for the inconvenience.
sgenoud: No problem, I have rebased it!

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 01f386f9..12da41a9 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -13,6 +13,11 @@ Unreleased
 -   Directive keys for the ``Set-Cookie`` response header are not
     ignored when parsing the ``Cookie`` request header. This allows
     cookies with names such as "expires" and "version". (:issue:`1495`)
+-   Request cookies are parsed into a ``MultiDict`` to capture all
+    values for cookies with the same key. ``cookies[key]`` returns the
+    first value rather than the last. Use ``cookies.getlist(key)`` to
+    get all values. ``parse_cookie`` also defaults to a ``MultiDict``.
+    :issue:`1562`, :pr:`1458`
 -   Add ``charset=utf-8`` to an HTTP exception response's
     ``CONTENT_TYPE`` header. (:pr:`1526`)
 -   The interactive debugger handles outer variables in nested scopes
diff --git a/src/werkzeug/http.py b/src/werkzeug/http.py
index 6d85ffb2..d5490bcc 100644
--- a/src/werkzeug/http.py
+++ b/src/werkzeug/http.py
@@ -1078,38 +1078,40 @@ def is_hop_by_hop_header(header):
 
 
 def parse_cookie(header, charset="utf-8", errors="replace", cls=None):
-    """Parse a cookie.  Either from a string or WSGI environ.
+    """Parse a cookie from a string or WSGI environ.
 
-    Per default encoding errors are ignored.  If you want a different behavior
-    you can set `errors` to ``'replace'`` or ``'strict'``.  In strict mode a
-    :exc:`HTTPUnicodeError` is raised.
+    The same key can be provided multiple times, the values are stored
+    in-order. The default :class:`MultiDict` will have the first value
+    first, and all values can be retrieved with
+    :meth:`MultiDict.getlist`.
+
+    :param header: The cookie header as a string, or a WSGI environ dict
+        with a ``HTTP_COOKIE`` key.
+    :param charset: The charset for the cookie values.
+    :param errors: The error behavior for the charset decoding.
+    :param cls: A dict-like class to store the parsed cookies in.
+        Defaults to :class:`MultiDict`.
+
+    .. versionchanged:: 1.0.0
+        Returns a :class:`MultiDict` instead of a
+        ``TypeConversionDict``.
 
     .. versionchanged:: 0.5
-       This function now returns a :class:`TypeConversionDict` instead of a
-       regular dict.  The `cls` parameter was added.
-
-    :param header: the header to be used to parse the cookie.  Alternatively
-                   this can be a WSGI environment.
-    :param charset: the charset for the cookie values.
-    :param errors: the error behavior for the charset decoding.
-    :param cls: an optional dict class to use.  If this is not specified
-                       or `None` the default :class:`TypeConversionDict` is
-                       used.
+       Returns a :class:`TypeConversionDict` instead of a regular dict.
+       The ``cls`` parameter was added.
     """
     if isinstance(header, dict):
         header = header.get("HTTP_COOKIE", "")
     elif header is None:
         header = ""
 
-    # If the value is an unicode string it's mangled through latin1.  This
-    # is done because on PEP 3333 on Python 3 all headers are assumed latin1
-    # which however is incorrect for cookies, which are sent in page encoding.
-    # As a result we
+    # On Python 3, PEP 3333 sends headers through the environ as latin1
+    # decoded strings. Encode strings back to bytes for parsing.
     if isinstance(header, text_type):
         header = header.encode("latin1", "replace")
 
     if cls is None:
-        cls = TypeConversionDict
+        cls = MultiDict
 
     def _parse_pairs():
         for key, val in _cookie_parse_impl(header):
@@ -1287,8 +1289,8 @@ from .datastructures import ContentSecurityPolicy
 from .datastructures import ETags
 from .datastructures import HeaderSet
 from .datastructures import IfRange
+from .datastructures import MultiDict
 from .datastructures import Range
 from .datastructures import RequestCacheControl
-from .datastructures import TypeConversionDict
 from .datastructures import WWWAuthenticate
 from .urls import iri_to_uri
diff --git a/src/werkzeug/wrappers/base_request.py b/src/werkzeug/wrappers/base_request.py
index d4655a4f..e18949b3 100644
--- a/src/werkzeug/wrappers/base_request.py
+++ b/src/werkzeug/wrappers/base_request.py
@@ -9,7 +9,6 @@ from ..datastructures import CombinedMultiDict
 from ..datastructures import EnvironHeaders
 from ..datastructures import ImmutableList
 from ..datastructures import ImmutableMultiDict
-from ..datastructures import ImmutableTypeConversionDict
 from ..datastructures import iter_multi_items
 from ..datastructures import MultiDict
 from ..formparser import default_stream_factory
@@ -116,13 +115,15 @@ class BaseRequest(object):
     #: .. versionadded:: 0.6
     list_storage_class = ImmutableList
 
-    #: the type to be used for dict values from the incoming WSGI environment.
-    #: By default an
-    #: :class:`~werkzeug.datastructures.ImmutableTypeConversionDict` is used
-    #: (for example for :attr:`cookies`).
+    #: The type to be used for dict values from the incoming WSGI
+    #: environment. (For example for :attr:`cookies`.) By default an
+    #: :class:`~werkzeug.datastructures.ImmutableMultiDict` is used.
+    #:
+    #: .. versionchanged:: 1.0.0
+    #:     Changed to ``ImmutableMultiDict`` to support multiple values.
     #:
     #: .. versionadded:: 0.6
-    dict_storage_class = ImmutableTypeConversionDict
+    dict_storage_class = ImmutableMultiDict
 
     #: The form data parser that shoud be used.  Can be replaced to customize
     #: the form date parsing.


Test patch:
diff --git a/tests/test_http.py b/tests/test_http.py
index 7fbef44f..61386b34 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -442,22 +442,22 @@ class TestHTTPUtility(object):
         assert http.http_date(0) == "Thu, 01 Jan 1970 00:00:00 GMT"
         assert http.http_date(datetime(1970, 1, 1)) == "Thu, 01 Jan 1970 00:00:00 GMT"
 
-    def test_cookies(self):
-        strict_eq(
-            dict(
-                http.parse_cookie(
-                    "dismiss-top=6; CP=null*; PHPSESSID=0a539d42abc001cd"
-                    'c762809248d4beed; a=42; b="\\";"'
-                )
-            ),
-            {
-                "CP": u"null*",
-                "PHPSESSID": u"0a539d42abc001cdc762809248d4beed",
-                "a": u"42",
-                "dismiss-top": u"6",
-                "b": u'";',
-            },
-        )
+    def test_parse_cookie(self):
+        cookies = http.parse_cookie(
+            "dismiss-top=6; CP=null*; PHPSESSID=0a539d42abc001cdc762809248d4beed;"
+            ' a=42; b="\\";"; ; fo234{=bar;blub=Blah;'
+        )
+        assert cookies.to_dict() == {
+            "CP": u"null*",
+            "PHPSESSID": u"0a539d42abc001cdc762809248d4beed",
+            "a": u"42",
+            "dismiss-top": u"6",
+            "b": u'";',
+            "fo234{": u"bar",
+            "blub": u"Blah",
+        }
+
+    def test_dump_cookie(self):
         rv = http.dump_cookie(
             "foo", "bar baz blub", 360, httponly=True, sync_expires=False
         )
@@ -468,42 +468,37 @@ class TestHTTPUtility(object):
             "Path=/",
             'foo="bar baz blub"',
         }
-
-        strict_eq(
-            dict(http.parse_cookie("fo234{=bar; blub=Blah")),
-            {"fo234{": u"bar", "blub": u"Blah"},
-        )
-
-        strict_eq(http.dump_cookie("key", "xxx/"), "key=xxx/; Path=/")
-        strict_eq(http.dump_cookie("key", "xxx="), "key=xxx=; Path=/")
+        assert http.dump_cookie("key", "xxx/") == "key=xxx/; Path=/"
+        assert http.dump_cookie("key", "xxx=") == "key=xxx=; Path=/"
 
     def test_bad_cookies(self):
-        strict_eq(
-            dict(
-                http.parse_cookie(
-                    "first=IamTheFirst ; a=1; oops ; a=2 ;second = andMeTwo;"
-                )
-            ),
-            {"first": u"IamTheFirst", "a": u"2", "oops": u"", "second": u"andMeTwo"},
-        )
+        cookies = http.parse_cookie(
+            "first=IamTheFirst ; a=1; oops ; a=2 ;second = andMeTwo;"
+        )
+        expect = {
+            "first": [u"IamTheFirst"],
+            "a": [u"1", u"2"],
+            "oops": [u""],
+            "second": [u"andMeTwo"],
+        }
+        assert cookies.to_dict(flat=False) == expect
+        assert cookies["a"] == u"1"
+        assert cookies.getlist("a") == [u"1", u"2"]
 
     def test_empty_keys_are_ignored(self):
-        strict_eq(
-            dict(
-                http.parse_cookie("first=IamTheFirst ; a=1; a=2 ;second=andMeTwo; ; ")
-            ),
-            {"first": u"IamTheFirst", "a": u"2", "second": u"andMeTwo"},
-        )
+        cookies = http.parse_cookie("spam=ham; duck=mallard; ; ")
+        expect = {"spam": u"ham", "duck": u"mallard"}
+        assert cookies.to_dict() == expect
 
     def test_cookie_quoting(self):
         val = http.dump_cookie("foo", "?foo")
-        strict_eq(val, 'foo="?foo"; Path=/')
-        strict_eq(dict(http.parse_cookie(val)), {"foo": u"?foo", "Path": u"/"})
-        strict_eq(dict(http.parse_cookie(r'foo="foo\054bar"')), {"foo": u"foo,bar"})
+        assert val == 'foo="?foo"; Path=/'
+        assert http.parse_cookie(val).to_dict() == {"foo": u"?foo", "Path": u"/"}
+        assert http.parse_cookie(r'foo="foo\054bar"').to_dict(), {"foo": u"foo,bar"}
 
     def test_parse_set_cookie_directive(self):
         val = 'foo="?foo"; version="0.1";'
-        strict_eq(dict(http.parse_cookie(val)), {"foo": u"?foo", "version": u"0.1"})
+        assert http.parse_cookie(val).to_dict() == {"foo": u"?foo", "version": u"0.1"}
 
     def test_cookie_domain_resolving(self):
         val = http.dump_cookie("foo", "bar", domain=u"\N{SNOWMAN}.com")
diff --git a/tests/test_wrappers.py b/tests/test_wrappers.py
index 03005325..b0ade103 100644
--- a/tests/test_wrappers.py
+++ b/tests/test_wrappers.py
@@ -28,8 +28,8 @@ from werkzeug.datastructures import CharsetAccept
 from werkzeug.datastructures import CombinedMultiDict
 from werkzeug.datastructures import Headers
 from werkzeug.datastructures import ImmutableList
+from werkzeug.datastructures import ImmutableMultiDict
 from werkzeug.datastructures import ImmutableOrderedMultiDict
-from werkzeug.datastructures import ImmutableTypeConversionDict
 from werkzeug.datastructures import LanguageAccept
 from werkzeug.datastructures import MIMEAccept
 from werkzeug.datastructures import MultiDict
@@ -1247,9 +1247,12 @@ def test_storage_classes():
     assert type(req.values) is CombinedMultiDict
     assert req.values["foo"] == u"baz"
 
-    req = wrappers.Request.from_values(headers={"Cookie": "foo=bar"})
-    assert type(req.cookies) is ImmutableTypeConversionDict
-    assert req.cookies == {"foo": "bar"}
+    req = wrappers.Request.from_values(headers={"Cookie": "foo=bar;foo=baz"})
+    assert type(req.cookies) is ImmutableMultiDict
+    assert req.cookies.to_dict() == {"foo": "bar"}
+
+    # it is possible to have multiple cookies with the same name
+    assert req.cookies.getlist("foo") == ["bar", "baz"]
     assert type(req.access_route) is ImmutableList
 
     MyRequest.list_storage_class = tuple