diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index 2f04feb66a..6ee1f7abd6 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -27,7 +27,8 @@ House Conventions reference, rather than any code at all; * :ref:`test-coverage` -- what the test suite has to prove; * :ref:`utilities-or-corekit` -- which of :mod:`pcapkit.utilities` and - :mod:`pcapkit.corekit` a support module belongs in; + :mod:`pcapkit.corekit` a support module belongs in, and where a context + variable lives; * :ref:`reassembly-integration` -- :mod:`pcapkit.foundation.reassembly` and what a new engine there needs from :mod:`pcapkit.toolkit` and :mod:`pcapkit.foundation.analysis`; diff --git a/docs/source/contributing/conventions/utilities-or-corekit.rst b/docs/source/contributing/conventions/utilities-or-corekit.rst index b65f771405..97cf650130 100644 --- a/docs/source/contributing/conventions/utilities-or-corekit.rst +++ b/docs/source/contributing/conventions/utilities-or-corekit.rst @@ -53,3 +53,25 @@ checks that. What needs IPv4 and IPv6 stays beside :class:`~pcapkit.protocols.misc.checksum.ip_pseudo.IP_Pseudo`. The package holds one module per checksum family, so a new family sits beside :mod:`~pcapkit.corekit.checksum.internet`. + +Context Variables +~~~~~~~~~~~~~~~~~ + +Where a :class:`~contextvars.ContextVar` lives depends on which modules use it. +Settled by the owner on :issue:`2616`: + +* **One that more than one module sets or reads lives in** + :mod:`pcapkit.utilities.contextvars`, under a public name in its ``__all__``. No + other module re-exports it, so every reader imports it from there. +* **One that only its own module uses stays in that module**, private, as + ``_exthdr_rebuilt`` does in :mod:`pcapkit.protocols.internet.internet`. + +A shared variable kept in one of the modules that use it makes the others import a +private name from that module, and ties them to its place in the import graph. A +module of bare declarations that imports nothing from :mod:`pcapkit` passes the +``utilities`` question above, so any module may import it -- +:mod:`pcapkit.corekit.checksum` included, which must not reach +:mod:`pcapkit.protocols`. :file:`tests/project/test_shared_contextvars_2616_unit.py` +fails on a module that takes a context variable from anywhere else, by name or as an +attribute of an imported module, and on a variable there that is private, missing +from ``__all__``, or read by one module alone. diff --git a/docs/source/pcapkit/utilities/contextvars.rst b/docs/source/pcapkit/utilities/contextvars.rst new file mode 100644 index 0000000000..fe5a5a6097 --- /dev/null +++ b/docs/source/pcapkit/utilities/contextvars.rst @@ -0,0 +1,24 @@ +Shared Context Variables +======================== + +.. module:: pcapkit.utilities.contextvars + +:mod:`pcapkit.utilities.contextvars` holds the :class:`~contextvars.ContextVar` +objects that more than one :mod:`pcapkit` module sets or reads, so that no module +imports another's private state (:issue:`2616`). A context variable that only its +own module uses stays there. This module imports nothing from :mod:`pcapkit`, so +a module at any layer may import it; see :ref:`utilities-or-corekit`. + +.. versionadded:: 1.6.0 + +.. autodata:: pcapkit.utilities.contextvars.beholding + :no-value: + +.. autodata:: pcapkit.utilities.contextvars.frame_depth + :no-value: + +.. autodata:: pcapkit.utilities.contextvars.in_fragment + :no-value: + +.. autodata:: pcapkit.utilities.contextvars.rebuilding + :no-value: diff --git a/docs/source/pcapkit/utilities/index.rst b/docs/source/pcapkit/utilities/index.rst index a893ab2bbc..386443f3b6 100644 --- a/docs/source/pcapkit/utilities/index.rst +++ b/docs/source/pcapkit/utilities/index.rst @@ -7,7 +7,8 @@ Module Utilities :mod:`pcapkit.utilities` holds the cross-cutting services :mod:`pcapkit` is built with: user-refined exceptions and warnings, logging, the tunable limits of :mod:`~pcapkit.utilities.config`, a compatibility layer, timestamp -conversion, and wrappers around third-party libraries. It imports nothing from +conversion, the :mod:`context variables ` its +modules share, and wrappers around third-party libraries. It imports nothing from :mod:`pcapkit` outside itself; see :ref:`utilities-or-corekit`. .. toctree:: @@ -19,6 +20,7 @@ conversion, and wrappers around third-party libraries. It imports nothing from config chardet datetime + contextvars Version Compatibility ===================== diff --git a/pcapkit/corekit/checksum/internet.py b/pcapkit/corekit/checksum/internet.py index 510833d896..d22f7a793d 100644 --- a/pcapkit/corekit/checksum/internet.py +++ b/pcapkit/corekit/checksum/internet.py @@ -25,12 +25,12 @@ """ import contextlib -import contextvars import enum import struct from typing import TYPE_CHECKING from pcapkit.corekit.enum import EnumLookup +from pcapkit.utilities.contextvars import in_fragment, rebuilding from pcapkit.utilities.exceptions import stacklevel from pcapkit.utilities.warnings import ChecksumWarning, warn @@ -43,18 +43,6 @@ 'fragment_payload', ] -#: contextvars.ContextVar[bool]: Whether a payload is being rebuilt by -#: :meth:`Protocol._make_payload -#: `, so that the layer -#: it is rebuilt for parses the same octets again; see -#: :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. It lives here so that -#: :func:`verify_checksum` reads it without importing :mod:`pcapkit.protocols`. -_rebuilding = contextvars.ContextVar('_rebuilding', default=False) - -#: contextvars.ContextVar[bool]: Whether the layer being parsed is the payload -#: of an IP fragment, see :func:`fragment_payload`. -_in_fragment = contextvars.ContextVar('_in_fragment', default=False) - class ChecksumStatus(EnumLookup, enum.IntEnum): """Outcome of verifying a checksum, as recorded in ``checksum_status``. @@ -193,7 +181,7 @@ def verify_checksum(alias: 'str', data: 'bytes', field: 'bytes', *, length: 'Opt value = int.from_bytes(field, 'big') rest = total + (~value & 0xFFFF) expected = ~((rest & 0xFFFF) + (rest >> 16)) & 0xFFFF - if not _rebuilding.get(): + if not rebuilding.get(): warn(f'{alias}: checksum {value:#06x} is wrong; {expected:#06x} expected', ChecksumWarning, stacklevel=stacklevel()) return ChecksumStatus.INVALID @@ -212,8 +200,8 @@ def fragment_payload(fragment: 'bool') -> 'Iterator[None]': fragment: Whether the IP layer is a fragment; an atomic one is not. """ - token = _in_fragment.set(fragment) + token = in_fragment.set(fragment) try: yield finally: - _in_fragment.reset(token) + in_fragment.reset(token) diff --git a/pcapkit/corekit/decorators.py b/pcapkit/corekit/decorators.py index f4a05e5352..020dcf03bf 100644 --- a/pcapkit/corekit/decorators.py +++ b/pcapkit/corekit/decorators.py @@ -24,13 +24,13 @@ decorators either. """ -import contextvars import functools import io import os import traceback from typing import TYPE_CHECKING, cast +from pcapkit.utilities.contextvars import beholding from pcapkit.utilities.exceptions import (CaughtByCaller, ProtocolError, StreamEOFError, StructError, stacklevel) from pcapkit.utilities.logging import DEVMODE, VERBOSE, get_logger @@ -55,14 +55,6 @@ #: :data:`pcapkit.utilities.logging.logger`. logger = get_logger(__name__) -#: contextvars.ContextVar[bool]: Whether a layer that raises is discarded by its -#: caller, as :func:`beholder` replaces it and -#: :func:`~pcapkit.toolkit.pypcapfile._default_extension_header` drops it, so -#: that :meth:`Protocol._parse_next_layer -#: ` drops its warnings -#: with it (:issue:`2308`). -_beholding = contextvars.ContextVar('_beholding', default=False) - def seekset(func: 'Callable[Concatenate[ProtocolBase, P], R_seekset]') -> 'Callable[P, R_seekset]': """Read file from start then set back to original. @@ -183,14 +175,14 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder': try: # call method - token = _beholding.set(True) + token = beholding.set(True) try: # NOTE: The whole call is quiet for a ProtocolError, which the ``except`` below # records; the recovery runs after it, loud unless an outer block is active (#2588). with CaughtByCaller(ProtocolError): return func(*args, **kwargs) finally: - _beholding.reset(token) + beholding.reset(token) except Exception as exc: if isinstance(exc, StructError) and exc.eof: # pylint: disable=no-member from pcapkit.protocols.misc.null import NoPayload as protocol # isort: skip # pylint: disable=import-outside-toplevel diff --git a/pcapkit/protocols/application/tls.py b/pcapkit/protocols/application/tls.py index 4e24cca544..50cdfaffad 100644 --- a/pcapkit/protocols/application/tls.py +++ b/pcapkit/protocols/application/tls.py @@ -64,7 +64,7 @@ from typing import TYPE_CHECKING, cast from pcapkit.const.tls.content_type import ContentType as Enum_ContentType -from pcapkit.corekit.decorators import _beholding, beholder +from pcapkit.corekit.decorators import beholder from pcapkit.protocols.application.ssl import (_TLS_VERSION_MAJOR, _WIRE_CONTENT_TYPES, SSL, TLS13_VERSION) from pcapkit.protocols.application.sslv2 import SSLv2 @@ -73,6 +73,7 @@ from pcapkit.protocols.data.application.sslv2 import SSLv2 as SSLv2_Data from pcapkit.protocols.data.application.tls import TLS as Data_TLS from pcapkit.protocols.schema.application.tls import TLS as Schema_TLS +from pcapkit.utilities.contextvars import beholding from pcapkit.utilities.exceptions import ProtocolError from pcapkit.utilities.warnings import hold_warnings @@ -311,7 +312,7 @@ def unpack(self, length: 'Optional[int]' = None, *, chained: 'bool' = False, # parsed = self._read_sslv2_fallback(len(self) if length is None else length, **kwargs) fallback.discard = parsed is None if parsed is None: - trial.discard = _beholding.get() + trial.discard = beholding.get() raise return parsed diff --git a/pcapkit/protocols/internet/icmpv4.py b/pcapkit/protocols/internet/icmpv4.py index 6c3fffbe40..28c186e685 100644 --- a/pcapkit/protocols/internet/icmpv4.py +++ b/pcapkit/protocols/internet/icmpv4.py @@ -205,8 +205,9 @@ def read(self, length: 'Optional[int]' = None, *, # an IP fragment carries only part of one. if config.CHECKSUM_VERIFY: from pcapkit.corekit.checksum.internet import ( # isort: skip # pylint: disable=import-outside-toplevel - ChecksumStatus, _in_fragment, verify_checksum) - if _in_fragment.get(): + ChecksumStatus, verify_checksum) + from pcapkit.utilities.contextvars import in_fragment # isort: skip # pylint: disable=import-outside-toplevel + if in_fragment.get(): status = ChecksumStatus.NO_PSEUDO_HEADER else: status = verify_checksum(self.alias, self._data, schema.checksum, length=length) diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index 5147f7ff9e..6528846ff9 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -18,13 +18,13 @@ from pcapkit.const.ipv6.extension_header import ExtensionHeader as Enum_ExtensionHeader from pcapkit.const.reg.transtype import TransType as Enum_TransType -from pcapkit.corekit.checksum.internet import _rebuilding from pcapkit.corekit.decorators import beholder from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.protocol import _PT, _ST, ProtocolBase from pcapkit.protocols.schema.schema import keep_short_read, replay_short_read from pcapkit.utilities import config +from pcapkit.utilities.contextvars import rebuilding from pcapkit.utilities.exceptions import stacklevel from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, warn @@ -428,7 +428,7 @@ class registered for ``proto``. Paired with it, the keywords that # NOTE: Inside a payload being rebuilt, the layer it is rebuilt for # warns of the same octets once its own parse gets here, as for # :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. - if not _rebuilding.get(): + if not rebuilding.get(): warn(message, ProtocolWarning, stacklevel=stacklevel()) return Raw, {'error': message} diff --git a/pcapkit/protocols/internet/ipv4.py b/pcapkit/protocols/internet/ipv4.py index 99c76ddd87..79bfad76e1 100644 --- a/pcapkit/protocols/internet/ipv4.py +++ b/pcapkit/protocols/internet/ipv4.py @@ -56,7 +56,6 @@ from pcapkit.const.ipv4.tos_thr import ToSThroughput as Enum_ToSThroughput from pcapkit.const.ipv4.ts_flag import TSFlag as Enum_TSFlag from pcapkit.const.reg.transtype import TransType as Enum_TransType -from pcapkit.corekit.checksum.internet import _rebuilding from pcapkit.corekit.fields.ipaddress import parse_ip_address from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.protocols.data.internet.ipv4 import EOOLOption as Data_EOOLOption @@ -106,6 +105,7 @@ from pcapkit.protocols.schema.internet.ipv4 import UnassignedOption as Schema_UnassignedOption from pcapkit.protocols.schema.schema import Schema from pcapkit.utilities import config +from pcapkit.utilities.contextvars import rebuilding from pcapkit.utilities.exceptions import ProtocolError from pcapkit.utilities.warnings import ProtocolWarning, RegistryWarning, warn @@ -605,12 +605,12 @@ def make(self, from pcapkit.protocols.misc.checksum.ip_pseudo import IP_Pseudo # isort: skip # pylint: disable=import-outside-toplevel # NOTE: The header is parsed back for a source route's destination, - # quietly, as this layer parses it again (cf. _rebuilding). - token = _rebuilding.set(True) + # quietly, as this layer parses it again (cf. rebuilding). + token = rebuilding.set(True) try: header = IPv4(schema.pack()[:ihl * 4]) finally: - _rebuilding.reset(token) + rebuilding.reset(token) # NOTE: The payload puts its own length in the pseudo-header. pseudo = IP_Pseudo(ip=header, protocol=proto, upper_length=0) upper = cast('ProtocolBase', payload) diff --git a/pcapkit/protocols/internet/ipv6.py b/pcapkit/protocols/internet/ipv6.py index 5606b2dfff..8e6c243b5a 100644 --- a/pcapkit/protocols/internet/ipv6.py +++ b/pcapkit/protocols/internet/ipv6.py @@ -37,11 +37,11 @@ from pcapkit.corekit.protochain import ProtoChain from pcapkit.protocols.data.internet.ipv6 import IPv6 as Data_IPv6 from pcapkit.protocols.internet.ip import IP -from pcapkit.protocols.protocol import _frame_depth from pcapkit.protocols.schema.internet.ipv6 import IP_MAX_LENGTH from pcapkit.protocols.schema.internet.ipv6 import IPv6 as Schema_IPv6 from pcapkit.protocols.schema.internet.ipv6 import jumbo_payload_length, jumbogram_error from pcapkit.utilities import config +from pcapkit.utilities.contextvars import frame_depth from pcapkit.utilities.exceptions import ProtocolError if TYPE_CHECKING: @@ -521,7 +521,7 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, _exthdr = [] # type: list[tuple[Type[ProtocolBase], Any]] # (parser class, info) per extension header split = None # type: Optional[tuple[int, int]] # at first Fragment header opaque = False # walk ended at a non-zero Fragment Offset - depth = _frame_depth.get() # layers of the frame up to this one + depth = frame_depth.get() # layers of the frame up to this one # traverse if next header is an extension header payload = self.__header__.get_payload() @@ -555,12 +555,12 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, # NOTE: Counted after every header walked, as it would be were each # dissected by the one before it. ESP dissects its decrypted # payload even when walked, and those layers count on from here. - token = _frame_depth.set(depth + len(_protos)) + token = frame_depth.set(depth + len(_protos)) try: next_ = self._import_next_layer(proto, packet=packet, version=6, extension=True, payload=payload) # type: ignore[misc,call-arg] finally: - _frame_depth.reset(token) + frame_depth.reset(token) info = next_.info # A layer with no ``next`` field ends the walk, as RFC 8200 section 4 @@ -656,14 +656,14 @@ def _decode_next_layer(self, ipv6: 'Data_IPv6', proto: 'Optional[int]' = None, from pcapkit.corekit.checksum.internet import fragment_payload # isort: skip # pylint: disable=import-outside-toplevel frags = self._exthdr.getlist(Enum_ExtensionHeader.IPv6_Frag) # type: ignore[arg-type] scope = fragment_payload(any(frag.info.get('offset') or frag.info.get('mf') for frag in frags)) - token = _frame_depth.set(depth + len(_protos)) + token = frame_depth.set(depth + len(_protos)) try: with scope: return super()._decode_next_layer(ipv6, None if opaque else proto, raw_len, packet=packet, ipv6_exthdr=ProtoChain.from_list(_protos), # type: ignore[arg-type] payload=payload) finally: - _frame_depth.reset(token) + frame_depth.reset(token) @beholder # type: ignore[arg-type] def _import_next_layer(self, proto: 'int', length: 'Optional[int]' = None, *, # pylint: disable=arguments-differ diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index e89f64aa5f..a8e5cb7011 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -25,7 +25,6 @@ import collections import collections.abc import contextlib -import contextvars import copy import difflib import enum @@ -43,9 +42,8 @@ import aenum -from pcapkit.corekit.checksum.internet import _rebuilding from pcapkit.corekit.context import ContextRegistry -from pcapkit.corekit.decorators import _beholding, beholder, seekset +from pcapkit.corekit.decorators import beholder, seekset from pcapkit.corekit.fields.misc import ConditionalField, SchemaField, SwitchField from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain @@ -60,6 +58,7 @@ from pcapkit.utilities import config from pcapkit.utilities.chardet import detect from pcapkit.utilities.compat import cached_property +from pcapkit.utilities.contextvars import beholding, frame_depth, rebuilding from pcapkit.utilities.exceptions import (BaseError, ProtocolError, ProtocolNotFound, ProtocolNotImplemented, RegistryError, StructError, UnsupportedCall, stacklevel) @@ -426,13 +425,6 @@ def _check_construction_keywords(cls: 'type', kwargs: 'dict[str, Any]', UnknownFieldWarning) -#: contextvars.ContextVar[int]: How many layers of the frame being dissected -#: come before the next one :meth:`Protocol._parse_next_layer -#: ` parses; see -#: :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. -_frame_depth = contextvars.ContextVar('_frame_depth', default=0) - - def _short_of_hint(klass: 'Type[ProtocolBase]', captured: 'int') -> 'bool': """Tell whether ``captured`` octets may be too few for a ``klass`` header. @@ -2207,12 +2199,12 @@ def _make_payload(cls, data: 'Data') -> 'ProtocolBase': NoPayload # pylint: disable=import-outside-toplevel return NoPayload() - token, rebuilding = _frame_depth.set(_frame_depth.get() + 1), _rebuilding.set(True) + token, rebuilt = frame_depth.set(frame_depth.get() + 1), rebuilding.set(True) try: return proto.from_data(data[name]) finally: - _rebuilding.reset(rebuilding) - _frame_depth.reset(token) + rebuilding.reset(rebuilt) + frame_depth.reset(token) @staticmethod def _lookup_registry(registry: 'DefaultDict[Any, _VT]', code: 'Any') -> '_VT': @@ -2433,7 +2425,7 @@ def _parse_next_layer(klass: 'Type[ProtocolBase]', file_: 'bytes', length: 'int' ProtocolWarning: If the next layer is past :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. """ - depth = _frame_depth.get() + 1 + depth = frame_depth.get() + 1 limit = config.FRAME_LAYER_LIMIT if limit != -1 and depth > limit: from pcapkit.protocols.misc.null import NoPayload # isort: skip # pylint: disable=import-outside-toplevel @@ -2445,32 +2437,32 @@ def _parse_next_layer(klass: 'Type[ProtocolBase]', file_: 'bytes', length: 'int' f'from {klass.__name__} on, is left undissected') # NOTE: Inside a payload being rebuilt, the layer it is rebuilt # for warns of the same octets once its own parse gets here. - if not _rebuilding.get(): + if not rebuilding.get(): warn(message, ProtocolWarning, stacklevel=stacklevel()) raw = Raw(file_, length, **{**kwargs, 'error': message}) raw._past_layer_limit = True # pylint: disable=protected-access return raw captured = min(length, len(file_)) - token = _frame_depth.set(depth) + token = frame_depth.set(depth) try: # NOTE: Under ``beholder`` a layer that raises is replaced at any length, # so its warnings are held however much was captured (#2516). - with hold_warnings(_beholding.get() or _short_of_hint(klass, captured)) as held: + with hold_warnings(beholding.get() or _short_of_hint(klass, captured)) as held: try: next_ = klass(file_, length, **kwargs) # type: ignore[abstract] except Exception: # NOTE: Under ``beholder`` the layer that raised is replaced, # so its warnings go with it; outside it, the error reaches # the caller and they are reported as it leaves (#2308). - held.discard = _beholding.get() + held.discard = beholding.get() raise length_hint = getattr(next_, '__length_hint__', None) hint = None if length_hint is None else length_hint() held.discard = hint is not None and captured < hint and captured < next_.length finally: - _frame_depth.reset(token) + frame_depth.reset(token) if held.discard: from pcapkit.protocols.misc.raw import Raw # isort: skip # pylint: disable=import-outside-toplevel next_ = Raw(file_, length, **kwargs) diff --git a/pcapkit/protocols/transport/dccp.py b/pcapkit/protocols/transport/dccp.py index d40d01da5e..b1c7b0de1d 100644 --- a/pcapkit/protocols/transport/dccp.py +++ b/pcapkit/protocols/transport/dccp.py @@ -1427,7 +1427,8 @@ def _verify_pseudo_checksum(self, pseudo: 'Optional[IP_Pseudo]') -> 'None': ChecksumWarning: If the checksum or its coverage is invalid. """ - from pcapkit.corekit.checksum.internet import ChecksumStatus, _rebuilding, verify_checksum # isort: skip # pylint: disable=import-outside-toplevel + from pcapkit.corekit.checksum.internet import ChecksumStatus, verify_checksum # isort: skip # pylint: disable=import-outside-toplevel + from pcapkit.utilities.contextvars import rebuilding # isort: skip # pylint: disable=import-outside-toplevel from pcapkit.utilities.exceptions import stacklevel # isort: skip # pylint: disable=import-outside-toplevel from pcapkit.utilities.warnings import ChecksumWarning # isort: skip # pylint: disable=import-outside-toplevel @@ -1440,7 +1441,7 @@ def _verify_pseudo_checksum(self, pseudo: 'Optional[IP_Pseudo]') -> 'None': status = verify_checksum(self.alias, self._data, self._info.checksum, length=coverage, prefix=pseudo.data) else: - if not _rebuilding.get(): + if not rebuilding.get(): warn(f'{self.alias}: checksum coverage {self._info.cscov} runs past the ' f'{length} octets of the packet', ChecksumWarning, stacklevel=stacklevel()) status = ChecksumStatus.INVALID diff --git a/pcapkit/toolkit/pypcapfile.py b/pcapkit/toolkit/pypcapfile.py index 132b3d54dd..02a11008de 100644 --- a/pcapkit/toolkit/pypcapfile.py +++ b/pcapkit/toolkit/pypcapfile.py @@ -48,7 +48,6 @@ from pcapkit.const.reg.ethertype import EtherType as Enum_EtherType from pcapkit.const.reg.transtype import TransType as Enum_TransType -from pcapkit.corekit.decorators import _beholding from pcapkit.foundation.engines.pypcapfile import PyPCAPFile from pcapkit.foundation.reassembly.data.ip import Packet as IP_Packet from pcapkit.foundation.reassembly.data.tcp import Packet as TCP_Packet @@ -61,6 +60,7 @@ from pcapkit.protocols.schema.transport import tcp as Schema_TCP_Module from pcapkit.protocols.transport.tcp import TCP as Protocol_TCP from pcapkit.utilities import config +from pcapkit.utilities.contextvars import beholding from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall, stacklevel from pcapkit.utilities.warnings import AttributeWarning, hold_warnings, warn @@ -486,7 +486,7 @@ def _default_accepts(name: 'str', header: 'bytes') -> 'bool': protocol = Protocol_IPv4 if name == 'IPv4' else Protocol_TCP try: # NOTE: The parser reads its own header, outside ``_parse_next_layer``, - # so ``_beholding`` would hold nothing here (#2526). + # so ``beholding`` would hold nothing here (#2526). with hold_warnings() as held: try: protocol(header, len(header), protocol=name) @@ -521,14 +521,14 @@ def _default_extension_header(protocol: 'int', data: 'bytes') -> 'Optional[tuple """ # pylint: disable-next=protected-access klass = Protocol_IPv4._lookup_next_layer(Protocol_IPv4.__proto__, protocol) - token = _beholding.set(True) + token = beholding.set(True) try: header = Protocol_IPv4._parse_next_layer( # pylint: disable=protected-access klass, data, len(data), version=4, extension=False, alias=protocol, protocol=klass) except Exception: # as beholder does return None finally: - _beholding.reset(token) + beholding.reset(token) nxt = getattr(header.info, 'next', None) # none on a header cut short, read as raw octets if nxt is None: return None diff --git a/pcapkit/utilities/__init__.py b/pcapkit/utilities/__init__.py index 08ff69fe81..1416d41558 100644 --- a/pcapkit/utilities/__init__.py +++ b/pcapkit/utilities/__init__.py @@ -9,7 +9,8 @@ :mod:`pcapkit` is built with: user-refined exceptions and warnings, logging, the tunable limits of :mod:`~pcapkit.utilities.config`, a compatibility layer, -timestamp conversion, and wrappers around third-party +timestamp conversion, the context variables its modules +share, and wrappers around third-party libraries. It imports nothing from :mod:`pcapkit` outside itself (:issue:`1712`). diff --git a/pcapkit/utilities/contextvars.py b/pcapkit/utilities/contextvars.py new file mode 100644 index 0000000000..ee11579690 --- /dev/null +++ b/pcapkit/utilities/contextvars.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +"""Shared Context Variables +========================== + +.. module:: pcapkit.utilities.contextvars + +:mod:`pcapkit.utilities.contextvars` holds the :class:`~contextvars.ContextVar` +objects that more than one :mod:`pcapkit` module sets or reads, so that no module +imports another's private state (:issue:`2616`). A context variable that only its +own module uses stays there. This module imports nothing from :mod:`pcapkit`, so +a module at any layer may import it; see :ref:`utilities-or-corekit`. + +.. versionadded:: 1.6.0 + +""" +import contextvars + +__all__ = ['beholding', 'frame_depth', 'in_fragment', 'rebuilding'] + +#: contextvars.ContextVar[bool]: Whether a layer that raises is discarded by its +#: caller, as :func:`~pcapkit.corekit.decorators.beholder` replaces it and +#: :func:`~pcapkit.toolkit.pypcapfile._default_extension_header` drops it, so +#: that :meth:`Protocol._parse_next_layer +#: ` drops its warnings +#: with it (:issue:`2308`). +beholding = contextvars.ContextVar('pcapkit.utilities.contextvars.beholding', default=False) + +#: contextvars.ContextVar[int]: How many layers of the frame being dissected +#: come before the next one :meth:`Protocol._parse_next_layer +#: ` parses; see +#: :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. +frame_depth = contextvars.ContextVar('pcapkit.utilities.contextvars.frame_depth', default=0) + +#: contextvars.ContextVar[bool]: Whether the layer being parsed is the payload +#: of an IP fragment, see :func:`~pcapkit.corekit.checksum.internet.fragment_payload`. +in_fragment = contextvars.ContextVar('pcapkit.utilities.contextvars.in_fragment', default=False) + +#: contextvars.ContextVar[bool]: Whether a payload is being rebuilt by +#: :meth:`Protocol._make_payload +#: `, so that the layer +#: it is rebuilt for parses the same octets again; see +#: :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT`. +rebuilding = contextvars.ContextVar('pcapkit.utilities.contextvars.rebuilding', default=False) diff --git a/tests/project/test_shared_contextvars_2616_unit.py b/tests/project/test_shared_contextvars_2616_unit.py new file mode 100644 index 0000000000..b11dc881de --- /dev/null +++ b/tests/project/test_shared_contextvars_2616_unit.py @@ -0,0 +1,275 @@ +# -*- coding: utf-8 -*- +"""A context variable crosses modules only from :mod:`pcapkit.utilities.contextvars`. + +GitHub issue #2616 settled where a :class:`~contextvars.ContextVar` lives: one +that more than one module sets or reads goes in +:mod:`pcapkit.utilities.contextvars`, under a public name in its ``__all__``, +and one that only its own module uses stays there. So this module fails when a +module under :file:`pcapkit/` imports a name bound to a ``ContextVar`` in another +:mod:`pcapkit` module, other than :mod:`pcapkit.utilities.contextvars`. That +covers an import at top level, inside a function and under ``TYPE_CHECKING``, +``from X import name`` and ``X.name`` on an imported module alike, and a name +another module re-binds by importing it. It fails as well on a variable there +that is private, missing from ``__all__``, or read by one module alone. + +Each module is read with :mod:`ast` rather than imported, as +:mod:`tests.project.test_utilities_import_boundary_1735_unit` reads them. The +rule is written down on +:file:`docs/source/contributing/conventions/utilities-or-corekit.rst`. + +This module is unit-tier and reads no capture. + +""" +from __future__ import annotations + +import ast +import collections +import unittest +from typing import TYPE_CHECKING, NamedTuple + +from tests.project.test_utilities_import_boundary_1735_unit import (ROOT, _module_name, + _relative_base) + +if TYPE_CHECKING: + from typing import ClassVar, Iterator + +#: The package the rule governs. +PCAPKIT = ROOT / 'pcapkit' + +#: The one module a context variable may be imported from. +HOME = 'pcapkit.utilities.contextvars' + + +class Module(NamedTuple): + """A parsed source file.""" + + #: Its syntax tree. + tree: 'ast.Module' + #: Whether it is a package's :file:`__init__.py`. + is_package: 'bool' + #: Its path, as a violation names it. + path: 'str' + + +class Reference(NamedTuple): + """One module's use of a context variable bound in another.""" + + #: The module that uses it. + reader: 'str' + #: The line it is used on. + line: 'int' + #: The module it is taken from. + source: 'str' + #: Its name there. + name: 'str' + + +def parse_tree() -> 'dict[str, Module]': + """Every module under :data:`PCAPKIT`, by dotted name.""" + return {_module_name(path): Module(ast.parse(path.read_text(encoding='utf-8')), + path.name == '__init__.py', + path.relative_to(ROOT).as_posix()) + for path in sorted(PCAPKIT.rglob('*.py'))} + + +def _module_level(tree: 'ast.Module') -> 'Iterator[ast.AST]': + """Statements that bind module-level names, inside ``if``, ``try`` and ``with`` too.""" + stack = list(tree.body) # type: list[ast.AST] + while stack: + node = stack.pop() + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + continue + yield node + stack.extend(child for child in ast.iter_child_nodes(node) + if isinstance(child, (ast.stmt, ast.excepthandler))) + + +def _is_contextvar(node: 'ast.AST | None') -> 'bool': + """Whether ``node`` calls ``ContextVar``, bare, as an attribute or subscripted.""" + if not isinstance(node, ast.Call): + return False + func = node.func.value if isinstance(node.func, ast.Subscript) else node.func + return (func.attr if isinstance(func, ast.Attribute) else getattr(func, 'id', None)) == 'ContextVar' + + +def _target(node: 'ast.ImportFrom', module: 'str', is_package: 'bool') -> 'str': + """The module a ``from`` import reads from, with relative imports resolved.""" + if not node.level: + return node.module or '' + base = _relative_base(module, is_package, node.level) + return f'{base}.{node.module}' if node.module else base + + +def bound_names(modules: 'dict[str, Module]') -> 'dict[str, set[str]]': + """The module-level names each module binds to a context variable. + + A name is bound by assigning ``ContextVar(...)`` to it, or by importing a + name another module binds, which is how a re-export would hand one on. + + """ + bound = {name: set() for name in modules} # type: dict[str, set[str]] + for name, module in modules.items(): + for node in _module_level(module.tree): + if isinstance(node, ast.Assign) and _is_contextvar(node.value): + bound[name].update(target.id for target in node.targets if isinstance(target, ast.Name)) + elif (isinstance(node, ast.AnnAssign) and _is_contextvar(node.value) + and isinstance(node.target, ast.Name)): + bound[name].add(node.target.id) + changed = True + while changed: + changed = False + for name, module in modules.items(): + for node in _module_level(module.tree): + if not isinstance(node, ast.ImportFrom): + continue + source = _target(node, name, module.is_package) + for alias in node.names: + local = alias.asname or alias.name + if alias.name in bound.get(source, ()) and local not in bound[name]: + bound[name].add(local) + changed = True + return bound + + +def _dotted(node: 'ast.AST', aliases: 'dict[str, str]') -> 'str | None': + """The dotted name ``node`` spells, its first part resolved through ``aliases``.""" + parts = [] # type: list[str] + while isinstance(node, ast.Attribute): + parts.append(node.attr) + node = node.value + if not isinstance(node, ast.Name) or node.id not in aliases: + return None + return '.'.join([aliases[node.id]] + parts[::-1]) + + +def references(modules: 'dict[str, Module]', bound: 'dict[str, set[str]]') -> 'list[Reference]': + """Every use of a context variable another module binds, in module and line order.""" + found = [] # type: list[Reference] + for name, module in modules.items(): + aliases = {} # type: dict[str, str] + nodes = sorted((node for node in ast.walk(module.tree) if hasattr(node, 'lineno')), + key=lambda node: (getattr(node, 'lineno'), getattr(node, 'col_offset'))) + for node in nodes: + if isinstance(node, ast.Import): + for alias in node.names: + if alias.asname: + aliases[alias.asname] = alias.name + else: + aliases[alias.name.split('.')[0]] = alias.name.split('.')[0] + elif isinstance(node, ast.ImportFrom): + source = _target(node, name, module.is_package) + for alias in node.names: + aliases[alias.asname or alias.name] = f'{source}.{alias.name}' + if alias.name in bound.get(source, ()) and source != name: + found.append(Reference(name, node.lineno, source, alias.name)) + elif isinstance(node, ast.Attribute): + owner = _dotted(node.value, aliases) + if owner is not None and node.attr in bound.get(owner, ()) and owner != name: + found.append(Reference(name, node.lineno, owner, node.attr)) + return found + + +def violations(modules: 'dict[str, Module]') -> 'list[str]': + """Every context variable taken from a module other than :data:`HOME`, as ``path:line: name``.""" + return [f'{modules[ref.reader].path}:{ref.line}: {ref.source}.{ref.name}' + for ref in references(modules, bound_names(modules)) if ref.source != HOME] + + +def _exported(tree: 'ast.Module') -> 'set[str]': + """The names a module's literal ``__all__`` lists.""" + for node in tree.body: + if (isinstance(node, ast.Assign) and isinstance(node.value, ast.List) + and any(isinstance(target, ast.Name) and target.id == '__all__' for target in node.targets)): + return {item.value for item in node.value.elts + if isinstance(item, ast.Constant) and isinstance(item.value, str)} + return set() + + +class SharedContextVarsTests(unittest.TestCase): + """Context variables are shared through :mod:`pcapkit.utilities.contextvars` alone.""" + + #: Every module under :data:`PCAPKIT`. + modules: 'ClassVar[dict[str, Module]]' + #: The names each of them binds to a context variable. + bound: 'ClassVar[dict[str, set[str]]]' + + @classmethod + def setUpClass(cls) -> None: + cls.modules = parse_tree() + cls.bound = bound_names(cls.modules) + + def test_the_walk_found_the_variables(self) -> None: + self.assertTrue(self.bound[HOME]) + self.assertTrue(any(names for name, names in self.bound.items() if name != HOME), + 'the walk found no context variable outside pcapkit.utilities.contextvars') + + def test_no_module_imports_a_context_variable_from_elsewhere(self) -> None: + found = violations(self.modules) + self.assertEqual(found, [], + 'these modules under pcapkit/ take a context variable from a module other ' + 'than pcapkit.utilities.contextvars, which #2616 rules out; move a shared ' + 'one there under a public name, with no re-export at its old path:\n' + + '\n'.join(found)) + + def test_every_shared_variable_is_public_and_exported(self) -> None: + names = self.bound[HOME] + self.assertEqual(sorted(names), sorted(_exported(self.modules[HOME].tree))) + self.assertEqual([name for name in sorted(names) if name.startswith('_')], []) + + def test_every_shared_variable_has_more_than_one_reader(self) -> None: + readers = collections.defaultdict(set) # type: collections.defaultdict[str, set[str]] + for ref in references(self.modules, self.bound): + if ref.source == HOME: + readers[ref.name].add(ref.reader) + lonely = {name: sorted(readers[name]) for name in sorted(self.bound[HOME]) if len(readers[name]) < 2} + self.assertEqual(lonely, {}, + 'these context variables in pcapkit.utilities.contextvars have at most one ' + 'reader, so #2616 puts each back in that module under a private name') + + def test_the_scanner_sees_every_form_of_import(self) -> None: + sources = { + HOME: "import contextvars\nshared = contextvars.ContextVar('shared', default=0)\n", + 'pcapkit.owner': '\n'.join([ + 'import contextvars', + 'from contextvars import ContextVar', + 'from pcapkit.utilities.contextvars import shared', + "_private = contextvars.ContextVar('_private', default=0)", + "_annotated: 'ContextVar[int]' = ContextVar('_annotated', default=0)", + "if True:\n _nested = ContextVar[int]('_nested', default=0)", + 'def f():', + " _local = ContextVar('_local', default=0)", + ' return _private.get()', + ]), + 'pcapkit.pkg.reader': '\n'.join([ + 'from typing import TYPE_CHECKING', # 1 + 'from pcapkit.utilities.contextvars import shared', # 2 + 'from pcapkit.utilities import contextvars', # 3 + 'from pcapkit.owner import _private', # 4 + 'import pcapkit.owner', # 5 + 'from pcapkit import owner as alias', # 6 + 'if TYPE_CHECKING:', # 7 + ' from pcapkit.owner import _annotated', # 8 + 'def f():', # 9 + ' from pcapkit.owner import shared as again', # 10 + ' from .. import owner', # 11 + ' return (contextvars.shared, pcapkit.owner._nested,', # 12 + ' alias._annotated, owner._private, owner.f)', # 13 + ]), + } + modules = {name: Module(ast.parse(source), False, name) for name, source in sources.items()} + bound = bound_names(modules) + self.assertEqual(bound['pcapkit.owner'], {'shared', '_private', '_annotated', '_nested'}) + self.assertEqual(violations(modules), [ + 'pcapkit.pkg.reader:4: pcapkit.owner._private', + 'pcapkit.pkg.reader:8: pcapkit.owner._annotated', + 'pcapkit.pkg.reader:10: pcapkit.owner.shared', + 'pcapkit.pkg.reader:12: pcapkit.owner._nested', + 'pcapkit.pkg.reader:13: pcapkit.owner._annotated', + 'pcapkit.pkg.reader:13: pcapkit.owner._private', + ]) + self.assertEqual(sorted({ref.reader for ref in references(modules, bound) if ref.source == HOME}), + ['pcapkit.owner', 'pcapkit.pkg.reader']) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/toolkit/test_pypcapfile_accepts_trial_2526_unit.py b/tests/toolkit/test_pypcapfile_accepts_trial_2526_unit.py index d81e8a563e..2dbaa6d499 100644 --- a/tests/toolkit/test_pypcapfile_accepts_trial_2526_unit.py +++ b/tests/toolkit/test_pypcapfile_accepts_trial_2526_unit.py @@ -9,7 +9,7 @@ warns from the caller's line. The parser reads its own header rather than a next layer, so setting -``_beholding`` around it holds nothing, and the trial holds its own warnings. +``beholding`` around it holds nothing, and the trial holds its own warnings. The answers are cached by octets, so each test clears the cache before it asks. :mod:`pcapkit` is imported inside each test, after diff --git a/tests/toolkit/test_pypcapfile_extension_header_trial_2526_unit.py b/tests/toolkit/test_pypcapfile_extension_header_trial_2526_unit.py index 3120d0a709..3096af45a8 100644 --- a/tests/toolkit/test_pypcapfile_extension_header_trial_2526_unit.py +++ b/tests/toolkit/test_pypcapfile_extension_header_trial_2526_unit.py @@ -51,14 +51,14 @@ def _ask(self, protocol: 'int', data: 'bytes', The trial must leave ``beholder``'s flag unset, whatever it answers. """ - from pcapkit.corekit import decorators from pcapkit.toolkit.pypcapfile import _default_extension_header + from pcapkit.utilities.contextvars import beholding _default_extension_header.cache_clear() with warnings.catch_warnings(record=True) as caught: warnings.simplefilter(action) answer = _default_extension_header(protocol, data) - self.assertFalse(decorators._beholding.get()) # pylint: disable=protected-access + self.assertFalse(beholding.get()) return answer, caught def test_raising_trial_warns_nothing(self) -> None: