fix cookie path parameter handling --> #1052 (#1053)

* fix cookie path parameter handling --> #1052

* change logic: use cookie path value unchanged it it contains either `%` (the application did all necessary quoting) or contains only characters allowed (unquoted) in an URL path (no quoting necessary); otherwise, apply `urllib.parse.quote`

* make `flake8` happy

* update `HTTPBaseResponse.cookies` only if `setCookie` does not raise an exception

CHANGES.rst

@@ -11,6 +11,15 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
 
 - Update dependencies to the latest releases for each supported Python version.
 
+- Fix cookie path parameter handling:
+  If the cookie path value contains ``%`` it is assumed to be
+  fully quoted and used as is;
+  if it contains only characters allowed (unquoted)
+  in an URL path (with the exception of ``;``),
+  it is used as is; otherwise, it is quoted using Python's
+  ``urllib.parse.quote``
+  (`#1052 <https://github.com/zopefoundation/Zope/issues/1052>`_).
+
 
 4.8.2 (2022-06-01)
 ------------------

src/ZPublisher/HTTPResponse.py

@@ -317,11 +317,7 @@ def setCookie(self, name, value, quoted=True, **kw):
             name = str(name)
             value = str(value)
 
-        cookies = self.cookies
-        if name in cookies:
-            cookie = cookies[name]
-        else:
-            cookie = cookies[name] = {}
+        cookie = {}
         for k, v in kw.items():
             k, v = convertCookieParameter(k, v)
             cookie[k] = v
@@ -330,6 +326,13 @@ def setCookie(self, name, value, quoted=True, **kw):
         # cookie['quoted'] = quoted
         getCookieParamPolicy().check_consistency(name, cookie)
 
+        # update ``self.cookies`` only if there have been no exceptions
+        cookies = self.cookies
+        if name in cookies:
+            cookies[name].update(cookie)
+        else:
+            cookies[name] = cookie
+
     def appendCookie(self, name, value):
         """ Set an HTTP cookie.
 

src/ZPublisher/cookie.py

@@ -255,25 +255,40 @@ def domain_converter(value):
 
 
 # ``Path``
-# we include "%" to handle the case that the path has already been quoted
-path_safe = compile(r"[a-zA-Z0-9_/%]*$").match
+path_safe = compile("["
+                    "-_.!~*'()"  # RFC 2396 section 2.3 (unreserved)
+                    "a-zA-Z0-9"  # letters and digits
+                    "/:@&=+$,"   # RFC 2396 section 3.3 (path reserved)
+                                 # excluding ``;``
+                    "]*$").match
 
 
 def path_converter(value):
-    """convert *value* to a path.
+    """convert *value* to a cookie path.
 
     The convertion is based on ``absolute_url_path``.
     If *value* lacks this method, it is assumed to be a string.
-    If the string contains ``%``, it is assumed that it is already
-    quoted.
+    If the string contains ``%``, it is used as is; otherwise,
+    it may be quoted by Python's ``urllib.parse.quote``.
+
+    **Note**:
+    According to RFC 6265 section 5.1.4 a cookie path match **does not**
+    unquote its arguments. It is therefore important to use
+    the same quoting algorithm for the URL and the cookie path.
+    If the cookie path contains only allowed characters
+    (RFC 2396 unreserved (section 2.3) and
+    RFC 2396 path special characters (section 3.3) excluding ``;``),
+    the value is taken verbatim; otherwise it is quoted
+    by Python's `urllib.parse.quote` (which
+    is used by `OFS.Traversable` as its ULR quoting). It quotes
+    all special characters apart from ``-_./``.
+    Quote yourself if this does not match your URL quoting.
     """
     ap = getattr(aq_base(value), "absolute_url_path", None)
     if ap is not None:
         return value.absolute_url_path()
-    if path_safe(value):
+    if "%" in value or path_safe(value):
         return value
-    if "%" in value:
-        raise ValueError("not path safe but apparently quoted")
     return quote(value)
 
 

src/ZPublisher/tests/test_cookie.py

@@ -170,9 +170,9 @@ def absolute_url_path(self):
         # check quote
         _, v = convertCookieParameter("path", "/a/ /c")
         self.assertEqual(v, "/a/%20/c")
-        # check incomplete quoting
-        with self.assertRaises(ValueError):
-            convertCookieParameter("path", "/a/%20/ ")
+        # check ``%`` dominates
+        _, v = convertCookieParameter("path", "/a/%20/ ")
+        self.assertEqual(v, "/a/%20/ ")
 
     def test_http_only(self):
         # test true