Repository: pallets/werkzeug
Pull request #2618 (2023-03-09 20:33:25)

refactor parse and dump header functions
* `parse_list_header` uses `urllib.request.parse_http_list`, which already unquotes and removes slash escapes. `parse_dict_header` and a few other places can remove inner `key="value"` quotes inline, without having to call `unquote_header_value`.
* ``quote_header_value`` quotes the empty string, since an empty value isn't a valid token on its own.
* `quote_header_value` `extra_chars` parameter is deprecated, it wasn't used anywhere and doesn't seem necessary for any HTTP header.
* As part of #2602, passing bytes to `quote_header_value` and `parse_dict_header` is deprecated.
* `dump_options_header` skips `None` values instead of writing a bare `key;`. While bare keys are useful for headers that use `dump_header`, they're not valid in `value; key=value` parameters.
* Inline the helper function that just did `if value[-1] == "*"` in `dump_header` and `dump_options_header`.
* `dump_header` `allow_token` parameter is deprecated. `quote_header_value` still has the parameter, mainly for the `Digest` auth scheme.
* `parse_dict_header` supports `key*=UTF-8''value` items the same way `parse_options_header` does.


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 1f49ad50..f9b91ed0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -35,8 +35,17 @@ Unreleased
 -   ``parse_options_header`` is 2-3 times faster. It conforms to :rfc:`9110`, some
     invalid parts that were previously accepted are now ignored. :issue:`1628`
 -   The ``is_filename`` parameter to ``unquote_header_value`` is deprecated. :pr:`2614`
+-   Deprecate the ``extra_chars`` parameter and passing bytes to ``quote_header_value``,
+    the ``allow_token`` parameter to ``dump_header``, and the ``cls`` parameter and
+    passing bytes to ``parse_dict_header``. :pr:`2618`
 -   Improve ``parse_accept_header`` implementation. Parse according to :rfc:`9110`.
     Discard items with invalid ``q`` values. :issue:`1623`
+-   ``quote_header_value`` quotes the empty string. :pr:`2618`
+-   ``dump_options_header`` skips ``None`` values rather than using a bare key.
+    :pr:`2618`
+-   ``dump_header`` and ``dump_options_header`` will not quote a value if the key ends
+    with an asterisk ``*``.
+-   ``parse_dict_header`` will decode values with charsets. :pr:`2618`
 
 
 Version 2.2.3
diff --git a/src/werkzeug/http.py b/src/werkzeug/http.py
index 45eccb2d..e3a6b4ae 100644
--- a/src/werkzeug/http.py
+++ b/src/werkzeug/http.py
@@ -139,42 +139,72 @@ class COOP(Enum):
     SAME_ORIGIN = "same-origin"
 
 
-def _is_extended_parameter(key: str) -> bool:
-    """Per RFC 5987/8187, "extended" values may *not* be quoted.
-    This is in keeping with browser implementations. So we test
-    using this function to see if the key indicates this parameter
-    follows the `ext-parameter` syntax (using a trailing '*').
-    """
-    return key.strip().endswith("*")
-
-
 def quote_header_value(
-    value: t.Union[str, int], extra_chars: str = "", allow_token: bool = True
+    value: t.Any,
+    extra_chars: t.Optional[str] = None,
+    allow_token: bool = True,
 ) -> str:
-    """Quote a header value if necessary.
+    """Add double quotes around a header value. If the header contains only ASCII token
+    characters, it will be returned unchanged. If the header contains ``"`` or ``\\``
+    characters, they will be escaped with an additional ``\\`` character.
 
-    .. versionadded:: 0.5
+    This is the reverse of :func:`unquote_header_value`.
+
+    :param value: The value to quote. Will be converted to a string.
+    :param allow_token: Disable to quote the value even if it only has token characters.
+
+    .. versionchanged:: 2.3
+        The value is quoted if it is the empty string.
 
-    :param value: the value to quote.
-    :param extra_chars: a list of extra characters to skip quoting.
-    :param allow_token: if this is enabled token values are returned
-                        unchanged.
+    .. versionchanged:: 2.3
+        Passing bytes is deprecated and will not be supported in Werkzeug 2.4.
+
+    .. versionchanged:: 2.3
+        The ``extra_chars`` parameter is deprecated and will be removed in Werkzeug 2.4.
+
+    .. versionadded:: 0.5
     """
     if isinstance(value, bytes):
+        warnings.warn(
+            "Passing bytes is deprecated and will not be supported in Werkzeug 2.4.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         value = value.decode("latin1")
+
+    if extra_chars is not None:
+        warnings.warn(
+            "The 'extra_chars' parameter is deprecated and will be"
+            " removed in Werkzeug 2.4.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
     value = str(value)
+
+    if not value:
+        return '""'
+
     if allow_token:
-        token_chars = _token_chars | set(extra_chars)
-        if set(value).issubset(token_chars):
+        token_chars = _token_chars
+
+        if extra_chars:
+            token_chars |= set(extra_chars)
+
+        if token_chars.issuperset(value):
             return value
+
     value = value.replace("\\", "\\\\").replace('"', '\\"')
     return f'"{value}"'
 
 
 def unquote_header_value(value: str, is_filename: t.Optional[bool] = None) -> str:
-    """Unquote a header value. Reverse of :func:`quote_header_value`.
+    """Remove double quotes and decode slash-escaped ``"`` and ``\\`` characters in a
+    header value.
+
+    This is the reverse of :func:`quote_header_value`.
 
-    :param value: the header value to unquote.
+    :param value: The header value to unquote.
 
     .. versionchanged:: 2.3
         The ``is_filename`` parameter is deprecated and will be removed in Werkzeug 2.4.
@@ -187,7 +217,7 @@ def unquote_header_value(value: str, is_filename: t.Optional[bool] = None) -> st
             stacklevel=2,
         )
 
-    if value and value[0] == value[-1] == '"':
+    if len(value) >= 2 and value[0] == value[-1] == '"':
         value = value[1:-1]
 
         if not is_filename:
@@ -196,51 +226,104 @@ def unquote_header_value(value: str, is_filename: t.Optional[bool] = None) -> st
     return value
 
 
-def dump_options_header(
-    header: t.Optional[str], options: t.Mapping[str, t.Optional[t.Union[str, int]]]
-) -> str:
-    """The reverse function to :func:`parse_options_header`.
+def dump_options_header(header: t.Optional[str], options: t.Mapping[str, t.Any]) -> str:
+    """Produce a header value and ``key=value`` parameters separated by semicolons
+    ``;``. For example, the ``Content-Type`` header.
+
+    .. code-block:: python
+
+        dump_options_header("text/html", {"charset": "UTF-8"})
+        'text/html; charset=UTF-8'
+
+    This is the reverse of :func:`parse_options_header`.
 
-    :param header: the header to dump
-    :param options: a dict of options to append.
+    If a value contains non-token characters, it will be quoted.
+
+    If a value is ``None``, the parameter is skipped.
+
+    In some keys for some headers, a UTF-8 value can be encoded using a special
+    ``key*=UTF-8''value`` form, where ``value`` is percent encoded. This function will
+    not produce that format automatically, but if a given key ends with an asterisk
+    ``*``, the value is assumed to have that form and will not be quoted further.
+
+    :param header: The primary header value.
+    :param options: Parameters to encode as ``key=value`` pairs.
+
+    .. versionchanged:: 2.3
+        Keys with ``None`` values are skipped rather than treated as a bare key.
+
+    .. versionchanged:: 2.2.3
+        If a key ends with ``*``, its value will not be quoted.
     """
     segments = []
+
     if header is not None:
         segments.append(header)
+
     for key, value in options.items():
         if value is None:
-            segments.append(key)
-        elif _is_extended_parameter(key):
+            continue
+
+        if key[-1] == "*":
             segments.append(f"{key}={value}")
         else:
             segments.append(f"{key}={quote_header_value(value)}")
+
     return "; ".join(segments)
 
 
 def dump_header(
-    iterable: t.Union[t.Dict[str, t.Union[str, int]], t.Iterable[str]],
-    allow_token: bool = True,
+    iterable: t.Union[t.Dict[str, t.Any], t.Iterable[t.Any]],
+    allow_token: t.Optional[bool] = None,
 ) -> str:
-    """Dump an HTTP header again.  This is the reversal of
-    :func:`parse_list_header`, :func:`parse_set_header` and
-    :func:`parse_dict_header`.  This also quotes strings that include an
-    equals sign unless you pass it as dict of key, value pairs.
-
-    >>> dump_header({'foo': 'bar baz'})
-    'foo="bar baz"'
-    >>> dump_header(('foo', 'bar baz'))
-    'foo, "bar baz"'
-
-    :param iterable: the iterable or dict of values to quote.
-    :param allow_token: if set to `False` tokens as values are disallowed.
-                        See :func:`quote_header_value` for more details.
+    """Produce a header value from a list of items or ``key=value`` pairs, separated by
+    commas ``,``.
+
+    This is the reverse of :func:`parse_list_header`, :func:`parse_dict_header`, and
+    :func:`parse_set_header`.
+
+    If a value contains non-token characters, it will be quoted.
+
+    If a value is ``None``, the key is output alone.
+
+    In some keys for some headers, a UTF-8 value can be encoded using a special
+    ``key*=UTF-8''value`` form, where ``value`` is percent encoded. This function will
+    not produce that format automatically, but if a given key ends with an asterisk
+    ``*``, the value is assumed to have that form and will not be quoted further.
+
+    .. code-block:: python
+
+        dump_header(["foo", "bar baz"])
+        'foo, "bar baz"'
+
+        dump_header({"foo": "bar baz"})
+        'foo="bar baz"'
+
+    :param iterable: The items to create a header from.
+
+    .. versionchanged:: 2.3
+        The ``allow_token`` parameter is deprecated and will be removed in Werkzeug 2.4.
+
+    .. versionchanged:: 2.2.3
+        If a key ends with ``*``, its value will not be quoted.
     """
+    if allow_token is not None:
+        warnings.warn(
+            "'The 'allow_token' parameter is deprecated and will be"
+            " removed in Werkzeug 2.4.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+    else:
+        allow_token = True
+
     if isinstance(iterable, dict):
         items = []
+
         for key, value in iterable.items():
             if value is None:
                 items.append(key)
-            elif _is_extended_parameter(key):
+            elif key[-1] == "*":
                 items.append(f"{key}={value}")
             else:
                 items.append(
@@ -248,6 +331,7 @@ def dump_header(
                 )
     else:
         items = [quote_header_value(x, allow_token=allow_token) for x in iterable]
+
     return ", ".join(items)
 
 
@@ -276,7 +360,7 @@ def parse_list_header(value: str) -> t.List[str]:
         parse_list_header('token, "quoted value"')
         ['token', 'quoted value']
 
-    Use :func:`dump_header` to convert a list of items to a header value.
+    This is the reverse of :func:`dump_header`.
 
     :param value: The header value to parse.
     """
@@ -291,43 +375,92 @@ def parse_list_header(value: str) -> t.List[str]:
     return result
 
 
-def parse_dict_header(value: str, cls: t.Type[dict] = dict) -> t.Dict[str, str]:
-    """Parse lists of key, value pairs as described by RFC 2068 Section 2 and
-    convert them into a python dict (or any other mapping object created from
-    the type with a dict like interface provided by the `cls` argument):
+def parse_dict_header(
+    value: str, cls: t.Optional[t.Type[dict]] = None
+) -> t.Dict[str, str]:
+    """Parse a list header using :func:`parse_list_header`, then parse each item as a
+    ``key=value`` pair.
 
-    >>> d = parse_dict_header('foo="is a fish", bar="as well"')
-    >>> type(d) is dict
-    True
-    >>> sorted(d.items())
-    [('bar', 'as well'), ('foo', 'is a fish')]
+    .. code-block:: python
 
-    If there is no value for a key it will be `None`:
+        parse_dict_header('a=b, c="d, e", f')
+        {"a": "b", "c": "d, e", "f": None}
 
-    >>> parse_dict_header('key_without_value')
-    {'key_without_value': None}
+    This is the reverse of :func:`dump_header`.
 
-    To create a header from the :class:`dict` again, use the
-    :func:`dump_header` function.
+    If a key does not have a value, it is ``None``.
 
-    .. versionchanged:: 0.9
-       Added support for `cls` argument.
+    This handles charsets for values as described in
+    `RFC 2231 <https://www.rfc-editor.org/rfc/rfc2231#section-3>`__. Only ASCII, UTF-8,
+    and ISO-8859-1 charsets are accepted, otherwise the value remains quoted.
+
+    :param value: The header value to parse.
+
+    .. versionchanged:: 2.3
+        Added support for ``key*=charset''value`` encoded items.
+
+    .. versionchanged:: 2.3
+        Passing bytes is deprecated, support will be removed in Werkzeug 2.4.
 
-    :param value: a string with a dict header.
-    :param cls: callable to use for storage of parsed results.
-    :return: an instance of `cls`
+    .. versionchanged:: 2.3
+        The ``cls`` argument is deprecated and will be removed in Werkzeug 2.4.
+
+    .. versionchanged:: 0.9
+       The ``cls`` argument was added.
     """
+    if cls is None:
+        cls = dict
+    else:
+        warnings.warn(
+            "The 'cls' parameter is deprecated and will be removed in Werkzeug 2.4.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
     result = cls()
+
     if isinstance(value, bytes):
+        warnings.warn(
+            "Passing bytes is deprecated and will be removed in Werkzeug 2.4.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         value = value.decode("latin1")
-    for item in _parse_list_header(value):
-        if "=" not in item:
-            result[item] = None
+
+    for item in parse_list_header(value):
+        key, has_value, value = item.partition("=")
+        key = key.strip()
+
+        if not has_value:
+            result[key] = None
             continue
-        name, value = item.split("=", 1)
-        if value[:1] == value[-1:] == '"':
-            value = unquote_header_value(value[1:-1])
-        result[name] = value
+
+        value = value.strip()
+        encoding: t.Optional[str] = None
+
+        if key[-1] == "*":
+            # key*=charset''value becomes key=value, where value is percent encoded
+            # adapted from parse_options_header, without the continuation handling
+            key = key[:-1]
+            match = _charset_value_re.match(value)
+
+            if match:
+                # If there is a charset marker in the value, split it off.
+                encoding, value = match.groups()
+                encoding = encoding.lower()
+
+            # A safe list of encodings. Modern clients should only send ASCII or UTF-8.
+            # This list will not be extended further. An invalid encoding will leave the
+            # value quoted.
+            if encoding in {"ascii", "us-ascii", "utf-8", "iso-8859-1"}:
+                # invalid bytes are replaced during unquoting
+                value = unquote(value, encoding=encoding)
+
+        if len(value) >= 2 and value[0] == value[-1] == '"':
+            value = value[1:-1]
+
+        result[key] = value
+
     return result
 
 
@@ -373,6 +506,8 @@ def parse_options_header(value: t.Optional[str]) -> t.Tuple[str, t.Dict[str, str
         parse_options_header("")
         ("", {})
 
+    This is the reverse of :func:`dump_options_header`.
+
     This parses valid parameter parts as described in
     `RFC 9110 <https://httpwg.org/specs/rfc9110.html#parameter>`__. Invalid parts are
     skipped.
@@ -380,8 +515,7 @@ def parse_options_header(value: t.Optional[str]) -> t.Tuple[str, t.Dict[str, str
     This handles continuations and charsets as described in
     `RFC 2231 <https://www.rfc-editor.org/rfc/rfc2231#section-3>`__, although not as
     strictly as the RFC. Only ASCII, UTF-8, and ISO-8859-1 charsets are accepted,
-    otherwise the value remains quoted. These features should not be sent by modern
-    clients.
+    otherwise the value remains quoted.
 
     Clients may not be consistent in how they handle a quote character within a quoted
     value. The `HTML Standard <https://html.spec.whatwg.org/#multipart-form-data>`__


Test patch:
diff --git a/tests/test_http.py b/tests/test_http.py
index 0a02f1d7..72ca72e7 100644
--- a/tests/test_http.py
+++ b/tests/test_http.py
@@ -351,15 +351,10 @@ class TestHTTPUtility:
 
     def test_dump_options_header(self):
         assert http.dump_options_header("foo", {"bar": 42}) == "foo; bar=42"
-        assert http.dump_options_header("foo", {"bar": 42, "fizz": None}) in (
-            "foo; bar=42; fizz",
-            "foo; fizz; bar=42",
-        )
+        assert "fizz" not in http.dump_options_header("foo", {"bar": 42, "fizz": None})
 
     def test_dump_header(self):
         assert http.dump_header([1, 2, 3]) == "1, 2, 3"
-        assert http.dump_header([1, 2, 3], allow_token=False) == '"1", "2", "3"'
-        assert http.dump_header({"foo": "bar"}, allow_token=False) == 'foo="bar"'
         assert http.dump_header({"foo": "bar"}) == "foo=bar"
         assert http.dump_header({"foo*": "UTF-8''bar"}) == "foo*=UTF-8''bar"