fix(corekit,utilities): unquote a StringField's bytes on Python 3.8 through compat.unquote (#1693)

docs/source/pcapkit/utilities/index.rst

@@ -48,6 +48,8 @@ following objects and functions:
      - Python 3.9
    * - :class:`dict[KT, VT] <dict>`
      - Python 3.9
+   * - :func:`urllib.parse.unquote` on :obj:`bytes`
+     - Python 3.9
    * - :class:`enum.StrEnum`
      - Python 3.11
    * - :func:`typing.final`

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:

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

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."""
 

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