fix(const): Method.get never checks values, so BASELINE-CONTROL and VERSION-CONTROL mis-resolve

Describe the bug

Method.get does no value lookup, so the two IANA methods whose member name differs from their value fail to resolve and come back with wrong metadata. A real HTTP/1 capture can hit this.

BASELINE_CONTROL -> value 'BASELINE-CONTROL'  (registry: safe=False idempotent=True)
VERSION_CONTROL  -> value 'VERSION-CONTROL'   (registry: safe=False idempotent=True)

Method.get('BASELINE-CONTROL')  is Method.BASELINE_CONTROL ?  False    idempotent=False   <- wrong
Method('BASELINE-CONTROL')      is Method.BASELINE_CONTROL ?  True
Method.get('VERSION-CONTROL')   is Method.VERSION_CONTROL  ?  False    idempotent=False   <- wrong
Method('VERSION-CONTROL')       is Method.VERSION_CONTROL  ?  True

Both are RFC 3253 methods. They are the only two of the 40 members whose name and value differ, because the hyphen cannot appear in a Python identifier.

Why a capture reaches it. pcapkit/protocols/application/httpv1.py:60:

_RE_METHOD = re.compile(rb"(?P<method>[A-Z][A-Z-]*)\Z")

That pattern admits hyphens, so a request line carrying BASELINE-CONTROL matches, reaches the lookup at httpv1.py:434, and is reported with idempotent=False where the registry says True. The parse succeeds; only the metadata is wrong, which is the harder kind to notice.

The mechanism. The override at pcapkit/const/http/method.py checks _member_map_ (names) and then falls through to building an unregistered member. It never consults _value2member_map_. The base EnumRegistry.get does exactly that fall-through — pcapkit/corekit/enum.py:368-376 — which is why the constructor resolves these two correctly and get does not.

Expected behavior

Method.get('BASELINE-CONTROL') should return Method.BASELINE_CONTROL, with the registry's own safe/idempotent values.

The fix that comes for free. Rather than adding a hand-rolled value check, delegate and keep the override only for the fallback:

try:
    return super().get(key)
except KeyError:
    return Method._unregistered_member(key, key.upper())

That preserves case-sensitivity (per RFC 9110 §9.1, settled in #896), preserves the caller's-casing unregistered member, and picks up the base's _value2member_map_ match — closing this defect without a special case. It must go in the crawler template (pcapkit/vendor/http/method.py, which carries its own LINE lambda rather than using pcapkit/vendor/default.py's), not in the generated file, or the next regeneration discards it.

System information

Measured on origin/main at 0981d771d, repo venv, PYTHONSAFEPATH=1.

Additional context

Found by the cross-review of #907, which fixed Method.get's case folding for #896. Not a regression from #907 — .upper() is a no-op on an already-uppercase token, so the behaviour is identical before and after — but #907 touches the exact line that would fix it, so it is recorded here rather than silently carried.

A second, related inconsistency in the same file, deliberately out of #896's scope and noted so it is not lost: _missing_ still folds case, and its docstring still reads "Matched case-insensitively against the canonical upper-case member names" with no RFC caveat — fifteen lines from a get whose docstring now says RFC 9110 requires the opposite. Method('get') still resolves to GET while Method.get('get') no longer does. That split was ruled deliberate, but a reader of the one file gets two contradictory rationales and no pointer between them. Whoever fixes the value lookup should add that pointer, in the template.

Related: #903 (the registry-wide case-sensitivity audit), #877 (the EnumLookup/EnumRegistry split, which is what makes delegating to super().get clean).

Checkable blocker, recorded so this label is not taken on trust: PR #907, and preferably #906 too.

#907 is open and rewrites the exact get body this defect lives in. Fixing it here first would conflict with that branch, and fixing it there would widen a PR that already has a GOOD TO GO verdict on 99e430cc6.

The recommended fix also reads better once #906 lands, because delegating to super().get(key) is cleaner when the base is the bare EnumLookup rather than EnumRegistry — and #906 renames the cross-reference, so writing :meth:~pcapkit.corekit.enum.EnumRegistry.get`` here now would go stale.

gh pr view 907 -R JarryShaw/PyPCAPKit --json state -q .state
gh pr view 906 -R JarryShaw/PyPCAPKit --json state -q .state

Unblocks when #907 merges. #906 is a preference, not a hard gate — if it slips, this can proceed against EnumRegistry.get and be renamed later.

The defect is real and live either way, so it must not be lost to the sequencing: a capture carrying BASELINE-CONTROL parses today with idempotent=False where the IANA registry says True.

Unblocked — both blockers merged. #907 landed as ef4ba1afd and #906 as c50f442db, so the get body this defect lives in is settled and the base is now the bare EnumLookup. Verified:

$ gh pr view 907 --json state -q .state   ->  MERGED  ef4ba1afd
$ gh pr view 906 --json state -q .state   ->  MERGED  c50f442db

Removing blocked, marking wip, worker dispatched. The defect is unchanged and still live on e38d695ae.

One correction to the fix this issue proposes, found while checking the sequencing. The recommended body is return super().get(key), but Method.get is declared @staticmethod — as are the Command, OptionType and AppType overrides — while the base EnumLookup.get is a @classmethod (pcapkit/corekit/enum.py:394). Zero-argument super() inside a @staticmethod has no first argument to bind and raises RuntimeError: super(): no arguments, so the snippet above cannot work as written.

The override has to become a @classmethod for the delegation to be available at all. That is not a workaround — it is the shape the base already uses, and #913 sets the precedent: its new FEATCode.get is @classmethod def get(cls, key, default=NO_DEFAULT) ending in return super().get(key, default). Following it also picks up the default=NO_DEFAULT signature, where this override currently takes default: 'Optional[str]' = None, so the worker should say explicitly whether changing that default is a caller-visible change and keep it out of scope if it is.

Callers are unaffected by the staticmethod → classmethod switch itself: Method.get('X') binds identically either way.

PR up: #915 (c59ee55f9). Cross-review dispatched on opus (author sonnet), briefed to falsify.

The fix took the @classmethod route this issue's own snippet could not — Method.get now delegates to the base and catches KeyError for the unregistered-member fallback. Verified by me in a fresh process rather than from the hand-back:

BASELINE-CONTROL resolves to canonical : True | idempotent = True
VERSION-CONTROL  resolves to canonical : True | idempotent = True
tables before/after (no minting)       : (40, 40) (40, 40)
descriptor kind                        : classmethod
case-sensitivity preserved (#896)      : Method.get("get") is Method.GET -> False
caller casing preserved (#860)         : value='frob' name='FROB'

So both defects close and neither settled invariant regresses. The fix went into the crawler template as well as the generated file, and an end-to-end test drives _read_http_header with a real BASELINE-CONTROL request line rather than only calling get() directly — which is the path a capture actually takes, through httpv1.py:60's hyphen-admitting _RE_METHOD.

One design call the author made deliberately, and I want it looked at rather than waved through. It kept the existing default: 'Optional[str]' = None instead of adopting the base's default=NO_DEFAULT, on the reasoning that the contracts genuinely differ — the base's default must name an already-registered value, while this override's supplies the value of a freshly-minted unregistered member. That is coherent, but it means that once #913 also lands, pcapkit/const/ftp/command.py's FEATCode.get and this file's Method.get will carry different default contracts. Defensible or a divergence that bites later is a judgement, so the reviewer is deciding it.

Two things the reviewer is also attacking: whether the staticmethod → classmethod switch is truly caller-invisible under introspection (the author's own test hit AttributeError: 'function' object has no attribute '__func__' on the old tree, so the difference is reachable), and whether except KeyError is too narrow given the base raises ValueError on its non-str path.

Reported honestly by the author and worth recording: of its 12 new tests, 8 fail against unmodified main and 4 pass on both trees by design, pinning the already-settled #896 and #860 invariants.

fix(const): value-lookup in Method.get, so BASELINE-CONTROL/VERSION-C…

fix(corekit): reparent the unblocked non-registry enums onto EnumLook…

docs(vendor,const): convert bare citations to the issue role in crawl…

Navigation Menu

Repository navigation

fix(const): Method.get never checks values, so BASELINE-CONTROL and VERSION-CONTROL mis-resolve #908

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

fix(const): Method.get never checks values, so BASELINE-CONTROL and VERSION-CONTROL mis-resolve #908

Description

Activity

JarryShaw commented on Sep 29, 2026

JarryShaw commented on Sep 29, 2026

JarryShaw commented on Sep 29, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions