From e37c14123247aea2cdab0fe947e541dfbce9ca14 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 13 Oct 2026 21:34:00 -0400 Subject: [PATCH] fix(corekit,utilities): unquote a StringField's bytes on Python 3.8 through compat.unquote (#1693) StringField(unquote=True) handed the raw field octets to urllib.parse.unquote, which accepts bytes only from Python 3.9; on 3.8 every unquoting read raised "TypeError: Expected str, got bytes". pcapkit.utilities.compat now exports unquote behind a sys.version_info gate: from 3.9 it is urllib.parse.unquote itself; below 3.9 it spells out what 3.9 does for bytes, unquote_to_bytes(value).decode(encoding, errors), and passes anything else to the interpreter's own unquote. StringField imports it for both call sites, including the UnicodeError fallback, and the docs' compatibility table lists it. Tests: test_compat checks the running interpreter's unquote and the pre-3.9 branch (under the faked (3, 5), once more with a bytes-rejecting unquote) against one table pinned to 3.9's results; WARM_BEFORE_FAKING gains urllib.parse. The StringField table stays, plus a check that the field unquotes through compat. --- docs/source/pcapkit/utilities/index.rst | 2 + pcapkit/corekit/fields/strings.py | 8 +- pcapkit/utilities/compat.py | 21 ++++- .../corekit/test_stringfield_lossless_unit.py | 85 +++++++++++++++++++ tests/utilities/test_compat.py | 74 ++++++++++++++-- 5 files changed, 179 insertions(+), 11 deletions(-) diff --git a/docs/source/pcapkit/utilities/index.rst b/docs/source/pcapkit/utilities/index.rst index b8d2636060..7d22240b4d 100644 --- a/docs/source/pcapkit/utilities/index.rst +++ b/docs/source/pcapkit/utilities/index.rst @@ -48,6 +48,8 @@ following objects and functions: - Python 3.9 * - :class:`dict[KT, VT] ` - Python 3.9 + * - :func:`urllib.parse.unquote` on :obj:`bytes` + - Python 3.9 * - :class:`enum.StrEnum` - Python 3.11 * - :func:`typing.final` diff --git a/pcapkit/corekit/fields/strings.py b/pcapkit/corekit/fields/strings.py index d7387f1138..b1efac3fac 100644 --- a/pcapkit/corekit/fields/strings.py +++ b/pcapkit/corekit/fields/strings.py @@ -6,7 +6,7 @@ from pcapkit.corekit.fields.field import NO_VALUE, Field from pcapkit.utilities.chardet import detect -from pcapkit.utilities.compat import Dict +from pcapkit.utilities.compat import Dict, unquote from pcapkit.utilities.exceptions import FieldValueError __all__ = [ @@ -138,7 +138,7 @@ class StringField(_TextField[str]): def __init__(self, length: 'int | Callable[[dict[str, Any]], int]', default: 'str | NoValueType' = NO_VALUE, encoding: 'Optional[str]' = None, errors: 'Literal["strict", "ignore", "replace"]' = 'strict', - unquote: 'bool' = False, + unquote: 'bool' = False, # pylint: disable=redefined-outer-name callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(length, default, callback) @@ -198,9 +198,9 @@ def post_process(self, value: 'bytes', packet: 'dict[str, Any]') -> 'str': # py """ if self._unquote: try: - ret = urllib_parse.unquote(value, encoding=self._encoding or 'utf-8', errors=self._errors) + ret = unquote(value, encoding=self._encoding or 'utf-8', errors=self._errors) except UnicodeError: - ret = urllib_parse.unquote(value.replace(b'%', rb'\x'), encoding='utf-8', errors='replace') + ret = unquote(value.replace(b'%', rb'\x'), encoding='utf-8', errors='replace') else: charset = self._encoding or detect(value) try: diff --git a/pcapkit/utilities/compat.py b/pcapkit/utilities/compat.py index 6686dbd55a..607673a14b 100644 --- a/pcapkit/utilities/compat.py +++ b/pcapkit/utilities/compat.py @@ -8,7 +8,7 @@ __all__ = [ # functions - 'final', 'localcontext', 'show_flag_values', 'default_threading_excepthook', + 'final', 'localcontext', 'show_flag_values', 'default_threading_excepthook', 'unquote', # exceptions 'ModuleNotFoundError', @@ -131,6 +131,25 @@ def __get__(self, instance: 'Optional[_S]', List: 'TypeAlias' = list Dict: 'TypeAlias' = dict +# urllib.parse.unquote accepts bytes from 3.9; before it, it raises +# ``TypeError: Expected str, got bytes`` on them (issue #1693) +if sys.version_info < (3, 9): + from urllib.parse import unquote as _unquote + from urllib.parse import unquote_to_bytes as _unquote_to_bytes + + def unquote(string: 'str | bytes', encoding: 'str' = 'utf-8', errors: 'str' = 'replace') -> 'str': + """Replace %xx escapes by their single-character equivalent. The optional + encoding and errors parameters specify how to decode percent-encoded + sequences into Unicode characters, as accepted by the bytes.decode() + method. A bytes string is unquoted to octets first and then decoded, + as from Python 3.9; anything else goes to this interpreter's own + unquote unchanged.""" + if isinstance(string, bytes): + return _unquote_to_bytes(string).decode(encoding, errors) + return _unquote(string, encoding, errors) +else: + from urllib.parse import unquote + # The ``else`` branch of each ``sys.version_info`` guard below imports a name # that only exists on the newer interpreter, so it never executes on the older # one the guard is there to support. Vermin's analysis is static and cannot see diff --git a/tests/corekit/test_stringfield_lossless_unit.py b/tests/corekit/test_stringfield_lossless_unit.py index b840299926..accf1b89cd 100644 --- a/tests/corekit/test_stringfield_lossless_unit.py +++ b/tests/corekit/test_stringfield_lossless_unit.py @@ -27,6 +27,7 @@ import pickle import tempfile import unittest +import urllib.parse from tests._support import reimport_once_per_class @@ -152,6 +153,90 @@ def test_decoded_string_compares_and_hashes_as_its_text(self) -> None: self.assertEqual({value: 1}['été'], 1) +#: ``(octets, field keyword arguments, decoded text, whether it is a DecodedString)`` +#: for ``unquote=True``, each as Python 3.9+ decodes it. A text holding a literal +#: ``\xE9`` is the ``UnicodeError`` fallback, which decodes with ``%`` turned into ``\x``. +UNQUOTED = ( + (b'', {}, '', False), + (b'a%2fb%2F', {}, 'a/b/', True), + (b'%C3%A9', {}, 'é', False), + (b'caf%E9', {'encoding': 'latin-1'}, 'café', False), + (b'a+b%20c', {}, 'a+b c', True), + (b'%zz%', {}, '%zz%', True), + (b'%E9', {'errors': 'replace'}, '�', True), + (b'%E9', {'errors': 'ignore'}, '', True), + (b'%FF%FEa%00', {'encoding': 'utf-16'}, 'a', True), + (b'%E9', {}, '\\xE9', True), + (b'\xff%41', {}, '�\\x41', True), + (b'%ED%A0%80', {}, '\\xED\\xA0\\x80', True), + (b'%C3%A9', {'encoding': 'ascii'}, '\\xC3\\xA9', True), + (b'%E9', {'encoding': 'idna'}, '\\xE9', True), +) + + +class TestStringFieldUnquote(unittest.TestCase): + """``unquote=True`` decodes alike on every Python (#1693). + + :func:`urllib.parse.unquote` accepts :obj:`bytes` only from Python 3.9, and + every value a field unpacks is :obj:`bytes`, so on 3.8 each of these raised + ``TypeError: Expected str, got bytes``. + """ + + def setUp(self) -> None: + reimport_once_per_class(self) + + def check_table(self) -> None: + from pcapkit.corekit.fields.strings import DecodedString, StringField + + for octets, kwargs, text, decoded in UNQUOTED: + with self.subTest(octets=octets, kwargs=kwargs): + field = StringField(length=len(octets), unquote=True, **kwargs) # type: ignore[arg-type] + value = field.unpack(octets, {}) + self.assertEqual(str(value), text) + self.assertIs(isinstance(value, DecodedString), decoded) + self.assertEqual(field.pack(value, {}), octets) + + def test_unquote_decodes_as_python_3_9_does(self) -> None: + self.check_table() + + def test_unquote_is_the_compat_one(self) -> None: + """The field unquotes through :func:`pcapkit.utilities.compat.unquote`. + + That is where the version gate lives, and + :mod:`tests.utilities.test_compat` runs both of its branches, the pre-3.9 + one included, on every Python. + """ + from pcapkit.corekit.fields import strings + from pcapkit.utilities import compat + + self.assertIs(strings.unquote, compat.unquote) + + def test_a_value_that_is_not_bytes_still_goes_to_urllib(self) -> None: + """Only :obj:`bytes` take the 3.9 path; the fix leaves anything else as it was. + + A field never unpacks one, but :meth:`post_process` is public: text is + unquoted, while a :obj:`bytearray` or :obj:`memoryview` raises exactly what + :func:`urllib.parse.unquote` raises on it. + """ + from pcapkit.corekit.fields.strings import DecodedString, StringField + + field = StringField(length=5, unquote=True) + value = field.post_process('a%20b', {}) # type: ignore[arg-type] + self.assertIsInstance(value, DecodedString) + self.assertEqual((str(value), value.raw), ('a b', 'a%20b')) + latin = StringField(length=6, unquote=True, encoding='latin-1') + self.assertEqual(latin.post_process('caf%E9', {}), 'café') # type: ignore[arg-type] + + for octets, error in ((bytearray(b'a%20b'), TypeError), + (memoryview(b'a%20b'), AttributeError)): + with self.subTest(type=type(octets).__name__): + with self.assertRaises(error) as expected: + urllib.parse.unquote(octets, 'utf-8', 'strict') # type: ignore[arg-type] + with self.assertRaises(error) as raised: + field.post_process(octets, {}) # type: ignore[arg-type] + self.assertEqual(str(raised.exception), str(expected.exception)) + + class TestStringFieldDumpers(unittest.TestCase): """The dumpers render an undecodable octet rather than failing on it.""" diff --git a/tests/utilities/test_compat.py b/tests/utilities/test_compat.py index c126410cbd..4281711e5d 100644 --- a/tests/utilities/test_compat.py +++ b/tests/utilities/test_compat.py @@ -58,6 +58,8 @@ import sys import textwrap import unittest +import urllib.parse +from typing import Callable from unittest import mock from tests._support import load_module, purge_modules, reimport_once_per_class @@ -110,10 +112,10 @@ #: running the test until it stopped complaining, and that distinction is the #: point of having it. Every import statement on a branch reachable at ``(3, 5)``: #: :mod:`collections.abc` (line 34), :mod:`pathlib` (63), :mod:`threading` (67), -#: :mod:`typing` (4, 68, 126), :mod:`aenum` (154), :mod:`typing_extensions` (172, -#: 226), :mod:`contextlib` (177), :mod:`decimal` (178), :mod:`enum` (203). A fake -#: ``(3, 9)`` reaches three more: :mod:`builtins` (31), :mod:`functools` (123) and -#: :mod:`_thread` (251). +#: :mod:`typing` (4, 68, 126), :mod:`urllib.parse` (137, 138), :mod:`aenum` (173), +#: :mod:`typing_extensions` (191, 245), :mod:`contextlib` (196), :mod:`decimal` +#: (197), :mod:`enum` (222). A fake ``(3, 9)`` reaches three more: :mod:`builtins` +#: (31), :mod:`functools` (123) and :mod:`_thread` (270). #: #: Four of those never actually surface, and *why* is the point. Three -- #: :mod:`collections.abc`, :mod:`contextlib` and :mod:`pathlib` -- are imported at @@ -125,10 +127,12 @@ #: and nothing else. Measured: with only the five that do surface warmed and #: :mod:`pathlib` forced cold, the window leaks ``['pathlib', 'pathlib._os']``; #: with the full list, then nine names, it is empty even with five of them forced -#: cold. +#: cold. :mod:`urllib.parse`, added since for issue #1693, is warm before this +#: module runs on 3.8 to 3.10, where :mod:`pathlib` imports it, and cold on 3.14 +#: (both under ``python -S``). WARM_BEFORE_FAKING = ('_thread', 'aenum', 'builtins', 'collections.abc', 'contextlib', 'decimal', 'enum', 'functools', 'pathlib', 'threading', 'typing', - 'typing_extensions') + 'typing_extensions', 'urllib.parse') #: Prefix of the names the loaders themselves write, which are excluded from the #: "nothing was imported under the fake" check. :func:`tests._support.load_module` @@ -149,6 +153,31 @@ #: assertion is aimed there. OWN_MODULE_PREFIX = 'pcapkit' +#: ``(value, encoding, errors, outcome)`` for ``compat.unquote``, each as Python +#: 3.9's :func:`urllib.parse.unquote` has it: the text returned, or the exception +#: type and message raised. ``None`` keywords leave the defaults in place. Before +#: 3.9 every :obj:`bytes` row raised ``TypeError: Expected str, got bytes`` (#1693). +UNQUOTED = ( + (b'', 'utf-8', 'strict', ''), + (b'a%2fb%2F', 'utf-8', 'strict', 'a/b/'), + (b'a+b%20c', 'utf-8', 'strict', 'a+b c'), + (b'%zz%', 'utf-8', 'strict', '%zz%'), + (b'caf%E9', 'latin-1', 'strict', 'café'), + (b'%FF%FEa%00', 'utf-16', 'strict', 'a'), + (b'%E9', 'utf-8', 'ignore', ''), + (b'%E9', None, None, '\ufffd'), + (b'caf%C3%A9', None, None, 'café'), + (b'%E9', 'utf-8', 'strict', + (UnicodeDecodeError, "'utf-8' codec can't decode byte 0xe9 in position 0: unexpected end of data")), + (b'\xff%41', 'utf-8', 'strict', + (UnicodeDecodeError, "'utf-8' codec can't decode byte 0xff in position 0: invalid start byte")), + ('a%20b', 'utf-8', 'strict', 'a b'), + ('caf%E9', 'latin-1', 'strict', 'café'), + ('%E9', None, None, '\ufffd'), + (bytearray(b'a%20b'), 'utf-8', 'strict', (TypeError, "a bytes-like object is required, not 'str'")), + (memoryview(b'a%20b'), 'utf-8', 'strict', (AttributeError, "'memoryview' object has no attribute 'split'")), +) + def aenum_pyver_readings() -> 'dict[str, tuple[int, ...]]': """``pyver`` as every imported :mod:`aenum` module currently sees it. @@ -296,6 +325,26 @@ def test_localcontext_applies_keyword_overrides(self) -> None: self.assertEqual(decimal.getcontext().prec, 7) self.assertEqual(decimal.getcontext().prec, original_prec) + def assert_unquotes_as_python39(self, unquote: 'Callable[..., str]') -> None: + """``unquote`` gives each :data:`UNQUOTED` row its outcome.""" + for value, encoding, errors, outcome in UNQUOTED: + kwargs = {} if encoding is None else {'encoding': encoding, 'errors': errors} + with self.subTest(value=value, **kwargs): + if isinstance(outcome, str): + self.assertEqual(unquote(value, **kwargs), outcome) + else: + with self.assertRaises(outcome[0]) as raised: + unquote(value, **kwargs) + self.assertEqual(str(raised.exception), outcome[1]) + + def test_unquote_decodes_bytes_as_python39_does(self) -> None: + """From 3.9 the name is :func:`urllib.parse.unquote` itself (#1693).""" + if sys.version_info >= (3, 9): + self.assertIs(self.compat.unquote, urllib.parse.unquote) + else: + self.assertIsNot(self.compat.unquote, urllib.parse.unquote) + self.assert_unquotes_as_python39(self.compat.unquote) + def test_show_flag_values_returns_individual_bits(self) -> None: class DemoFlag(enum.IntFlag): READ = 1 @@ -454,6 +503,19 @@ class SlotDemo: self.assertEqual(ctx.prec, 9) self.assertEqual(decimal.getcontext().prec, original_prec) + # The table again with the bytes rows kept from an unquote that rejects + # them, as every interpreter's did before 3.9 (#1693). + self.assertIsNot(compat.unquote, urllib.parse.unquote) + self.assert_unquotes_as_python39(compat.unquote) + + def unquote_before_3_9(string: 'str', *args: 'str') -> 'str': + if isinstance(string, bytes): + raise TypeError('Expected str, got bytes') + return urllib.parse.unquote(string, *args) + + with mock.patch.object(compat, '_unquote', unquote_before_3_9): + self.assert_unquotes_as_python39(compat.unquote) + class OldFlag(enum.IntFlag): READ = 1 EXEC = 4