diff --git a/CHANGES/1342.bugfix.rst b/CHANGES/1342.bugfix.rst new file mode 100644 index 000000000..479065284 --- /dev/null +++ b/CHANGES/1342.bugfix.rst @@ -0,0 +1,5 @@ +Fixed ``__repr__`` of :class:`~multidict.MultiDict`, +:class:`~multidict.CIMultiDict`, their proxies, and the keys/items views +producing invalid output when keys contained quote characters -- +keys are now formatted with :func:`repr` so the result is a valid Python +string literal -- by :user:`aiolibsbot`. diff --git a/multidict/_multidict_py.py b/multidict/_multidict_py.py index e286713ec..d34f90d83 100644 --- a/multidict/_multidict_py.py +++ b/multidict/_multidict_py.py @@ -1,5 +1,6 @@ import enum import functools +import re import reprlib import sys from array import array @@ -47,6 +48,17 @@ class istr(str): _version = array("Q", [0]) +# Matches any character that ``repr()`` would escape inside single quotes. +_repr_needs_repr = re.compile(r"[^\x20-\x26\x28-\x5b\x5d-\x7e]").search + + +def _key_repr(key: str) -> str: + # Skip ``repr()`` for the common case where single-quote wrapping suffices. + if _repr_needs_repr(key) is None: + return f"'{key}'" + return repr(key) + + class _Iter(Generic[_T]): __slots__ = ("_size", "_iter") @@ -103,7 +115,7 @@ def _iter(self, version: int) -> Iterator[tuple[str, _V]]: def __repr__(self) -> str: lst = [] for e in self._md._keys.iter_entries(): - lst.append(f"'{e.key}': {e.value!r}") + lst.append(f"{_key_repr(e.key)}: {e.value!r}") body = ", ".join(lst) return f"<{self.__class__.__name__}({body})>" @@ -300,7 +312,7 @@ def _iter(self, version: int) -> Iterator[str]: def __repr__(self) -> str: lst = [] for e in self._md._keys.iter_entries(): - lst.append(f"'{e.key}'") + lst.append(_key_repr(e.key)) body = ", ".join(lst) return f"<{self.__class__.__name__}({body})>" @@ -753,7 +765,9 @@ def __contains__(self, key: object) -> bool: @reprlib.recursive_repr() def __repr__(self) -> str: - body = ", ".join(f"'{e.key}': {e.value!r}" for e in self._keys.iter_entries()) + body = ", ".join( + f"{_key_repr(e.key)}: {e.value!r}" for e in self._keys.iter_entries() + ) return f"<{self.__class__.__name__}({body})>" if sys.implementation.name != "pypy": @@ -1207,7 +1221,7 @@ def __contains__(self, key: object) -> bool: @reprlib.recursive_repr() def __repr__(self) -> str: - body = ", ".join(f"'{k}': {v!r}" for k, v in self.items()) + body = ", ".join(f"{_key_repr(k)}: {v!r}" for k, v in self.items()) return f"<{self.__class__.__name__}({body})>" def copy(self) -> MultiDict[_V]: diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index f2ba9868a..bdef29f42 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1809,15 +1809,39 @@ md_repr(MultiDictObject *md, PyObject *name, bool show_keys, bool show_values) } } if (show_keys) { - if (PyUnicodeWriter_WriteChar(writer, '\'') < 0) { - goto fail; - } - /* Don't need to convert key to istr, the text is the same*/ - if (PyUnicodeWriter_WriteStr(writer, key) < 0) { - goto fail; + /* Fast path: ASCII keys without characters that would be escaped + * by repr() can be wrapped in single quotes directly. Falls back + * to PyUnicodeWriter_WriteRepr for keys containing quotes, + * backslashes, or non-printable characters so the output stays + * a valid Python string literal. */ + int fast = 0; + if (PyUnicode_IS_ASCII(key)) { + Py_ssize_t klen = PyUnicode_GET_LENGTH(key); + const unsigned char *kdata = + (const unsigned char *)PyUnicode_DATA(key); + fast = 1; + for (Py_ssize_t ki = 0; ki < klen; ++ki) { + unsigned char c = kdata[ki]; + if (c < 0x20 || c == 0x7f || c == '\'' || c == '\\') { + fast = 0; + break; + } + } } - if (PyUnicodeWriter_WriteChar(writer, '\'') < 0) { - goto fail; + if (fast) { + if (PyUnicodeWriter_WriteChar(writer, '\'') < 0) { + goto fail; + } + if (PyUnicodeWriter_WriteStr(writer, key) < 0) { + goto fail; + } + if (PyUnicodeWriter_WriteChar(writer, '\'') < 0) { + goto fail; + } + } else { + if (PyUnicodeWriter_WriteRepr(writer, key) < 0) { + goto fail; + } } } if (show_keys && show_values) { diff --git a/tests/test_multidict.py b/tests/test_multidict.py index d62db8c5c..136ebb5e0 100644 --- a/tests/test_multidict.py +++ b/tests/test_multidict.py @@ -635,6 +635,24 @@ def test_repr_aiohttp_issue_410(self, cls: type[MutableMultiMapping[str]]) -> No assert sys.exc_info()[1] == e # noqa: PT017 + def test__repr__quotes_keys(self, cls: type[MultiDict[str]]) -> None: + # Keys containing quotes must be repr'd as parseable Python literals, + # not naively wrapped in single quotes. + d = cls([("a'b", "v")]) + _cls = type(d) + + # repr("a'b") == '"a\'b"' (Python uses double quotes when the string + # contains a single quote and no double quote). + assert str(d) == f"<{_cls.__name__}(\"a'b\": 'v')>" + + def test_items__repr__quotes_keys(self, cls: type[MultiDict[str]]) -> None: + d = cls([("a'b", "v")]) + assert repr(d.items()) == "<_ItemsView(\"a'b\": 'v')>" + + def test_keys__repr__quotes_keys(self, cls: type[MultiDict[str]]) -> None: + d = cls([("a'b", "v")]) + assert repr(d.keys()) == '<_KeysView("a\'b")>' + @pytest.mark.parametrize( "op", (operator.or_, operator.and_, operator.sub, operator.xor),