Issue #23138: Fixed parsing cookies with absent keys or values in cookiejar.

Patch by Demian Brecht.

3 files changed, 57 insertions, 15 deletions

diff --git a/Lib/cookielib.py b/Lib/cookielib.py
index f260bc5..f2df467 100644
--- a/Lib/cookielib.py
+++ b/Lib/cookielib.py
@@ -464,26 +464,42 @@ def parse_ns_headers(ns_headers):
     for ns_header in ns_headers:
         pairs = []
         version_set = False
-        for ii, param in enumerate(re.split(r";\s*", ns_header)):
-            param = param.rstrip()
-            if param == "": continue
-            if "=" not in param:
-                k, v = param, None
-            else:
-                k, v = re.split(r"\s*=\s*", param, 1)
-                k = k.lstrip()
+
+        # XXX: The following does not strictly adhere to RFCs in that empty
+        # names and values are legal (the former will only appear once and will
+        # be overwritten if multiple occurrences are present). This is
+        # mostly to deal with backwards compatibility.
+        for ii, param in enumerate(ns_header.split(';')):
+            param = param.strip()
+
+            key, sep, val = param.partition('=')
+            key = key.strip()
+
+            if not key:
+                if ii == 0:
+                    break
+                else:
+                    continue
+
+            # allow for a distinction between present and empty and missing
+            # altogether
+            val = val.strip() if sep else None
+
             if ii != 0:
-                lc = k.lower()
+                lc = key.lower()
                 if lc in known_attrs:
-                    k = lc
-                if k == "version":
+                    key = lc
+
+                if key == "version":
                     # This is an RFC 2109 cookie.
-                    v = _strip_quotes(v)
+                    if val is not None:
+                        val = _strip_quotes(val)
                     version_set = True
-                if k == "expires":
+                elif key == "expires":
                     # convert expires date to seconds since epoch
-                    v = http2time(_strip_quotes(v))  # None if invalid
-            pairs.append((k, v))
+                    if val is not None:
+                        val = http2time(_strip_quotes(val))  # None if invalid
+            pairs.append((key, val))
 
         if pairs:
             if not version_set:
diff --git a/Lib/test/test_cookielib.py b/Lib/test/test_cookielib.py
index d4b80fa..d272d12 100644
--- a/Lib/test/test_cookielib.py
+++ b/Lib/test/test_cookielib.py
@@ -445,6 +445,9 @@ class CookieTests(TestCase):
         interact_netscape(c, "http://www.acme.com:80/", 'foo=bar; expires=')
         interact_netscape(c, "http://www.acme.com:80/", 'spam=eggs; '
                           'expires="Foo Bar 25 33:22:11 3022"')
+        interact_netscape(c, 'http://www.acme.com/', 'fortytwo=')
+        interact_netscape(c, 'http://www.acme.com/', '=unladenswallow')
+        interact_netscape(c, 'http://www.acme.com/', 'holyhandgrenade')
 
         cookie = c._cookies[".acme.com"]["/"]["spam"]
         self.assertEqual(cookie.domain, ".acme.com")
@@ -471,6 +474,16 @@ class CookieTests(TestCase):
         self.assertIsNone(foo.expires)
         self.assertIsNone(spam.expires)
 
+        cookie = c._cookies['www.acme.com']['/']['fortytwo']
+        self.assertIsNotNone(cookie.value)
+        self.assertEqual(cookie.value, '')
+
+        # there should be a distinction between a present but empty value
+        # (above) and a value that's entirely missing (below)
+
+        cookie = c._cookies['www.acme.com']['/']['holyhandgrenade']
+        self.assertIsNone(cookie.value)
+
     def test_ns_parser_special_names(self):
         # names such as 'expires' are not special in first name=value pair
         # of Set-Cookie: header
@@ -1092,6 +1105,13 @@ class CookieTests(TestCase):
             parse_ns_headers(["foo"]),
             [[("foo", None), ("version", "0")]]
             )
+        # missing cookie values for parsed attributes
+        self.assertEqual(
+            parse_ns_headers(['foo=bar; expires']),
+            [[('foo', 'bar'), ('expires', None), ('version', '0')]])
+        self.assertEqual(
+            parse_ns_headers(['foo=bar; version']),
+            [[('foo', 'bar'), ('version', None)]])
         # shouldn't add version if header is empty
         self.assertEqual(parse_ns_headers([""]), [])
 
@@ -1106,6 +1126,8 @@ class CookieTests(TestCase):
             c.extract_cookies(r, req)
             return c
 
+        future = cookielib.time2netscape(time.time()+3600)
+
         # none of these bad headers should cause an exception to be raised
         for headers in [
             ["Set-Cookie: "],  # actually, nothing wrong with this
@@ -1116,6 +1138,7 @@ class CookieTests(TestCase):
             ["Set-Cookie: b=foo; max-age=oops"],
             # bad version
             ["Set-Cookie: b=foo; version=spam"],
+            ["Set-Cookie:; Expires=%s" % future],
             ]:
             c = cookiejar_from_cookie_headers(headers)
             # these bad cookies shouldn't be set
diff --git a/Misc/NEWS b/Misc/NEWS
index a364f57..ac03bf7 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -21,6 +21,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #23138: Fixed parsing cookies with absent keys or values in cookiejar.
+  Patch by Demian Brecht.
+
 - Issue #23051: multiprocessing.Pool methods imap() and imap_unordered() now
   handle exceptions raised by an iterator.  Patch by Alon Diamant and Davin
   Potts.