From c6695e019bc631f234516ab66964aef58fb6f0ce Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 29 Sep 2026 02:28:04 -0400 Subject: [PATCH] fix(const): value-lookup in Method.get, so BASELINE-CONTROL/VERSION-CONTROL resolve (#908) `Method.get` checked only `_member_map_` (names), never `_value2member_map_` (values), so `BASELINE_CONTROL`/`'BASELINE-CONTROL'` and `VERSION_CONTROL`/`'VERSION-CONTROL'` -- whose name and value differ, a hyphen being unusable in an identifier -- fell through to an unregistered member with the wrong `safe`/`idempotent`. `httpv1.py`'s permissive method regex lets a real capture reach this. - Delegate to the base `EnumLookup.get`, keeping the override only for the unregistered fallback; moved `staticmethod` -> `classmethod` since zero-argument `super()` needs `cls` to bind. - Kept `default`'s existing meaning (the fallback member's value) rather than the base's `NO_DEFAULT`/registered-value-only semantics, which would be caller-visible. - Case-sensitivity (#896/#907) and caller's-own-casing (#860) are unchanged, each pinned by its own test; fixed in the crawler template and the generated file together, verified byte-identical. Added an end-to-end `httpv1` test and a template/generated-file parity test. Build: `coverage run -m unittest` over `tests/const/` and `test_http_unit.py`, all passing. Document and pin the non-`str` surface, which this fix moves as a side effect. Delegating to the base routes a non-`str` key through `cls(key)` and so `_missing_`, which raises `ValueError`; `except KeyError` catches only a failed *name* lookup, so that `ValueError` reaches the caller. Measured before and after: get(42) AttributeError: 'int' object has no attribute 'upper' -> ValueError: 42 is not a valid Method get(None) AttributeError -> ValueError get(b'GET') RETURNED name=b'GET' value=b'GET' -> ValueError: b'GET' is not a valid Method The bytes row is a return-to-raise change, and bytes is the plausible mistake: `_RE_METHOD` is a bytes pattern and `httpv1` is bytes throughout, so `httpv1.py:434` is safe only because it wraps the match in `self.decode(...)`. Raising is the intended behaviour -- a bytes-valued `Method` is not something this registry should hand back -- so the fix is a `Raises:` clause plus four pins in a new `NonStrKeyTests`, all four of which fail against unmodified main (failures=2, errors=2; four distinct method names). The docstring pin reads the source via `inspect.getsource(Method.get)` rather than `Method.get.__func__`, which a `staticmethod` does not carry -- against the pre-fix module the `__func__` form died with `AttributeError` before any assertion ran, so it pinned `classmethod`-ness rather than the clause it is named for. Added to the crawler template as well as the generated module, so regeneration cannot drop it. --- pcapkit/const/http/method.py | 103 ++++-- pcapkit/vendor/http/method.py | 103 ++++-- ...st_const_method_case_sensitive_896_unit.py | 16 +- ...test_const_method_value_lookup_908_unit.py | 346 ++++++++++++++++++ 4 files changed, 504 insertions(+), 64 deletions(-) create mode 100644 tests/const/test_const_method_value_lookup_908_unit.py diff --git a/pcapkit/const/http/method.py b/pcapkit/const/http/method.py index edb8141eed..0e0ae483ab 100644 --- a/pcapkit/const/http/method.py +++ b/pcapkit/const/http/method.py @@ -228,47 +228,85 @@ def _unregistered_member(cls, value: 'str', name: 'str') -> 'Method': obj.idempotent = False return obj - @staticmethod - def get(key: 'str', default: 'Optional[str]' = None) -> 'Method': + @classmethod + def get(cls, key: 'str', default: 'Optional[str]' = None) -> 'Method': """Backport support for original codes. + Delegates to :meth:`~pcapkit.corekit.enum.EnumLookup.get` for the + lookup itself, per GitHub issue #908: the previous override checked + only ``_member_map_`` (names), never ``_value2member_map_`` + (values), so the two IANA methods whose member *name* differs from + their *value* -- ``BASELINE_CONTROL`` / ``'BASELINE-CONTROL'`` and + ``VERSION_CONTROL`` / ``'VERSION-CONTROL'``, a hyphen being unusable + in a Python identifier -- failed to resolve through ``get`` even + though :meth:`_missing_` (and so the constructor) already found + them via that same value-side lookup. + + The base is a :class:`classmethod` + (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), and a zero-argument + ``super()`` needs a first argument to bind, so this override had to + move from :class:`staticmethod` to :class:`classmethod` to delegate + at all -- see GitHub issue #908's own correction of the fix it + originally proposed. Callers are unaffected by the switch itself: + ``Method.get('X')`` binds identically either way. + + ``default`` keeps its existing meaning here rather than adopting + the base's ``NO_DEFAULT`` sentinel and its value-only fallback -- + widening it would be caller-visible, since the base's ``default`` + must already name a *registered* value (resolved through + ``_value2member_map_``), while this override's ``default`` instead + supplies the *value* of a freshly minted unregistered member. + Nothing in this tree calls ``get`` with a non-``None`` ``default`` + to notice today, but widening the signature is a separate change + from this defect and stays out of scope. + Args: key: Key to get enum item. Looked up case-**sensitively**, per :rfc:`9110#section-9.1` -- the method token is case-sensitive, unlike :meth:`~pcapkit.const.ftp.command. Command.get`'s equivalent override, which stays case-insensitive because :rfc:`959#section-4.1` says FTP - command codes are not. - default: Default value if not found. + command codes are not. Checked against both member names + and values, via the base's own precedence -- name before + value -- so a value-only match such as + ``'BASELINE-CONTROL'`` now resolves too, closing GitHub + issue #908. + default: Value for the unregistered member built when ``key`` + matches neither a name nor a value. ``None``, the + default, uses ``key`` itself -- unchanged from before + GitHub issue #908. + + Raises: + ValueError: If ``key`` is not a :class:`str`. This reaches the + caller from the base's non-``str`` branch, which calls + ``cls(key)`` and so ``_missing_``, and is **not** caught by + the ``except KeyError`` fallback below -- only a failed + *name* lookup is. The non-``str`` surface therefore moved + with GitHub issue #908: ``get(42)`` and ``get(None)`` used + to raise :exc:`AttributeError` from ``key.upper()``, and + ``get(b'GET')`` used to *return* a member whose name and + value were both the :class:`bytes` object. Raising is the + intended behaviour -- a ``bytes``-valued ``Method`` is not a + thing this registry should hand back -- but it is a change, + so it is stated rather than left to be discovered. :meta private: """ - name = key.upper() - if key not in Method._member_map_: # type: ignore[misc] # pylint: disable=no-member + try: + return super().get(key) + except KeyError: # NOTE: the value is ``default`` if the caller supplied one, or - # else ``key`` exactly as given -- never ``name`` -- so an - # unregistered member's value is the caller's own casing, the - # same convention :meth:`_unregistered_member` documents and - # :class:`~pcapkit.const.ftp.command.FEATCode` already followed - # unchanged. Matching against ``Method._member_map_`` is on - # ``key`` itself now, not ``key.upper()`` -- GitHub issue #896: - # a token differing only in case from a registered member's - # name, e.g. ``get('get')`` against ``GET``, no longer resolves - # to it and instead builds an unregistered member of its own, - # since RFC 9110 makes that a distinct wire token rather than a - # differently-spelled name for the same one. ``name`` stays the - # canonical upper-case form -- only the identifier is - # canonicalised, matching :meth:`_missing_` and every registered - # member's own name. Two calls naming the same method in - # different case, e.g. ``get('frob')`` and ``get('FROB')``, - # therefore build results that are *not* equal -- each is - # exactly what its own caller passed, which minting's - # ``_member_map_`` cache used to paper over by returning the - # *first* casing seen for every later call regardless of case. - # Losing that was the one observable behaviour change in GitHub - # issue #860's conversion. - return Method._unregistered_member(default if default is not None else key, name) - return Method[key] # type: ignore[misc] + # else ``key`` exactly as given -- never its upper-cased form -- + # so an unregistered member's value is the caller's own casing, + # the same convention :meth:`_unregistered_member` documents and + # :class:`~pcapkit.const.ftp.command.FEATCode` already followed. + # The name is always the canonical upper-case form, matching + # :meth:`_missing_` and every registered member's own name. Two + # calls naming the same method in different case, e.g. + # ``get('frob')`` and ``get('FROB')``, therefore build results + # that are *not* equal -- each is exactly what its own caller + # passed, per GitHub issue #860's conversion away from minting. + return cls._unregistered_member(default if default is not None else key, key.upper()) @classmethod def _missing_(cls, value: 'str') -> 'Method': @@ -276,7 +314,12 @@ def _missing_(cls, value: 'str') -> 'Method': Args: value: Value to get enum item. Matched case-insensitively against - the canonical upper-case member names. + the canonical upper-case member names -- deliberately unlike + :meth:`get`, which is case-**sensitive** per + :rfc:`9110#section-9.1`. The split was ruled deliberate on + GitHub issue #896; GitHub issue #908 is the pointer between + the two, so a reader of this one file is not left with two + contradictory rationales and nothing tying them together. """ if not isinstance(value, str): diff --git a/pcapkit/vendor/http/method.py b/pcapkit/vendor/http/method.py index 7fcad6d361..eff8fe0a95 100644 --- a/pcapkit/vendor/http/method.py +++ b/pcapkit/vendor/http/method.py @@ -133,47 +133,85 @@ def _unregistered_member(cls, value: 'str', name: 'str') -> '{NAME}': obj.idempotent = False return obj - @staticmethod - def get(key: 'str', default: 'Optional[str]' = None) -> '{NAME}': + @classmethod + def get(cls, key: 'str', default: 'Optional[str]' = None) -> '{NAME}': """Backport support for original codes. + Delegates to :meth:`~pcapkit.corekit.enum.EnumLookup.get` for the + lookup itself, per GitHub issue #908: the previous override checked + only ``_member_map_`` (names), never ``_value2member_map_`` + (values), so the two IANA methods whose member *name* differs from + their *value* -- ``BASELINE_CONTROL`` / ``'BASELINE-CONTROL'`` and + ``VERSION_CONTROL`` / ``'VERSION-CONTROL'``, a hyphen being unusable + in a Python identifier -- failed to resolve through ``get`` even + though :meth:`_missing_` (and so the constructor) already found + them via that same value-side lookup. + + The base is a :class:`classmethod` + (:meth:`~pcapkit.corekit.enum.EnumLookup.get`), and a zero-argument + ``super()`` needs a first argument to bind, so this override had to + move from :class:`staticmethod` to :class:`classmethod` to delegate + at all -- see GitHub issue #908's own correction of the fix it + originally proposed. Callers are unaffected by the switch itself: + ``{NAME}.get('X')`` binds identically either way. + + ``default`` keeps its existing meaning here rather than adopting + the base's ``NO_DEFAULT`` sentinel and its value-only fallback -- + widening it would be caller-visible, since the base's ``default`` + must already name a *registered* value (resolved through + ``_value2member_map_``), while this override's ``default`` instead + supplies the *value* of a freshly minted unregistered member. + Nothing in this tree calls ``get`` with a non-``None`` ``default`` + to notice today, but widening the signature is a separate change + from this defect and stays out of scope. + Args: key: Key to get enum item. Looked up case-**sensitively**, per :rfc:`9110#section-9.1` -- the method token is case-sensitive, unlike :meth:`~pcapkit.const.ftp.command. Command.get`'s equivalent override, which stays case-insensitive because :rfc:`959#section-4.1` says FTP - command codes are not. - default: Default value if not found. + command codes are not. Checked against both member names + and values, via the base's own precedence -- name before + value -- so a value-only match such as + ``'BASELINE-CONTROL'`` now resolves too, closing GitHub + issue #908. + default: Value for the unregistered member built when ``key`` + matches neither a name nor a value. ``None``, the + default, uses ``key`` itself -- unchanged from before + GitHub issue #908. + + Raises: + ValueError: If ``key`` is not a :class:`str`. This reaches the + caller from the base's non-``str`` branch, which calls + ``cls(key)`` and so ``_missing_``, and is **not** caught by + the ``except KeyError`` fallback below -- only a failed + *name* lookup is. The non-``str`` surface therefore moved + with GitHub issue #908: ``get(42)`` and ``get(None)`` used + to raise :exc:`AttributeError` from ``key.upper()``, and + ``get(b'GET')`` used to *return* a member whose name and + value were both the :class:`bytes` object. Raising is the + intended behaviour -- a ``bytes``-valued ``Method`` is not a + thing this registry should hand back -- but it is a change, + so it is stated rather than left to be discovered. :meta private: """ - name = key.upper() - if key not in {NAME}._member_map_: # type: ignore[misc] # pylint: disable=no-member + try: + return super().get(key) + except KeyError: # NOTE: the value is ``default`` if the caller supplied one, or - # else ``key`` exactly as given -- never ``name`` -- so an - # unregistered member's value is the caller's own casing, the - # same convention :meth:`_unregistered_member` documents and - # :class:`~pcapkit.const.ftp.command.FEATCode` already followed - # unchanged. Matching against ``{NAME}._member_map_`` is on - # ``key`` itself now, not ``key.upper()`` -- GitHub issue #896: - # a token differing only in case from a registered member's - # name, e.g. ``get('get')`` against ``GET``, no longer resolves - # to it and instead builds an unregistered member of its own, - # since RFC 9110 makes that a distinct wire token rather than a - # differently-spelled name for the same one. ``name`` stays the - # canonical upper-case form -- only the identifier is - # canonicalised, matching :meth:`_missing_` and every registered - # member's own name. Two calls naming the same method in - # different case, e.g. ``get('frob')`` and ``get('FROB')``, - # therefore build results that are *not* equal -- each is - # exactly what its own caller passed, which minting's - # ``_member_map_`` cache used to paper over by returning the - # *first* casing seen for every later call regardless of case. - # Losing that was the one observable behaviour change in GitHub - # issue #860's conversion. - return {NAME}._unregistered_member(default if default is not None else key, name) - return {NAME}[key] # type: ignore[misc] + # else ``key`` exactly as given -- never its upper-cased form -- + # so an unregistered member's value is the caller's own casing, + # the same convention :meth:`_unregistered_member` documents and + # :class:`~pcapkit.const.ftp.command.FEATCode` already followed. + # The name is always the canonical upper-case form, matching + # :meth:`_missing_` and every registered member's own name. Two + # calls naming the same method in different case, e.g. + # ``get('frob')`` and ``get('FROB')``, therefore build results + # that are *not* equal -- each is exactly what its own caller + # passed, per GitHub issue #860's conversion away from minting. + return cls._unregistered_member(default if default is not None else key, key.upper()) @classmethod def _missing_(cls, value: 'str') -> '{NAME}': @@ -181,7 +219,12 @@ def _missing_(cls, value: 'str') -> '{NAME}': Args: value: Value to get enum item. Matched case-insensitively against - the canonical upper-case member names. + the canonical upper-case member names -- deliberately unlike + :meth:`get`, which is case-**sensitive** per + :rfc:`9110#section-9.1`. The split was ruled deliberate on + GitHub issue #896; GitHub issue #908 is the pointer between + the two, so a reader of this one file is not left with two + contradictory rationales and nothing tying them together. """ if not isinstance(value, str): diff --git a/tests/const/test_const_method_case_sensitive_896_unit.py b/tests/const/test_const_method_case_sensitive_896_unit.py index c481090c46..81f9e6e85a 100644 --- a/tests/const/test_const_method_case_sensitive_896_unit.py +++ b/tests/const/test_const_method_case_sensitive_896_unit.py @@ -258,12 +258,20 @@ def test_vendor_template_matches_the_generated_module(self) -> None: # And the fix itself is actually present in both -- a byte-identical # comparison above already implies this, but names the exact shape - # so a failure here is legible without a diff. + # so a failure here is legible without a diff. GitHub issue #908 + # moved ``get`` from testing ``_member_map_`` by hand to delegating + # to the base's own ``get`` (which also checks + # ``_value2member_map_``), so the markers pinned here moved with it + # -- see tests.const.test_const_method_value_lookup_908_unit for + # that change's own dedicated coverage. for label, source in (('template render', rendered), ('generated module', committed)): with self.subTest(rendering=label): - self.assertIn("if key not in Method._member_map_", source) - self.assertNotIn("if name not in Method._member_map_", source) - self.assertIn('return Method[key]', source) + self.assertIn('return super().get(key)', source) + self.assertIn( + 'return cls._unregistered_member(default if default is not None ' + "else key, key.upper())", source) + self.assertNotIn('if key not in Method._member_map_', source) + self.assertNotIn('return Method[key]', source) def test_regenerating_twice_is_byte_reproducible(self) -> None: """The same fixture fed to the same template twice must render diff --git a/tests/const/test_const_method_value_lookup_908_unit.py b/tests/const/test_const_method_value_lookup_908_unit.py new file mode 100644 index 0000000000..c557c159a6 --- /dev/null +++ b/tests/const/test_const_method_value_lookup_908_unit.py @@ -0,0 +1,346 @@ +# -*- coding: utf-8 -*- +"""``Method.get`` must consult ``_value2member_map_``, not only ``_member_map_``. + +GitHub issue #908. :class:`~pcapkit.const.http.method.Method` has 40 members, +and exactly two of them -- ``BASELINE_CONTROL`` (value ``'BASELINE-CONTROL'``) +and ``VERSION_CONTROL`` (value ``'VERSION-CONTROL'``) -- have a member *name* +that differs from their *value*, because a hyphen cannot appear in a Python +identifier. The override's own ``get`` checked only ``_member_map_`` (names) +and fell straight through to building an *unregistered* member on any miss, +so a caller spelling either method by its registered *value* got back a +freshly minted, hollowed-out member (``safe=False``, ``idempotent=False``) +instead of the real one the IANA registry describes (``idempotent=True`` for +both) -- even though the very same value already resolved correctly through +the constructor (:meth:`~pcapkit.const.http.method.Method._missing_`, which +this override never touched). + +**Why a capture reaches it.** ``pcapkit/protocols/application/httpv1.py``'s +``_RE_METHOD = re.compile(rb"(?P[A-Z][A-Z-]*)\\Z")`` admits a hyphen, +so a request line carrying ``BASELINE-CONTROL`` matches, reaches +``Enum_Method.get(...)`` at ``httpv1.py:434``, parses successfully, and is +reported with the wrong ``idempotent`` -- the harder kind of defect to +notice, since the parse itself never fails. +:class:`HTTPv1EndToEndTests` below pins that whole path, not just the direct +``Method.get`` call, so a fix that only patched the unit-level symptom would +not be enough. + +**The fix, and the trap in the version the issue first proposed.** Delegate +to :meth:`~pcapkit.corekit.enum.EnumLookup.get` and keep the override only +for the unregistered-member fallback -- but ``Method.get`` was a +:class:`staticmethod` while the base is a :class:`classmethod`, and a +zero-argument ``super()`` inside a :class:`staticmethod` has no first +argument to bind (``RuntimeError: super(): no arguments``). So the override +had to move to :class:`classmethod` as well, following the shape +GitHub issue #913's ``FEATCode.get`` sets for the same base method. + +**Two behaviours settled by earlier issues must survive unchanged**, each +pinned by its own test below rather than only implied by the others: + +* Case-sensitivity, per :rfc:`9110#section-9.1` and GitHub issue #896/#907 -- + ``Method.get('get')`` must *not* resolve to :attr:`Method.GET`. +* An unregistered member's *value* is the caller's own casing, never the + upper-cased form -- GitHub issue #860's ruling, already exercised by + :mod:`tests.const.test_const_method_case_sensitive_896_unit`. + +**The ``default`` signature question, decided rather than assumed.** The base +``get`` is ``get(cls, key, default=NO_DEFAULT)``, where ``default`` must +already name a *registered* value (resolved through +``_value2member_map_``); this override's ``default`` has always meant +something else -- the *value* to give the freshly minted unregistered member +instead of ``key`` itself. Widening the signature to adopt ``NO_DEFAULT`` +would therefore be caller-visible (a non-``None`` ``default`` on an unknown +``key`` would start resolving to an *existing* registered member instead of +minting a new one carrying that string), even though nothing in this tree +currently calls ``get`` with a non-``None`` ``default`` to notice -- +confirmed by grepping every call site under ``pcapkit/`` and ``tests/``. +So the signature and the default's meaning are left exactly as they were; +:class:`DefaultSignatureTests` below pins that explicitly. + +""" +from __future__ import annotations + +import inspect +import unittest + + +class ValueLookupTests(unittest.TestCase): + """The direct repro: the two methods whose name and value differ.""" + + def test_baseline_control_resolves_via_its_value(self) -> None: + """The issue's own first repro line.""" + from pcapkit.const.http.method import Method + + before_names = len(Method._member_map_) + before_values = len(Method._value2member_map_) + + probed = Method.get('BASELINE-CONTROL') + self.assertIs(probed, Method.BASELINE_CONTROL) + self.assertIs(probed, Method('BASELINE-CONTROL')) + self.assertFalse(probed.safe) + self.assertTrue(probed.idempotent) + + # No minting happened to answer the lookup -- both tables are the + # same size before and after. + self.assertEqual(len(Method._member_map_), before_names) + self.assertEqual(len(Method._value2member_map_), before_values) + + def test_version_control_resolves_via_its_value(self) -> None: + """The issue's second repro line -- the only other member like it.""" + from pcapkit.const.http.method import Method + + before_names = len(Method._member_map_) + before_values = len(Method._value2member_map_) + + probed = Method.get('VERSION-CONTROL') + self.assertIs(probed, Method.VERSION_CONTROL) + self.assertIs(probed, Method('VERSION-CONTROL')) + self.assertFalse(probed.safe) + self.assertTrue(probed.idempotent) + + self.assertEqual(len(Method._member_map_), before_names) + self.assertEqual(len(Method._value2member_map_), before_values) + + def test_value_lookup_stays_case_sensitive(self) -> None: + """The value side is a plain ``_value2member_map_`` lookup, not a + casefolded one -- a lower-cased spelling of the registered value + must not resolve to the real member either.""" + from pcapkit.const.http.method import Method + + probed = Method.get('baseline-control') + self.assertIsNot(probed, Method.BASELINE_CONTROL) + self.assertEqual(probed.value, 'baseline-control') + + def test_an_exact_name_match_still_wins_and_ignores_a_supplied_default(self) -> None: + """The base's own precedence (name before value) survives the + delegation, and ``default`` is never consulted on a hit -- matching + the pre-#908 behaviour where a resolved ``key`` ignored ``default`` + entirely.""" + from pcapkit.const.http.method import Method + + self.assertIs(Method.get('GET', default='IGNORED'), Method.GET) + self.assertIs(Method.get('BASELINE-CONTROL', default='IGNORED'), + Method.BASELINE_CONTROL) + + +class PreservedBehaviourTests(unittest.TestCase): + """The two settled behaviours the fix must not disturb.""" + + def test_case_sensitivity_from_896_still_holds(self) -> None: + """RFC 9110 Section 9.1, via GitHub issue #896/#907: a name match is + case-**sensitive**, so ``get('get')`` must not resolve to + :attr:`Method.GET`. Fails against a naive fix that resolves ``key`` + through ``cls(key)`` (which folds case via ``_missing_``) instead of + through the base's own non-minting ``get``.""" + from pcapkit.const.http.method import Method + + probed = Method.get('get') + self.assertIsNot(probed, Method.GET) + self.assertEqual(probed.value, 'get') + self.assertEqual(probed.name, 'GET') + + def test_unregistered_member_keeps_the_callers_own_casing(self) -> None: + """GitHub issue #860: an unregistered member's *value* is exactly + what the caller passed, never upper-cased -- only the *name* (the + identifier) is canonicalised. Fails if the fallback were changed to + build the value from ``name`` instead of the original ``key``.""" + from pcapkit.const.http.method import Method + + before = len(Method.__members__) + probed = Method.get('frob') + self.assertEqual(probed.value, 'frob') + self.assertEqual(probed.name, 'FROB') + self.assertEqual(len(Method.__members__), before) + + +class DefaultSignatureTests(unittest.TestCase): + """``default`` keeps its pre-#908 meaning rather than widening to the + base's ``NO_DEFAULT`` sentinel -- see the module docstring's reasoning.""" + + def test_default_still_names_the_unregistered_members_value(self) -> None: + """Omitted or ``None``, ``default`` uses ``key`` itself; supplied, + it replaces the *value* of the freshly minted unregistered member -- + unchanged from before #908, and distinct from the base's own + ``default``, which must already name a registered value.""" + from pcapkit.const.http.method import Method + + before = len(Method.__members__) + + omitted = Method.get('MixedCase-Unregistered') + self.assertEqual(omitted.value, 'MixedCase-Unregistered') + self.assertEqual(omitted.name, 'MIXEDCASE-UNREGISTERED') + + supplied = Method.get('MixedCase-Unregistered', default='PLACEHOLDER') + self.assertEqual(supplied.value, 'PLACEHOLDER') + self.assertEqual(supplied.name, 'MIXEDCASE-UNREGISTERED') + + self.assertEqual(len(Method.__members__), before) + + def test_get_is_now_a_classmethod_bound_the_same_way_for_callers(self) -> None: + """The ``staticmethod`` -> ``classmethod`` switch GitHub issue #908's + own correction required is invisible to a caller: ``Method.get('X')`` + binds identically either way.""" + from pcapkit.const.http.method import Method + + self.assertIsInstance(inspect.getattr_static(Method, 'get'), classmethod) + self.assertIs(Method.get('GET'), Method.GET) + + +class HTTPv1EndToEndTests(unittest.TestCase): + """The path a real capture takes, per the issue's own "why it matters".""" + + def test_a_baseline_control_request_line_reports_the_registrys_idempotent_flag( + self) -> None: + """``_RE_METHOD`` admits the hyphen, so this request line reaches + ``Enum_Method.get(...)`` at ``httpv1.py:434`` and used to come back + with ``idempotent=False`` -- the wrong answer for a parse that + otherwise succeeds silently. Pins the whole path, not only the + direct ``Method.get`` call above.""" + from pcapkit.const.http.method import Method + from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1 + + proto = object.__new__(HTTPv1) + header, _ = proto._read_http_header( + b'BASELINE-CONTROL /webdav/doc.txt HTTP/1.1\r\nHost: example.test') + + self.assertIs(header.method, Method.BASELINE_CONTROL) + self.assertTrue(header.method.idempotent) + self.assertFalse(header.method.safe) + self.assertEqual(header.uri, '/webdav/doc.txt') + + def test_a_version_control_request_line_reports_the_registrys_idempotent_flag( + self) -> None: + """The issue's second repro, through the same end-to-end path.""" + from pcapkit.const.http.method import Method + from pcapkit.protocols.application.httpv1 import HTTP as HTTPv1 + + proto = object.__new__(HTTPv1) + header, _ = proto._read_http_header( + b'VERSION-CONTROL /repo/doc.txt HTTP/1.1\r\nHost: example.test') + + self.assertIs(header.method, Method.VERSION_CONTROL) + self.assertTrue(header.method.idempotent) + + +class NonStrKeyTests(unittest.TestCase): + """The non-``str`` surface, which GitHub issue #908's fix moved. + + Delegating to the base routes a non-``str`` key through ``cls(key)`` and + so ``_missing_``, which raises :exc:`ValueError`. The override catches + only :exc:`KeyError` -- a failed *name* lookup -- so that + :exc:`ValueError` reaches the caller. Pinned because it is a + caller-visible change that the fix makes as a side effect rather than as + its purpose, and because the ``bytes`` case moved from *returning* a + member to raising. + """ + + def setUp(self) -> None: + """Snapshot both lookup tables -- a probe on a minting registry is + not a read, and these tests must not grow either table.""" + from pcapkit.const.http.method import Method + + self.before = (len(Method._member_map_), len(Method._value2member_map_)) + + def tearDown(self) -> None: + """No member may have been installed by any lookup above.""" + from pcapkit.const.http.method import Method + + self.assertEqual( + (len(Method._member_map_), len(Method._value2member_map_)), + self.before, + 'a non-str lookup must not mint', + ) + + def test_an_int_key_raises_value_error(self) -> None: + """Was :exc:`AttributeError` from ``key.upper()`` before #908.""" + from pcapkit.const.http.method import Method + + with self.assertRaises(ValueError): + Method.get(42) + + def test_a_none_key_raises_value_error(self) -> None: + """Was :exc:`AttributeError` from ``key.upper()`` before #908.""" + from pcapkit.const.http.method import Method + + with self.assertRaises(ValueError): + Method.get(None) # type: ignore[arg-type] + + def test_a_bytes_key_raises_instead_of_returning_a_bytes_valued_member(self) -> None: + """The one that changed from a *return* to a raise. + + Before #908 this handed back a member whose ``name`` and ``value`` + were both ``b'GET'``. ``bytes`` is the plausible mistake here, since + :attr:`~pcapkit.protocols.application.httpv1.HTTP._RE_METHOD` is a + bytes pattern and ``httpv1`` is bytes throughout -- the live call at + ``httpv1.py:434`` is safe only because it wraps the match in + ``self.decode(...)``. + """ + from pcapkit.const.http.method import Method + + with self.assertRaises(ValueError): + Method.get(b'GET') # type: ignore[arg-type] + + def test_the_docstring_documents_the_value_error(self) -> None: + """A caller-visible raise with no ``Raises:`` entry is the gap this + pin exists to keep closed, in the template as well as the generated + module.""" + from pcapkit.const.http.method import Method + from pcapkit.vendor.http.method import LINE + + # NOTE: ``inspect.getsource(Method.get)`` rather than + # ``Method.get.__func__``, which a ``staticmethod`` does not carry -- + # so against the pre-#908 module this would die with + # :exc:`AttributeError` before reaching a single assertion, and would + # be pinning ``classmethod``-ness (already pinned by + # :class:`DefaultSignatureTests`) rather than the docstring it is + # named for. ``getsource`` works on both descriptor kinds. + source = inspect.getsource(Method.get) + rendered = LINE('Method', 'HTTP Method', '', 'pcapkit.vendor.http.method') + + self.assertIn('Raises:', source) + # NOTE: a distinctive sentence rather than the bare word + # ``ValueError``, which occurs incidentally elsewhere in 4kB of + # docstring and would make this assertion near-vacuous. + self.assertIn('value were both the :class:`bytes` object', source) + self.assertIn('value were both the :class:`bytes` object', rendered) + + +class VendorTemplateParityTests(unittest.TestCase): + """The fix lives in the crawler template, so a regeneration cannot + silently discard it -- following GitHub issue #913's own precedent + (``test_the_crawler_template_carries_the_same_get``).""" + + def test_the_crawler_template_carries_the_same_get(self) -> None: + """:class:`~pcapkit.const.http.method.Method` is written out + longhand inside :data:`pcapkit.vendor.http.method.LINE`, so this + renders that template and requires the generated module's own + ``get`` source to appear in it verbatim. Importing the crawler + module reads its module-level template only; the crawl itself is + behind ``if __name__ == '__main__'`` and is never run here, so this + needs no network access.""" + from pcapkit.const.http.method import Method + from pcapkit.vendor.http.method import LINE + + rendered = LINE('Method', 'HTTP Method', '', 'pcapkit.vendor.http.method') + source = inspect.getsource(Method.get.__func__) # type: ignore[attr-defined] + + self.assertIn('GitHub issue #908', source) + self.assertIn('return super().get(key)', source) + self.assertIn(source.rstrip('\n'), rendered) + + def test_the_crawler_template_carries_the_missing_cross_reference_too(self) -> None: + """The issue's own "second, related inconsistency": ``_missing_``'s + docstring gets a pointer to ``get``'s RFC caveat so the two + contradictory case rationales in one file are at least tied + together.""" + from pcapkit.const.http.method import Method + from pcapkit.vendor.http.method import LINE + + rendered = LINE('Method', 'HTTP Method', '', 'pcapkit.vendor.http.method') + source = inspect.getsource(Method._missing_.__func__) # type: ignore[attr-defined] + + self.assertIn('GitHub issue #908', source) + self.assertIn(source.rstrip('\n'), rendered) + + +if __name__ == '__main__': + unittest.main()