From 99e430cc66d2e5f31c80a1d4d09dd32908acce90 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 28 Sep 2026 23:45:40 -0400 Subject: [PATCH] fix(const,vendor): make Method.get case-sensitive, per RFC 9110 (#896) Method.get upper-cased its key before matching, so Method.get('get') resolved to the standardised Method.GET member -- RFC 9110 Section 9.1 makes the HTTP method token case-sensitive, so a lower-case get is a distinct, non-standardised method, not an alias of GET. - pcapkit/vendor/http/method.py: the get() override in this crawler's own LINE template now matches against `key` itself rather than `key.upper()`, and falls through to build an unregistered member (preserving the caller's own casing) exactly as before on a miss. Command.get (FTP) is untouched: RFC 959 Section 4.1 makes FTP command codes case-insensitive, so its equivalent override stays. - pcapkit/const/http/method.py regenerated from that template; the diff is confined to get()'s body and docstring. Verified byte-reproducible across two independent offline regenerations fed from a CSV fixture reconstructed from the committed file (no live IANA crawl). - Updated three pre-existing tests that pinned the old case-insensitive Method.get behaviour (#582/#583), and docs/source/conventions.rst's illustration of the same. Added tests/const/test_const_method_case_sensitive_896_unit.py for the direct repro, the unregistered-member casing guarantee, and the crawler/generated-file parity check. - Did not delete the override in favour of EnumRegistry.get, as the issue suggested: measured directly, the base's get() raises KeyError for an unmatched str key with no default, rather than building an unregistered member -- deleting it would have made Method.get('get') raise instead of resolving to a pseudo-member preserving 'get', which is what issue #896 and the tests both require. Build/tests: `coverage run -m pytest tests/const/ tests/protocols/application/test_http_unit.py` green (264 tests / 40325 subtests in tests/const/, 60 tests / 387 subtests in test_http_unit.py); pylint and mypy report no new findings against pcapkit/const/http/method.py and pcapkit/vendor/http/method.py. --- docs/source/conventions.rst | 11 +- pcapkit/const/http/method.py | 36 ++- pcapkit/vendor/http/method.py | 36 ++- tests/const/test_const_enum_builtin_parity.py | 12 +- tests/const/test_const_enum_no_mint.py | 80 ++++- ...st_const_method_case_sensitive_896_unit.py | 287 ++++++++++++++++++ tests/protocols/application/test_http_unit.py | 52 +++- 7 files changed, 462 insertions(+), 52 deletions(-) create mode 100644 tests/const/test_const_method_case_sensitive_896_unit.py diff --git a/docs/source/conventions.rst b/docs/source/conventions.rst index 697db867fe..9a9bef5e18 100644 --- a/docs/source/conventions.rst +++ b/docs/source/conventions.rst @@ -267,8 +267,15 @@ do not share the generated template; bringing them onto the base is tracked in :class:`~pcapkit.const.http.method.Method`, :class:`~pcapkit.const.pcapng.option_type.OptionType` and :class:`~pcapkit.const.reg.apptype.apptype.AppType` -- and probing one of those - measures the override rather than the base. ``Method.get`` upper-cases its key, - which makes it look as though the base were case-insensitive. + measures the override rather than the base. ``Command.get`` upper-cases its key + before matching, which makes it look as though the base were case-insensitive -- + deliberately, since :rfc:`959#section-4.1` treats FTP command codes identically + regardless of case. ``Method.get`` used to fold case the same way, but + `#896 `__ made it + case-sensitive instead: :rfc:`9110#section-9.1` says the HTTP method token is + case-sensitive, so ``Method.get('get')`` no longer resolves to + ``Method.GET`` -- it builds its own unregistered member, preserving the + caller's exact casing, the same way an unrecognised value always does. What does survive is narrower and deliberate: a **declared-but-unassigned** ``str`` value resolves through ``cls(value)`` but not through ``get(value)``, because diff --git a/pcapkit/const/http/method.py b/pcapkit/const/http/method.py index 434ab0450e..edb8141eed 100644 --- a/pcapkit/const/http/method.py +++ b/pcapkit/const/http/method.py @@ -233,28 +233,42 @@ def get(key: 'str', default: 'Optional[str]' = None) -> 'Method': """Backport support for original codes. Args: - key: Key to get enum item. Looked up case-insensitively, since - member names are canonicalised to upper case on registration. + 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. :meta private: """ name = key.upper() - if name not in Method._member_map_: # type: ignore[misc] # pylint: disable=no-member + if key not in Method._member_map_: # type: ignore[misc] # pylint: disable=no-member # 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. 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 is the one - # observable behaviour change in GitHub issue #860's conversion. + # 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[name] # type: ignore[misc] + return Method[key] # type: ignore[misc] @classmethod def _missing_(cls, value: 'str') -> 'Method': diff --git a/pcapkit/vendor/http/method.py b/pcapkit/vendor/http/method.py index f4a83c0c01..7fcad6d361 100644 --- a/pcapkit/vendor/http/method.py +++ b/pcapkit/vendor/http/method.py @@ -138,28 +138,42 @@ def get(key: 'str', default: 'Optional[str]' = None) -> '{NAME}': """Backport support for original codes. Args: - key: Key to get enum item. Looked up case-insensitively, since - member names are canonicalised to upper case on registration. + 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. :meta private: """ name = key.upper() - if name not in {NAME}._member_map_: # type: ignore[misc] # pylint: disable=no-member + if key not in {NAME}._member_map_: # type: ignore[misc] # pylint: disable=no-member # 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. 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 is the one - # observable behaviour change in GitHub issue #860's conversion. + # 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}[name] # type: ignore[misc] + return {NAME}[key] # type: ignore[misc] @classmethod def _missing_(cls, value: 'str') -> '{NAME}': diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index b7a374469a..baaaa732a3 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -731,10 +731,18 @@ def test_the_guard_leaves_the_other_lookup_paths_alone(self) -> None: from pcapkit.const.reg.apptype import TransportProtocol from pcapkit.const.tcp.flags import Flags - # String paths, which bypass ``_missing_``. + # String paths, which bypass ``_missing_``. ``Method.get`` is probed + # with the exact registered casing rather than ``'get'`` -- GitHub + # issue #896 made its matching case-sensitive, so a lower-cased probe + # would no longer resolve to ``Method.GET`` at all; that behaviour + # change is pinned on its own in + # :mod:`tests.const.test_const_enum_no_mint` + # (``BespokeGetUnchangedTests.test_method_get_is_now_case_sensitive``) + # rather than here, where the point is only that the string path + # still bypasses the guard this test is about. self.assertIs(Flags.get('SYN'), Flags.SYN) self.assertIs(Command.get('abor'), Command.ABOR) - self.assertIs(Method.get('get'), Method.GET) + self.assertIs(Method.get('GET'), Method.GET) self.assertIs(TransportProtocol.get('tcp'), TransportProtocol.tcp) # Integer paths, which do. diff --git a/tests/const/test_const_enum_no_mint.py b/tests/const/test_const_enum_no_mint.py index 2cc15e03c3..4d2f151788 100644 --- a/tests/const/test_const_enum_no_mint.py +++ b/tests/const/test_const_enum_no_mint.py @@ -2100,14 +2100,27 @@ def test_registered_lookups_are_still_unaffected(self) -> None: """A word IANA already assigned still resolves to the same, genuinely-registered, identical member every time -- this conversion only changes what happens for a word that is not one of - those.""" + those. + + ``Method.get`` is probed with its exact registered casing + (``'GET'``), not the lower-cased ``'get'`` this test used before + GitHub issue #896: ``Command``'s FTP command codes stay + case-insensitive per :rfc:`959#section-4.1`, but the HTTP method + token :rfc:`9110#section-9.1` covers is case-sensitive, so + ``Method.get('get')`` no longer resolves to :attr:`Method.GET` -- + see :class:`BespokeGetUnchangedTests`'s + ``test_method_get_is_now_case_sensitive`` for that behaviour + directly. ``Method('GET')`` (the constructor, reaching + :meth:`Method._missing_` rather than :meth:`Method.get`) is + untouched either way, since #896 is scoped to ``get`` alone. + """ from pcapkit.const.ftp.command import Command from pcapkit.const.http.method import Method self.assertIs(Command('RETR'), Command.RETR) # type: ignore[attr-defined] self.assertIs(Command.get('retr'), Command.RETR) # type: ignore[attr-defined] self.assertIs(Method('GET'), Method.GET) # type: ignore[attr-defined] - self.assertIs(Method.get('get'), Method.GET) # type: ignore[attr-defined] + self.assertIs(Method.get('GET'), Method.GET) # type: ignore[attr-defined] def test_all_three_carry_the_registry_protocol(self) -> None: """#842's ruling is that ``get``/``get_all``/``register``/ @@ -2235,14 +2248,26 @@ class BespokeGetUnchangedTests(unittest.TestCase): :class:`~pcapkit.const.http.method.Method` and :class:`~pcapkit.const.pcapng.option_type.OptionType` keep their own hand-written ``get()`` in PR 1, because each does real dispatch the - base's generic ``get()`` does not replicate and at least two of the three - are pinned, tested behaviour already: :class:`Command`/:class:`Method` - resolve case-insensitively (GitHub issues #582/#583 -- ``Command. - get('abor')`` must return :attr:`Command.ABOR`, not raise, and - ``Method.get('Get')`` must return :attr:`Method.GET` with its - :attr:`safe`/:attr:`idempotent` attributes intact), which the base's - plain ``_member_map_``/``_value2member_map_`` lookup does not do -- - swapping in the base would silently reintroduce #582/#583. + base's generic ``get()`` does not replicate. :class:`Command` resolves + case-insensitively (GitHub issue #582 -- ``Command.get('abor')`` must + return :attr:`Command.ABOR`, not raise), which the base's plain + ``_member_map_``/``_value2member_map_`` lookup does not do -- swapping in + the base would silently reintroduce #582. + + :class:`Method` used to resolve case-insensitively the same way (#583), + but GitHub issue #896 retired that: RFC 9110 Section 9.1 makes the HTTP + method token case-sensitive, unlike FTP's command codes (RFC 959 Section + 4.1), so ``Method.get('Get')`` no longer returns :attr:`Method.GET` -- + see ``test_method_get_is_now_case_sensitive`` below, which replaces the + case-insensitive pin this class used to carry for it. ``Method`` still + keeps its own ``get()`` rather than the base's, because it still needs to + build an unregistered member preserving the caller's own casing on a + miss (the base's generic ``get()`` only ever raises or falls back to an + already-registered ``default`` for a ``str`` key, per + :meth:`~pcapkit.corekit.enum.EnumRegistry.get`'s own docstring) -- what + #896 changed is only whether the lookup that precedes that fallback is + case-sensitive. + :class:`OptionType`'s ``get()`` does its own multi-namespace dispatch via :attr:`__members_ns__` with no base equivalent at all. These are regression guards, not new coverage. @@ -2261,13 +2286,40 @@ def test_command_get_is_still_case_insensitive(self) -> None: with self.subTest(key=key): self.assertIs(Command.get(key), Command.RETR) # type: ignore[attr-defined] - def test_method_get_is_still_case_insensitive(self) -> None: + def test_method_get_is_now_case_sensitive(self) -> None: + """GitHub issue #896: only the exact registered casing resolves. + + Replaces this class's own ``test_method_get_is_still_case_ + insensitive``, whose title and body pinned the opposite -- that + ``Method.get('Get')``/``Method.get('get')`` resolved to + :attr:`Method.GET`. RFC 9110 Section 9.1 makes the method token + case-sensitive, so a differently-cased probe now builds its own + unregistered member (preserving the caller's casing, same + convention as :meth:`Method._unregistered_member`) rather than + resolving to :attr:`Method.GET`. + """ from pcapkit.const.http.method import Method - for key in ('GET', 'Get', 'get', 'gEt'): + self.assertIs(Method.get('GET'), Method.GET) # type: ignore[attr-defined] + self.assertTrue(Method.get('GET').safe) + + before = len(Method.__members__) + for key in ('Get', 'get', 'gEt'): with self.subTest(key=key): - self.assertIs(Method.get(key), Method.GET) # type: ignore[attr-defined] - self.assertTrue(Method.get('Get').safe) + probed = Method.get(key) # type: ignore[attr-defined] + self.assertIsNot(probed, Method.GET) + # Building the pseudo-member never registers it -- 'GET' + # itself is the only real member these differently-cased + # names could collide with, and membership does not grow. + self.assertEqual(len(Method.__members__), before) + # The caller's own casing survives on the value; only the + # (unregistered) member's name is canonicalised. + self.assertEqual(probed.value, key) + self.assertEqual(probed.name, key.upper()) + # Neither attribute a bare wire token cannot supply is + # fabricated -- same as any other unregistered member. + self.assertFalse(probed.safe) + self.assertFalse(probed.idempotent) def test_optiontype_get_namespace_dispatch_is_unchanged(self) -> None: from pcapkit.const.pcapng.option_type import OptionType diff --git a/tests/const/test_const_method_case_sensitive_896_unit.py b/tests/const/test_const_method_case_sensitive_896_unit.py new file mode 100644 index 0000000000..c481090c46 --- /dev/null +++ b/tests/const/test_const_method_case_sensitive_896_unit.py @@ -0,0 +1,287 @@ +# -*- coding: utf-8 -*- +"""``Method.get`` must be case-sensitive, per RFC 9110 Section 9.1. + +GitHub issue #896: :meth:`pcapkit.const.http.method.Method.get` used to +upper-case its ``key`` before looking it up (``name = key.upper()``, then +``name not in Method._member_map_``), so ``Method.get('get')`` resolved to +the same object as ``Method.get('GET')`` -- :attr:`Method.GET`. RFC 9110 +Section 9.1 says the opposite is required:: + + The method token is case-sensitive because it might be used as a gateway + to object-based systems with case-sensitive method names. By convention, + standardized methods are defined in all-uppercase US-ASCII letters. + +so a request whose method token is the lower-case ``get`` is a distinct, +non-standardised method from the registered ``GET`` -- conflating the two +was the defect. Contrast :class:`~pcapkit.const.ftp.command.Command`'s own +equivalent override, which stays case-insensitive deliberately: RFC 959 +Section 4.1 says FTP's four-letter command codes *are* to be treated +identically regardless of case, so ``Command.get`` is untouched by this +issue and is repinned below (:meth:`MethodGetCaseSensitivityTests. +test_command_get_stays_case_insensitive`) so a future change cannot quietly +fold the two together again. + +**What the fix is not.** The issue's own suggested mechanism was to delete +``Method.get`` outright and let :meth:`~pcapkit.corekit.enum.EnumRegistry.get` +(the base every ``EnumRegistry`` mixin shares) handle it instead. Measured +directly against the base before doing that: for a ``str`` key that matches +neither a member name nor an already-registered value, and with no +``default`` supplied, ``EnumRegistry.get`` raises :exc:`KeyError` -- it never +builds an unregistered member the way ``Method._missing_`` does for the +*constructor* path. Confirmed on :class:`~pcapkit.const.ftp.command.FEATCode` +(no bespoke ``get`` of its own, so it already runs the base unmodified): +``FEATCode.get('totally-unknown-thing')`` raises ``KeyError``, while +``FEATCode('totally-unknown-thing')`` -- the constructor, reaching +``_missing_`` -- resolves to an unregistered member. Deleting ``Method.get`` +outright would therefore have turned ``Method.get('get')`` from "resolves to +``GET``" into "raises ``KeyError``", not into "resolves to an unregistered +member preserving ``'get'``" as the issue's own *expected behaviour* section +asks for -- and as :mod:`tests.protocols.application.test_http_unit` and +:mod:`tests.const.test_const_enum_no_mint` already require elsewhere in this +change. So the fix keeps ``Method``'s own ``get`` override -- it still needs +to build that unregistered member on a miss, which the base's ``get`` does +not do for a ``str`` key -- and only removes the ``.upper()`` folding from +the *match* it performs before falling through to that fallback. The +``_unregistered_member`` call it falls through to still canonicalises the +*name* (the identifier) to upper case, exactly as :meth:`Method._missing_` +does; only the *value* -- and now, the match against a *registered* name -- +keeps the caller's exact casing. + +This module also pins the crawler and the generated file against each +other, the way :mod:`tests.const.test_const_enum_get`'s own +``test_the_vendor_template_still_emits_the_fix`` does for a different +registry: :mod:`pcapkit.const` is generated from :mod:`pcapkit.vendor`, so a +fix applied only to the generated file is silently discarded by the next +crawl. Unlike that sibling test, :class:`~pcapkit.const.http.method.Method`'s +own ``get`` is rendered from a template private to +:mod:`pcapkit.vendor.http.method` itself (its own ``LINE`` lambda), not from +the shared one in :mod:`pcapkit.vendor.default` -- so the parity check here +renders that module's own template with a CSV fixture reconstructed from the +40 members the committed file already declares, offline, and compares the +result against the committed file byte for byte, rather than against a +synthetic single-member sample. + +""" +from __future__ import annotations + +import csv +import inspect +import io +import re +import unittest + +from tests._support import purge_modules + + +def _reconstruct_fixture_csv() -> 'str': + """Rebuild the IANA-shaped CSV that reproduces the committed generated + file byte for byte, by reading the members and comments already in + :mod:`pcapkit.const.http.method` back into ``Method Name,Safe, + Idempotent,Reference`` rows. + + This is the offline substitute for a live crawl against + ``https://www.iana.org/assignments/http-methods/methods.csv``, which + this change must not perform. Faithfulness is what + :meth:`MethodGetCaseSensitivityTests. + test_vendor_template_matches_the_generated_module` below checks: feeding + this fixture back through :meth:`~pcapkit.vendor.http.method.Method. + context` must reproduce the committed generated file byte for byte, not + merely render something self-consistent. + + Returns: + CSV text, ``\\r\\n``-terminated, matching what + :meth:`~pcapkit.vendor.default.Vendor.request` expects. + + """ + import pcapkit.const.http.method as const_method + + src = inspect.getsource(const_method) + + start_marker = (" def __repr__(self) -> 'str':\n" + " return f'<{self.__class__.__name__}.{self._value_}>'\n\n") + start = src.index(start_marker) + len(start_marker) + end_marker = "\n @classmethod\n def _unregistered_member" + end = src.index(end_marker, start) + enum_block = src[start:end] + + rows = [] # type: list[tuple[str, str, str, str]] + for chunk in enum_block.split('\n\n'): + lines = [line for line in chunk.split('\n') if line.strip()] + comment_lines = [line for line in lines if line.strip().startswith('#:')] + code_lines = [line for line in lines if not line.strip().startswith('#:')] + if not code_lines: + continue + match = re.match(r"^\s*(\w+) = '([^']*)', (True|False), (True|False)$", code_lines[0]) + assert match is not None, code_lines[0] + _name, value, safe_flag, idem_flag = match.groups() + + desc = ' '.join(line.strip()[3:] for line in comment_lines) + assert desc.startswith(value), (desc, value) + rest = desc[len(value):].lstrip(' ') + + rfcs = '' + if rest: + groups = re.findall(r'\[([^\[\]]*)\]', rest) + assert ''.join(f'[{g}]' for g in groups) == rest, (groups, rest) + parts = [] # type: list[str] + for group in groups: + rfc_match = re.match(r'^:rfc:`(\d+)(?:#([a-z0-9.\-]+))?`$', group) + if rfc_match is None: + parts.append('[' + group.replace(' ', '_') + ']') + continue + num, section = rfc_match.groups() + if section is None: + parts.append(f'[RFC{num}]') + else: + section = section[len('section'):].lstrip('-').replace('-', ' ') + parts.append(f'[RFC{num}, Section {section}]') + rfcs = ''.join(parts) + + rows.append((value, + 'yes' if safe_flag == 'True' else 'no', + 'yes' if idem_flag == 'True' else 'no', + rfcs)) + + buf = io.StringIO() + writer = csv.writer(buf, lineterminator='\r\n') + writer.writerow(['Method Name', 'Safe', 'Idempotent', 'Reference']) + writer.writerows(rows) + return buf.getvalue() + + +class MethodGetCaseSensitivityTests(unittest.TestCase): + """The direct repro and fix for GitHub issue #896.""" + + def setUp(self) -> None: + purge_modules(['pcapkit']) + + def test_get_exact_case_resolves_the_standardised_member(self) -> None: + """The unaffected half: the registered, all-uppercase casing every + member is actually declared under still resolves to it.""" + from pcapkit.const.http.method import Method + + self.assertIs(Method.get('GET'), Method.GET) + self.assertTrue(Method.get('GET').safe) + self.assertTrue(Method.get('GET').idempotent) + + def test_get_lowercase_does_not_resolve_to_the_standard_member(self) -> None: + """The issue's own repro: ``Method.get('get')`` must stop resolving + to :attr:`Method.GET`. + + Fails against the pre-fix override (``name = key.upper()`` folded + into the ``_member_map_`` membership test): there, + ``Method.get('get') is Method.GET`` is :data:`True`. + """ + from pcapkit.const.http.method import Method + + probed = Method.get('get') + self.assertIsNot(probed, Method.GET) + self.assertNotEqual(probed, 'GET') + + def test_unregistered_member_preserves_the_callers_casing(self) -> None: + """The value of the unregistered member ``get('get')`` now builds is + exactly what the caller passed -- never upper-cased -- matching the + convention :meth:`~pcapkit.const.http.method.Method. + _unregistered_member` documents and :class:`~pcapkit.const.ftp. + command.FEATCode` already follows. Only the *name* (the identifier, + not the value) is canonicalised. + """ + from pcapkit.const.http.method import Method + + before = len(Method.__members__) + for key in ('get', 'Get', 'gEt', 'GeT'): + with self.subTest(key=key): + probed = Method.get(key) + self.assertEqual(probed.value, key) + self.assertEqual(str(probed), key) + self.assertEqual(probed.name, key.upper()) + # Building the pseudo-member never registers it -- 'GET' + # itself stays the only real member this loop's names could + # collide with, and membership does not grow. + self.assertEqual(len(Method.__members__), before) + + # Two calls naming the same method in different case build results + # that are genuinely unequal -- each is exactly its own caller's + # casing, not folded onto a shared cached member. + self.assertNotEqual(Method.get('frob'), Method.get('FROB')) + self.assertNotEqual(Method.get('frob'), Method.get('Frob')) + + def test_command_get_stays_case_insensitive(self) -> None: + """The registry #896 explicitly leaves alone. + + RFC 959 Section 4.1: FTP command codes are to be treated + identically regardless of case, so + :meth:`~pcapkit.const.ftp.command.Command.get` keeps folding case -- + pinned here so a future edit to the sibling ``Method`` fix cannot + silently carry the case-sensitive change over to ``Command`` too. + """ + from pcapkit.const.ftp.command import Command + + for key in ('RETR', 'retr', 'ReTr', 'rEtR'): + with self.subTest(key=key): + self.assertIs(Command.get(key), Command.RETR) + + def test_vendor_template_matches_the_generated_module(self) -> None: + """A regeneration must not undo the fix, and must not have been + needed to introduce it in the first place. + + Renders :mod:`pcapkit.vendor.http.method`'s own template with a CSV + fixture reconstructed from the *committed* generated file (no + network access), and requires the result to match that same file + byte for byte -- proving the crawler template, not just the + generated file alone, carries the fix. + """ + import pcapkit.const.http.method as const_method + import pcapkit.vendor.http.method as vendor_method + + csv_text = _reconstruct_fixture_csv() + + inst = object.__new__(vendor_method.Method) + inst.NAME = 'Method' + inst.DOCS = vendor_method.Method.__doc__ + data = inst.request(csv_text) + rendered_ctx = inst.context(data) + + temp_ctx = [] # type: list[str] + for line in rendered_ctx.splitlines(): + if line: + if line.strip(): + temp_ctx.append(line.rstrip()) + else: + temp_ctx.append(line) + rendered = '\n'.join(temp_ctx) + '\n' + + with open(const_method.__file__, encoding='utf-8') as handle: + committed = handle.read() + + self.assertEqual(rendered, committed) + + # 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. + 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) + + def test_regenerating_twice_is_byte_reproducible(self) -> None: + """The same fixture fed to the same template twice must render + identically -- the reproducibility the vendor crawler promises for + any fixed input, checked directly rather than assumed.""" + import pcapkit.vendor.http.method as vendor_method + + csv_text = _reconstruct_fixture_csv() + + def render() -> 'str': + inst = object.__new__(vendor_method.Method) + inst.NAME = 'Method' + inst.DOCS = vendor_method.Method.__doc__ + data = inst.request(csv_text) + return inst.context(data) + + self.assertEqual(render(), render()) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/application/test_http_unit.py b/tests/protocols/application/test_http_unit.py index fef508f0e6..7dba7830bf 100644 --- a/tests/protocols/application/test_http_unit.py +++ b/tests/protocols/application/test_http_unit.py @@ -1477,23 +1477,51 @@ def test_httpv1_id_make_data_and_request_construction(self) -> None: with self.assertRaises(ProtocolError): proto._read_http_header(b'BAD nope nope\r\nHost: example') - def test_method_get_is_case_insensitive(self) -> None: - """``Method.get`` tested the raw key and registered the upper-cased one, - so a mixed-case method raised ``TypeError`` -- #583, item 1. - - The same mismatch as #582 in :mod:`pcapkit.const.ftp.command`. Resolving - to the existing member matters beyond not crashing: a duplicate - registered alongside ``GET`` would carry neither its ``safe`` nor its - ``idempotent`` attribute. + def test_method_get_is_now_case_sensitive(self) -> None: + """``Method.get`` used to test the upper-cased key, so a mixed-case + method resolved to the same registered member regardless of its own + casing -- #583, item 1, at the time framed the same way as #582's fix + in :mod:`pcapkit.const.ftp.command` (`Command.get` stays that way; see + its own RFC 959 Section 4.1 rationale). GitHub issue #896 retired that + for ``Method`` specifically: RFC 9110 Section 9.1 makes the HTTP + method *token* case-sensitive -- "standardized methods are defined in + all-uppercase US-ASCII letters" by convention, not by requirement -- + so a request whose method token is ``get`` is a distinct, + non-standardised method from ``GET``, and conflating the two through + ``.get()`` was the defect. + + This replaces the class's own former pin of the opposite behaviour + (same method name, opposite body) rather than adding a second test + beside it, since the two claims cannot both be true. """ from pcapkit.const.http.method import Method - for key in ('GET', 'Get', 'get', 'gEt'): + # The exact registered casing still resolves. + self.assertIs(Method.get('GET'), Method.GET) + self.assertTrue(Method.get('GET').safe) + + # A differently-cased token no longer resolves to the registered + # member -- each becomes its own unregistered member instead, + # carrying the caller's own casing on its value (same convention + # :meth:`Method._unregistered_member` documents) and never + # registering. + before = len(Method.__members__) + for key in ('Get', 'get', 'gEt'): with self.subTest(key=key): - self.assertIs(Method.get(key), Method.GET) - + probed = Method.get(key) + self.assertIsNot(probed, Method.GET) + self.assertNotEqual(probed, 'GET') + self.assertEqual(probed, key) + # Building the pseudo-member never registers it -- 'GET' + # itself is the only real member these differently-cased + # names could collide with, and membership does not grow. + self.assertEqual(len(Method.__members__), before) + + # The constructor path (``Method(...)`` -> ``_missing_``) is a + # separate mechanism from ``.get()`` and #896 does not touch it -- + # it still folds case, exactly as before. self.assertIs(Method('Get'), Method.GET) - self.assertTrue(Method.get('Get').safe) + self.assertEqual([name for name in Method._member_map_ if name.upper() == 'GET'], ['GET'])