Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions docs/source/conventions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/JarryShaw/PyPCAPKit/issues/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
Expand Down
36 changes: 25 additions & 11 deletions pcapkit/const/http/method.py
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
36 changes: 25 additions & 11 deletions pcapkit/vendor/http/method.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}':
Expand Down
12 changes: 10 additions & 2 deletions tests/const/test_const_enum_builtin_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
80 changes: 66 additions & 14 deletions tests/const/test_const_enum_no_mint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``/
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
Loading
Loading