diff --git a/Pipfile b/Pipfile index 0db42c4fef..34684c964a 100644 --- a/Pipfile +++ b/Pipfile @@ -6,7 +6,7 @@ verify_ssl = true [packages] aenum = "*" chardet = "*" -dictdumper = "~=0.8.4" +dictdumper = ">=0.8.5" emoji = "*" tbtrim = "~=0.3.1" diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py index b27e4ccdcb..4e9e6b33b4 100644 --- a/pcapkit/dumpkit/common.py +++ b/pcapkit/dumpkit/common.py @@ -20,7 +20,6 @@ import re import string import types -import xml.sax.saxutils # nosec: B406 # escape() only, nothing parsed from typing import TYPE_CHECKING import aenum @@ -301,6 +300,40 @@ def _object_name(value: 'Any') -> 'str': return qualname +def _surrogate_pairs(text: 'str') -> 'str': + """Split every character above ``U+FFFF`` in ``text`` into its UTF-16 surrogate pair. + + Args: + text: Text to split. + + Returns: + ``text`` with each astral character (above ``U+FFFF``) replaced by the + two lone-surrogate code points :func:`json.dumps` would write it as; + every other character is returned unchanged. + + Note: + This is for :mod:`dictdumper.json`'s *key* path, which escapes a + mapping key through its module-level :func:`~dictdumper.json.encode_string` + directly rather than through an overridable method (GitHub issue + :issue:`1261`). That function already escapes a lone surrogate as a + four-digit ``\\uXXXX`` sequence -- it is only a *pair* of code points + above ``U+FFFF`` it cannot spell -- so splitting first and handing it + the pair is enough; no change to :func:`~dictdumper.json.encode_string` + itself is needed. + + """ + chars = [] # type: list[str] + for char in text: + codepoint = ord(char) + if codepoint > 0xFFFF: + codepoint -= 0x10000 + chars.append(chr(0xD800 + (codepoint >> 10))) + chars.append(chr(0xDC00 + (codepoint & 0x3FF))) + else: + chars.append(char) + return ''.join(chars) + + def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]': """Create a customised :class:`~dictdumper.dumper.Dumper` class. @@ -328,89 +361,49 @@ def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]': # NOTE: :class:`~dictdumper.plist.PLIST` -- which is also what # :attr:`Extractor.__output__ ` # maps the ``'xml'`` format to, so the two are the same writer under two - # names here -- interpolates every ````/```` value into its - # XML-shaped markup with no entity escaping at all: neither ``&``, ``<`` - # nor ``>`` is escaped anywhere in :mod:`dictdumper`. Filed upstream as - # JarryShaw/DictDumper#125 and tracked here as GitHub issue #772. ``json``, - # ``tree`` and ``text`` have no such defect and all three characters are - # legal in their output, so escaping is conditioned on the output class - # rather than done unconditionally in :func:`render_enum`, which would - # double-escape those three. :class:`~dictdumper.xml.XML` itself defines - # no ``_append_string`` of its own -- its own module docstring says not to - # use it directly -- so :class:`~dictdumper.plist.PLIST` is the only - # concrete writer this applies to. - escape_strings = issubclass(output, dictdumper.plist.PLIST) - # NOTE: :class:`~dictdumper.json.JSON` escapes every string *value* it - # writes, but interpolates a mapping key into ``'"{item}": '`` raw: a ``"`` - # in a key closes the JSON string early, and a ``\`` escapes the character - # after it (GitHub issue #1152). The defect is :mod:`dictdumper`'s, so it is - # worked around here. - escape_json_keys = issubclass(output, dictdumper.json.JSON) - escape_keys = escape_strings or escape_json_keys + # names here -- used to interpolate every ````/```` value into + # its XML-shaped markup with no entity escaping at all: neither ``&``, + # ``<`` nor ``>`` was escaped anywhere in :mod:`dictdumper`. Filed upstream + # as JarryShaw/DictDumper#125 and tracked here as GitHub issue #772 -- along + # with the sibling ``json`` key-quoting defect, DictDumper#121 -- and both + # halves are now fixed upstream in ``dictdumper`` 0.8.5 (DictDumper#123): + # :func:`dictdumper.plist.encode_xml` and :func:`dictdumper.json.encode_string` + # each escape a mapping key exactly as they escape the value beside it, so + # the escaping this module used to do for both is removed rather than + # doubled up on top of the upstream fix. + # + # ``is_plist_output`` survives under its own name for two narrower, + # still-unfixed reasons: a string holding a character XML 1.0 cannot spell + # at all (GitHub issue #1262, below) still has to be written as + # :class:`bytes` instead, and an aware :class:`~datetime.datetime` still has + # to be converted to UTC before the (now upstream-fixed) ```` + # formatting runs, which neither :mod:`dictdumper` nor + # :func:`plistlib.dump` does on its own (GitHub issue #1448, below). + is_plist_output = issubclass(output, dictdumper.plist.PLIST) + # NOTE: :meth:`~dictdumper.json.JSON._append_string` and the module-level + # :func:`dictdumper.json.encode_string` it shares with + # :meth:`~dictdumper.json.JSON._append_object` both escape a character + # above ``U+FFFF`` as a single five- or six-digit ``\uXXXX`` escape, which a + # reader takes as one four-digit escape followed by literal text (GitHub + # issue #1261). Unlike the key-escaping defect above, ``dictdumper`` 0.8.5 + # left this one alone -- confirmed by reading the installed package's + # :func:`dictdumper.json.encode_string` -- so the surrogate-pair workarounds + # on :meth:`DictDumper._append_string` (values) and + # :meth:`DictDumper._append_object` (keys) below still apply. + is_json_output = issubclass(output, dictdumper.json.JSON) # NOTE: :meth:`~dictdumper.plist.PLIST._append_dict` and # :meth:`~dictdumper.plist.PLIST._append_array` skip every :data:`None` # value and item outright, so a ``plist`` report lacks keys the ``json`` - # report has (GitHub issue #1257). The defect is :mod:`dictdumper`'s, so it - # is worked around here. + # report has (GitHub issue #1257). The defect is :mod:`dictdumper`'s, and + # ``dictdumper`` 0.8.5 did not touch it, so it is still worked around here. keep_none = issubclass(output, dictdumper.plist.PLIST) - # NOTE: :meth:`~dictdumper.plist.PLIST._append_date` writes every date with - # six fractional digits, which the property list ```` grammar does not - # admit, so :func:`plistlib.load` rejects every report (GitHub issue #1448). - # The defect is :mod:`dictdumper`'s, so it is worked around here. - plist_dates = issubclass(output, dictdumper.plist.PLIST) # NOTE: :meth:`~dictdumper.tree.Tree._append_number` hands every number to # :func:`math.isnan`, which raises :exc:`TypeError` for a :class:`complex` # one, although :class:`~dictdumper.tree.Tree` routes :class:`complex` there - # (GitHub issue #1266). The defect is :mod:`dictdumper`'s, so it is worked - # around here. + # (GitHub issue #1266). The defect is :mod:`dictdumper`'s, and ``dictdumper`` + # 0.8.5 did not touch it, so it is still worked around here. tree_numbers = issubclass(output, dictdumper.tree.Tree) - def escape_key(key: 'Any') -> 'Any': - """Escape a mapping key on its way to the writer. - - Args: - key: Mapping key, as the writer would interpolate it. - - Returns: - The key unchanged where ``output`` needs no escaping, otherwise the - escaped text of the writer's own rendering of it. - - Note: - :meth:`~dictdumper.plist.PLIST._append_dict` writes a key straight - into ``'{item}'``, and - :meth:`~dictdumper.json.JSON._append_object` into ``'"{item}": '``; - both call :meth:`~dictdumper.dumper.Dumper._encode_value` on the - *value* that follows, never on the key -- so - :meth:`DictDumper.object_hook` is handed every value the writer will - interpolate but no key at all, and cannot escape one on the way out - the way it does a value. Each branch below that builds a mapping - therefore escapes its own keys through here. - - A non-:class:`str` key is rendered with :func:`format`, which is the - very conversion ``'{item}'.format(item=key)`` already applies to it, - so the key text is unchanged apart from the escaping. - Rendering such a key rather than passing it over is deliberate: - :file:`examples/captures/test.pcapng` keys the TLS key log entries - of its decryption secrets block by a raw :class:`bytes` client - random (:meth:`TLSKeyLog.post_process - `), and - the ``bytes`` repr of that one carries ``&``, ``<``, ``>``, ``"`` - *and* ``\\``. Unescaped, that makes the fixture's ``plist`` report - unparseable at the key's ``&``, and its ``json`` report at the - key's ``"``. - - For ``json`` the key is escaped by :func:`json.dumps` with the - surrounding quotes stripped, since the writer supplies its own. Like - :meth:`~dictdumper.json.JSON._append_string`, that spells every - non-ASCII character as a ``\\uXXXX`` escape. - - """ - if escape_strings: - return xml.sax.saxutils.escape(format(key, '')) - if escape_json_keys: - return json.dumps(format(key, ''))[1:-1] - return key - def excluded(info: 'Info | MultiDict[Any, Any]') -> 'Collection[Any]': """Return the keys of ``info`` the writer leaves out. @@ -440,13 +433,14 @@ def fields(info: 'Info | MultiDict[Any, Any]') -> 'dict[Any, Any]': :meth:`~pcapkit.corekit.infoclass.Info.to_dict` returns. Returns: - One entry per field, in order, with its key escaped as - :func:`escape_key` does. A field held once maps to its value; a field - held more than once, such as a repeated IPv6 extension header, maps - to the :class:`list` of all its values, in order (GitHub issue - :issue:`1484`). The values are left as they are, so each comes back - through :meth:`DictDumper.object_hook` when written. The keys - :func:`excluded` names are left out. + One entry per field, in order, with its key unchanged -- the writer + itself now escapes a mapping key where ``output`` needs that + (``dictdumper`` 0.8.5, DictDumper#123). A field held once maps to + its value; a field held more than once, such as a repeated IPv6 + extension header, maps to the :class:`list` of all its values, in + order (GitHub issue :issue:`1484`). The values are left as they + are, so each comes back through :meth:`DictDumper.object_hook` when + written. The keys :func:`excluded` names are left out. """ skip = excluded(info) @@ -455,12 +449,12 @@ def fields(info: 'Info | MultiDict[Any, Any]') -> 'dict[Any, Any]': result = {} # type: dict[Any, Any] for key, val in pairs: if counts[key] > 1: - result.setdefault(escape_key(key), []).append(val) + result.setdefault(key, []).append(val) else: # NOTE: read by indexing, so that a model overriding it is # honoured -- the traceflow and reassembly models resolve a # deferred ``packet`` there. - result[escape_key(key)] = info[key] + result[key] = info[key] return result #: Whether the writer is handed the fields of an @@ -513,26 +507,22 @@ def object_hook(self, o: 'Any') -> 'Any': o: object to convert Returns: - Converted object, escaped for XML-shaped output where needed. + Converted object. Escaping a string or a mapping key for + XML-shaped output is the writer's own job since ``dictdumper`` + 0.8.5 (DictDumper#123); this method no longer does it. Notes: :meth:`~dictdumper.dumper.Dumper._encode_value` -- and so this method -- is called once per node the dumper writes, however - deeply nested, so escaping the :class:`str` result here on the - way out reaches every string the writer will ever interpolate - raw, not merely the ones built directly in this method. The - one exception is a mapping *key*: - :meth:`~dictdumper.plist.PLIST._append_dict` and - :meth:`~dictdumper.json.JSON._append_object` write those - straight from the mapping without ever calling this method on - them, so both branches that hand the writer a mapping -- a - :class:`~pcapkit.corekit.multidict.MultiDict` and a plain - :class:`dict` -- escape their own keys through - :func:`escape_key` instead. An - :class:`~pcapkit.corekit.multidict.OrderedMultiDict` becomes a - :class:`list` of single-key :class:`dict` objects, each of - which comes back through the :class:`dict` branch, and so - does an :class:`~pcapkit.corekit.infoclass.OrderedMultiInfo`, + deeply nested. A mapping *key* never reaches this method at + all: :meth:`~dictdumper.plist.PLIST._append_dict` and + :meth:`~dictdumper.json.JSON._append_object` write a key + straight from the mapping, and now escape it themselves where + ``output`` needs that, exactly as they escape the value beside + it. An :class:`~pcapkit.corekit.multidict.OrderedMultiDict` + becomes a :class:`list` of single-key :class:`dict` objects, + each of which comes back through the :class:`dict` branch, and + so does an :class:`~pcapkit.corekit.infoclass.OrderedMultiInfo`, the option lists an :class:`~pcapkit.corekit.infoclass.Info` holds. An :class:`~pcapkit.corekit.infoclass.Info` itself becomes the :class:`dict` of its fields that :func:`fields` @@ -562,11 +552,11 @@ def object_hook(self, o: 'Any') -> 'Any': # NOTE: one single-key mapping per entry, in insertion order -- # i.e. wire order for the option lists -- since a mapping keyed # by name could only gather the repeats of a key under its first - # occurrence (GitHub issue #1263). The keys are escaped when the - # writer hands each mapping back to this hook, through the - # :class:`dict` branch below. The keys an export of an - # :class:`Info` excludes are left out, as :func:`fields` leaves - # them out (#2003). + # occurrence (GitHub issue #1263). Each single-key mapping comes + # back through the :class:`dict` branch below, which is where the + # writer itself now escapes the key, where ``output`` needs that. + # The keys an export of an :class:`Info` excludes are left out, + # as :func:`fields` leaves them out (#2003). skip = excluded(o) result = [ {render_enum(key) if isinstance(key, (enum.Enum, aenum.Enum)) else key: val} @@ -579,16 +569,14 @@ def object_hook(self, o: 'Any') -> 'Any': for key, val in o.items(multi=True): if isinstance(key, (enum.Enum, aenum.Enum)): key = render_enum(key) - temp[escape_key(key)].append(val) + temp[key].append(val) result = temp elif isinstance(o, dict): - # NOTE: rebuilt only where the keys need escaping, so every other - # output is still handed the caller's own mapping rather than a - # copy of it. - if escape_keys: - result = {escape_key(key): val for key, val in o.items()} - else: - result = o + # NOTE: never rebuilt -- the writer now escapes a key itself + # where ``output`` needs that (``dictdumper`` 0.8.5, + # DictDumper#123), so every output is handed the caller's own + # mapping, with the caller's own key objects in it. + result = o elif isinstance(o, (enum.Enum, aenum.Enum)): addon = {key: val for key, val in o.__dict__.items() if not key.startswith('_')} if addon: @@ -601,16 +589,47 @@ def object_hook(self, o: 'Any') -> 'Any': else: result = super().object_hook(o) - if escape_strings and isinstance(result, str): - # NOTE: XML 1.0 has no spelling at all for some characters, not - # even a character reference, so a string holding one is written - # as ````, the way :class:`bytes` already is (GitHub issue - # #1262). - if _XML_ILLEGAL.search(result) is not None: - return result.encode('utf-8', 'surrogatepass') - return xml.sax.saxutils.escape(result) + # NOTE: XML 1.0 has no spelling at all for some characters, not + # even a character reference, so a PLIST string holding one is + # written as ````, the way :class:`bytes` already is (GitHub + # issue #1262). Any escaping the string itself still needs -- + # ``&``, ``<`` and ``>`` -- is now the writer's own job. + if is_plist_output and isinstance(result, str) and _XML_ILLEGAL.search(result) is not None: + return result.encode('utf-8', 'surrogatepass') return result + if is_plist_output: + def _append_date(self, value: 'datetime.date', file: 'TextIO') -> 'None': + """Call this function to write date contents. + + Args: + self: Dumper instance. + value: Content to be dumped. + file: Output file. + + Notes: + An aware value is converted to UTC first, a naive one is + taken as UTC already -- neither :mod:`dictdumper` nor + :func:`plistlib.dump` (without its 3.13+ ``aware_datetime`` + flag) does this conversion on its own, so it stays pcapkit's + own addition on top of the upstream writer. The exact value + is not lost: every timestamp pcapkit reports sits next to a + field that carries it exactly -- a :class:`~decimal.Decimal` + ``*_epoch`` for frames and PCAP-NG blocks and options, e.g. + ``time_epoch``, and the raw ``ntp_timestamp`` or + ``pmip_timestamp`` for the MH options that carry one. + + Formatting to whole seconds -- the property list ```` + grammar has no fractional part (GitHub issue :issue:`1448`) -- is now + :meth:`~dictdumper.plist.PLIST._append_date`'s own job, fixed + upstream in ``dictdumper`` 0.8.5 (DictDumper#124), so it is + left to :meth:`super`. + + """ + if isinstance(value, datetime.datetime) and value.tzinfo is not None: + value = value.astimezone(datetime.timezone.utc) + super()._append_date(value, file) + if keep_none: def _append_dict(self, value: 'dict[Any, Any]', file: 'TextIO') -> 'None': """Call this function to write dict contents. @@ -645,9 +664,9 @@ def _append_array(self, value: 'list[Any]', file: 'TextIO') -> 'None': value = [self._encode_value(item) if item is None else item for item in value] super()._append_array(value, file) - if plist_dates: - def _append_date(self, value: 'datetime.date', file: 'TextIO') -> 'None': - """Call this function to write date contents. + if is_json_output: + def _append_object(self, value: 'dict[Any, Any]', file: 'TextIO') -> 'None': + """Call this function to write object (mapping) contents. Args: self: Dumper instance. @@ -655,32 +674,29 @@ def _append_date(self, value: 'datetime.date', file: 'TextIO') -> 'None': file: Output file. Notes: - The date is written in UTC to whole seconds, as - :func:`plistlib.dump` writes one: an aware value is - converted to UTC first, a naive one is taken as UTC already, - and the sub-second part of the :class:`~datetime.datetime` - is truncated. That value is already rounded to the - microsecond where it is parsed, so within half a microsecond - of a second boundary the date is one second past the floor - of a finer ``*_epoch``. The exact value is not - lost: every timestamp pcapkit reports sits next to a field - that carries it exactly -- a :class:`~decimal.Decimal` - ``*_epoch`` for frames and PCAP-NG blocks and options, e.g. - ``time_epoch``, and the raw ``ntp_timestamp`` or - ``pmip_timestamp`` for the MH options that carry one. + :meth:`dictdumper.json.JSON._append_object` escapes each key + through the module-level :func:`dictdumper.json.encode_string` + directly, not through :meth:`_append_string`, so the + surrogate-pair handling there never reaches a key -- a + :class:`str` key holding a character above ``U+FFFF`` is + still written as the single five- or six-digit escape + :func:`~dictdumper.json.encode_string` always wrote (GitHub + issue :issue:`1261`). Splitting such a key into its UTF-16 + surrogate pair first, with :func:`_surrogate_pairs`, makes + :func:`~dictdumper.json.encode_string` emit the same pair of + four-digit escapes it already writes for a lone surrogate, + matching what :func:`json.dumps` writes and what + :meth:`_append_string` above already does for a value. """ - if isinstance(value, datetime.datetime) and value.tzinfo is not None: - value = value.astimezone(datetime.timezone.utc) - # NOTE: not :meth:`~datetime.date.strftime`, whose ``%Y`` does not - # zero-pad a year below 1000 on every platform. - hour, minute, second = (getattr(value, name, 0) - for name in ('hour', 'minute', 'second')) - tabs = '\t' * self._tctr - file.write(f'{tabs}{value.year:04d}-{value.month:02d}-{value.day:02d}' - f'T{hour:02d}:{minute:02d}:{second:02d}Z\n') - - if escape_json_keys: + if any(isinstance(key, str) and any(ord(char) > 0xFFFF for char in key) + for key in value): + value = { + _surrogate_pairs(key) if isinstance(key, str) else key: val + for key, val in value.items() + } + super()._append_object(value, file) + def _append_string(self, value: 'str', file: 'TextIO') -> 'None': """Call this function to write string contents. diff --git a/pyproject.toml b/pyproject.toml index 89d4a4740a..b8f76cb0d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,7 +74,7 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "dictdumper~=0.8.0", # for formatted output + "dictdumper>=0.8.5", # for formatted output -- the floor fixing #772/#1448 upstream "chardet", # for bytes decode "aenum", # for const types "tbtrim>=0.2.1", # for refined exceptions diff --git a/tests/dumpkit/test_common_unit.py b/tests/dumpkit/test_common_unit.py index d3109140fb..53696b05c9 100644 --- a/tests/dumpkit/test_common_unit.py +++ b/tests/dumpkit/test_common_unit.py @@ -77,7 +77,6 @@ def __init__(self) -> None: #: because neither is special inside an XML text node. BYTES_KEY = bytes(range(0x20, 0x40)) RAW_BYTES_KEY = """b' !"#$%&\\'()*+,-./0123456789:;<=>?'""" -ESCAPED_BYTES_KEY = """b' !"#$%&\\'()*+,-./0123456789:;<=>?'""" def plist_like_dumper() -> 'object': @@ -226,123 +225,85 @@ def test_make_dumper_object_hook_conversions_and_fallbacks(self) -> None: dumper._append_fallback(123, file) self.assertEqual(file.getvalue(), "{'base': '123'}") - def test_make_dumper_escapes_strings_only_for_plist_like_output(self) -> None: - """GitHub issue #772: no entity escaping anywhere in :mod:`dictdumper`. + def test_make_dumper_object_hook_leaves_strings_and_enum_renderings_unescaped(self) -> None: + """GitHub issue #772: escaping is now the writer's own job, not this hook's. ``dictdumper.plist.PLIST`` (which is also what ``'xml'`` maps to -- - see ``Extractor.__output__``) interpolates every ````/```` - value straight into its markup, so a value containing ``&``, ``<`` or - ``>`` breaks the document it is embedded in. Filed upstream as - JarryShaw/DictDumper#125. ``json``/``tree``/``text`` have no such - defect and all three characters are legal there, so the escaping in - :func:`~pcapkit.dumpkit.common.make_dumper` must fire for a - PLIST-rooted ``output`` and stay off for anything else -- a plain - :class:`str`, an enum rendered by :func:`~pcapkit.dumpkit.common.render_enum`, - and a :class:`~pcapkit.corekit.multidict.MultiDict` key built from one, - since that key is written by the PLIST writer directly and never - passed back through this hook the way a value is. + see ``Extractor.__output__``) used to interpolate every + ````/```` value straight into its markup with no entity + escaping at all, so a value containing ``&``, ``<`` or ``>`` broke the + document it was embedded in. Filed upstream as JarryShaw/DictDumper#125 + and fixed there in ``dictdumper`` 0.8.5 (DictDumper#123): + :func:`dictdumper.plist.encode_xml` now escapes a value itself. So + :meth:`~pcapkit.dumpkit.common.make_dumper.DictDumper.object_hook` no + longer escapes anything on the way out, for *any* ``output`` -- a plain + :class:`str`, an enum rendered by + :func:`~pcapkit.dumpkit.common.render_enum`, or a + :class:`~pcapkit.corekit.multidict.MultiDict` key built from one, since + that key is written by the writer directly and never passed back + through this hook the way a value is. """ from pcapkit.corekit.multidict import MultiDict from pcapkit.dumpkit.common import make_dumper - plist_dumper = plist_like_dumper() - self.assertEqual(plist_dumper.object_hook('a & b '), 'a & b <c>') - unknown = enum.IntEnum('', {'': 1}) member = getattr(unknown, '') - self.assertEqual(plist_dumper.object_hook(member), - '<unknown>::<unassigned> [1]') - multidict = MultiDict() - multidict.add(member, 'value') - converted = plist_dumper.object_hook(multidict) - self.assertEqual(converted['<unknown>::<unassigned> [1]'], ['value']) + for name, dumper in (('plist-like', plist_like_dumper()), ('identity', make_dumper(IdentityDumper)())): + with self.subTest(dumper=name): + self.assertEqual(dumper.object_hook('a & b '), 'a & b ') + self.assertEqual(dumper.object_hook(member), ':: [1]') - plain_dumper = make_dumper(IdentityDumper)() - self.assertEqual(plain_dumper.object_hook('a & b '), 'a & b ') - self.assertEqual(plain_dumper.object_hook(member), ':: [1]') + multidict = MultiDict() + multidict.add(member, 'value') + converted = dumper.object_hook(multidict) + self.assertEqual(converted[':: [1]'], ['value']) - def test_make_dumper_escapes_mapping_keys_for_plist_like_output(self) -> None: + def test_make_dumper_never_rebuilds_or_escapes_a_mapping(self) -> None: """GitHub issue #772, the half the hook above cannot reach. ``PLIST._append_dict`` in :mod:`dictdumper.plist` writes a key as - ``'{item}'.format(item=item)`` and then calls ``_encode_value`` - on the *value*, never on the key -- so :meth:`object_hook` - is handed every value the writer interpolates and no key at all, and the - escaping proven above cannot reach one. Both branches that build a - mapping therefore escape their own keys: - :class:`~pcapkit.corekit.multidict.MultiDict`, where without the fix - only an enum-derived key is escaped, and a plain :class:`dict`, where - nothing is. - - The :class:`bytes` key is :file:`examples/captures/test.pcapng`'s own, - and it is the reason that fixture's ``plist`` report did not parse: - ``&``, ``<`` and ``>`` all inside one key. It is rendered with - :func:`format`, i.e. the conversion the writer's interpolation already - applies to it, so the ```` text is unchanged apart from the - escaping -- which is what the :data:`RAW_BYTES_KEY` assertion pins, - since an expected escaping is only as good as the rendering it is - derived from. + ``'{item}'.format(item=item)`` and + ``JSON._append_object`` into ``'"{item}": '``, then call + ``_encode_value`` on the *value*, never on the key -- so + :meth:`object_hook` is handed every value the writer interpolates and + no key at all. Since ``dictdumper`` 0.8.5 (DictDumper#123) both writers + escape a key themselves, exactly as they escape the value beside it, so + neither a plain :class:`dict` nor the single-key mappings a + :class:`~pcapkit.corekit.multidict.MultiDict` or + :class:`~pcapkit.corekit.multidict.OrderedMultiDict` becomes is rebuilt + here any more: the writer is handed the caller's own mapping, with the + caller's own key objects in it, exactly as given -- for *any* + ``output``, not only a non-PLIST one. + + The :class:`bytes` key is :file:`examples/captures/test.pcapng`'s own + -- it used to be the reason that fixture's ``plist`` report did not + parse, since its repr carries ``&``, ``<`` and ``>``. """ from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict + from pcapkit.dumpkit.common import make_dumper - plist_dumper = plist_like_dumper() - - # A plain dict: str keys, and the bytes key of the fixture. - self.assertEqual(format(BYTES_KEY, ''), RAW_BYTES_KEY) - converted = plist_dumper.object_hook({'a & b ': 'value', BYTES_KEY: 'secret'}) - self.assertEqual(list(converted), ['a & b <c>', ESCAPED_BYTES_KEY]) - self.assertEqual(converted['a & b <c>'], 'value') - self.assertEqual(converted[ESCAPED_BYTES_KEY], 'secret') - - # An OrderedMultiDict: the bytes key of the fixture arrives through here - # -- its TLS key log entries are an OrderedMultiDict. Each entry becomes - # a single-key mapping, whose key is escaped when the writer hands that - # mapping back to the hook. - multidict = OrderedMultiDict() - multidict.add('a & b ', 'value') - multidict.add(BYTES_KEY, 'secret') - converted = [plist_dumper.object_hook(entry) for entry in plist_dumper.object_hook(multidict)] - self.assertEqual(converted, [{'a & b <c>': 'value'}, {ESCAPED_BYTES_KEY: 'secret'}]) - - # An enum-derived key is escaped exactly once: this branch renders it - # with ``render_enum`` and escapes the result through the one shared - # ``escape_key`` helper, so escaping it twice is the mistake to rule out. unknown = enum.IntEnum('', {'': 1}) member = getattr(unknown, '') - multidict = MultiDict() - multidict.add(member, 'value') - converted = plist_dumper.object_hook(multidict) - self.assertEqual(list(converted), ['<unknown>::<unassigned> [1]']) - self.assertNotIn('&', ''.join(converted)) - - def test_make_dumper_hands_other_output_the_mapping_it_was_given(self) -> None: - """``json``, ``tree`` and ``text`` take all three characters literally. - - So no key is escaped for them, and -- the stronger statement, and the - one that keeps this branch free of any risk of double-escaping -- a - plain :class:`dict` is not even rebuilt: the writer is handed the - caller's own mapping, with the caller's own key objects in it, exactly - as given. A :class:`bytes` key stays :class:`bytes` there, - which is what ``dictdumper``'s ``json`` writer then breaks on, for - reasons of its own. - - """ - from pcapkit.corekit.multidict import OrderedMultiDict - from pcapkit.dumpkit.common import make_dumper - - plain_dumper = make_dumper(IdentityDumper)() - - mapping = {'a & b ': 'value', BYTES_KEY: 'secret'} - self.assertIs(plain_dumper.object_hook(mapping), mapping) - multidict = OrderedMultiDict() - multidict.add('a & b ', 'value') - multidict.add(BYTES_KEY, 'secret') - converted = plain_dumper.object_hook(multidict) - self.assertEqual(converted, [{'a & b ': 'value'}, {BYTES_KEY: 'secret'}]) + for name, dumper in (('plist-like', plist_like_dumper()), ('identity', make_dumper(IdentityDumper)())): + with self.subTest(dumper=name): + mapping = {'a & b ': 'value', BYTES_KEY: 'secret'} + self.assertIs(dumper.object_hook(mapping), mapping) + self.assertEqual(format(BYTES_KEY, ''), RAW_BYTES_KEY) + + multidict = OrderedMultiDict() + multidict.add('a & b ', 'value') + multidict.add(BYTES_KEY, 'secret') + converted = dumper.object_hook(multidict) + self.assertEqual(converted, [{'a & b ': 'value'}, {BYTES_KEY: 'secret'}]) + + key_multidict = MultiDict() + key_multidict.add(member, 'value') + converted_keys = dumper.object_hook(key_multidict) + self.assertEqual(list(converted_keys), [':: [1]']) def test_an_unassigned_port_dumps_its_addon_keys_in_the_declared_order(self) -> None: """GitHub issue #575's fallback must not reorder what it renders. diff --git a/tests/dumpkit/test_dumper_roundtrip_unit.py b/tests/dumpkit/test_dumper_roundtrip_unit.py index d7b814fd91..9557f0b8d9 100644 --- a/tests/dumpkit/test_dumper_roundtrip_unit.py +++ b/tests/dumpkit/test_dumper_roundtrip_unit.py @@ -173,6 +173,19 @@ def test_json_writes_astral_characters_as_surrogate_pairs(self) -> None: self.assertIn(b'"a\\u007f\\u000b\\u00e9\\"\\\\\\n"', data) self.assertEqual(json.loads(data)['x'], value) + def test_json_writes_astral_characters_in_keys_as_surrogate_pairs(self) -> None: + """#1261: a mapping KEY above ``U+FFFF`` round-trips, not just a value. + + ``dictdumper.json.JSON._append_object`` escapes a key through the + module-level :func:`dictdumper.json.encode_string` directly, bypassing + the overridable :meth:`_append_string` the previous test covers, so the + key side needed its own, separate fix. + """ + value = {'astral\U0001F600key': 'v', 'plain': 1} + data = self.dump('JSON', value) + self.assertIn(b'"astral\\ud83d\\ude00key"', data) + self.assertEqual(json.loads(data)['x'], value) + def test_plist_writes_strings_xml_cannot_spell_as_data(self) -> None: """#1262: a control character does not make the document unparseable.""" value = {'c': 'a\x01b\x1bc', 'ok': 'tab\there & '} diff --git a/tests/integration/test_output_formats.py b/tests/integration/test_output_formats.py index 53d364b2f6..e90ef19907 100644 --- a/tests/integration/test_output_formats.py +++ b/tests/integration/test_output_formats.py @@ -117,10 +117,11 @@ class PlistRoundTripTests(EndToEndTestCase): def test_plist_report_round_trips_through_plistlib(self) -> None: """A ``plist`` report is readable by :func:`plistlib.load` (GitHub issue #1448). - ``dictdumper`` writes a ```` with fractional seconds, which - :mod:`plistlib` rejects; :mod:`pcapkit.dumpkit` writes it to whole - seconds instead, and the exact value stays in ``time_epoch``. The date - is compared as naive UTC, since ``aware_datetime`` is new in Python 3.13. + ``dictdumper`` used to write a ```` with fractional seconds, + which :mod:`plistlib` rejects; fixed upstream in ``dictdumper`` 0.8.5 + (DictDumper#124), which now writes whole seconds itself. The exact + value stays in ``time_epoch`` regardless. The date is compared as + naive UTC, since ``aware_datetime`` is new in Python 3.13. """ import datetime diff --git a/tests/integration/test_pcapng_end_to_end.py b/tests/integration/test_pcapng_end_to_end.py index 7a053cc11c..01e5e695bd 100644 --- a/tests/integration/test_pcapng_end_to_end.py +++ b/tests/integration/test_pcapng_end_to_end.py @@ -171,9 +171,12 @@ class PcapngUnescapedKeyTests(EndToEndTestCase): """The ``json`` report of :file:`test.pcapng`, whose mapping keys need escaping. The fixture's decryption secrets block keys its TLS key log entries by a raw - :class:`bytes` client random, and :mod:`dictdumper` writes a mapping key - into a ``json`` report without escaping it. The report parses only because - :mod:`pcapkit.dumpkit` escapes the keys itself (GitHub issue #1152). + :class:`bytes` client random. ``dictdumper.json.JSON._append_object`` used + to write a mapping key into a ``json`` report without escaping it (GitHub + issue #1152), and the report parsed only because :mod:`pcapkit.dumpkit` + escaped the key itself. Fixed upstream in ``dictdumper`` 0.8.5 + (DictDumper#123), so the report now parses on the writer's own escaping; + this test pins that the behaviour survives either way. """ @@ -184,12 +187,9 @@ def test_json_report_of_a_decryption_secrets_block_parses(self) -> None: ` keys the entries by ``bytes.fromhex(random)``, so the key the writer formats is a :class:`bytes` repr carrying ``"`` and ``\\``: - ``b' !"#$%&\\'()*+,-./0123456789:;<=>?'``. - :meth:`dictdumper.json.JSON._append_object` writes a key into - ``'"{item}": '`` raw, so that ``"`` would end the JSON string early -- - still open upstream as JarryShaw/DictDumper#121. The ``escape_key`` - helper of :func:`~pcapkit.dumpkit.common.make_dumper` escapes it with - :func:`json.dumps` first; without that, this test fails. + ``b' !"#$%&\\'()*+,-./0123456789:;<=>?'``, which a key written into + ``'"{item}": '`` raw -- as ``dictdumper`` 0.8.5 no longer does -- would + close the JSON string early. """ extractor = self.extract(fin=sample_path('test.pcapng'), fout=self.out('report'), diff --git a/tests/project/test_rfc_constants_one_home_1739_unit.py b/tests/project/test_rfc_constants_one_home_1739_unit.py index fc3f01bf1f..856c90fd36 100644 --- a/tests/project/test_rfc_constants_one_home_1739_unit.py +++ b/tests/project/test_rfc_constants_one_home_1739_unit.py @@ -336,6 +336,8 @@ 'the IP version an IPv6-in-IP tunnel carries, not the Next Header offset', ('pcapkit/dumpkit/common.py', 'ord(char) > 65535'): 'the last code point of the Basic Multilingual Plane, not an IP length', + ('pcapkit/dumpkit/common.py', 'codepoint > 65535'): + 'above the Basic Multilingual Plane, needs a UTF-16 surrogate pair, not an IP length', } #: Sweep hits of a homed value in a module whose own protocol owns no constant