Skip to content

fix(const): accept RFC 2113's Router Alert 0 and IPX's own socket 0 - #503

Merged
JarryShaw merged 3 commits into
mainfrom
fix/492-const-enum-zero-values
Sep 19, 2026
Merged

JarryShaw merged 3 commits into
mainfrom
fix/492-const-enum-zero-values

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Fixes #492 — three pcapkit/const/** lookup defects, each fixed in both the generated file and the pcapkit/vendor/** generator that produces it.

1. HIGH — RouterAlert(0), and the root cause is better than the symptom

RFC 2113 §2.1 defines exactly one value, 0 ("Router shall examine packet"), and it is what IGMP, RSVP and MLD put on the wire. RouterAlert(0) raised ValueError.

The obvious fix would have been to add the member. The actual cause is in the crawler: pcapkit/vendor/ipv4/router_alert.py opened with next(reader) # header, but this IANA registry ships with no header row — its first row is the value-0 entry. Verified against the live CSV myself:

$ curl -sL https://www.iana.org/assignments/ip-parameters/ipv4-router-alert-option-values.csv | head -3
0,Router shall examine packet,[RFC2113]
1-32,Aggregated Reservation Nesting Level,[RFC3175]
33-64,QoS NSLP Aggregation Levels 0-31,[RFC5974]

So the generator had been silently discarding RFC 2113's only defined value on every regeneration. Removing that next(reader) and re-running the real entrypoint (python -m pcapkit.vendor.__main__ ipv4.router_alert) against the live registry regenerates the const file with exactly one new member and nothing else changed — which is the proof the vendor half actually propagates, rather than an assertion that it should.

before: RouterAlert(0) -> ValueError: 0 is not a valid RouterAlert
after:  RouterAlert(0) -> <RouterAlert.Router_shall_examine_packet: 0>

A standards-compliant packet now parses: 4600001c...9404000011000000 → options[148].alert == 0.

2. MEDIUM — Socket(0)

0x0000 is an ordinary unspecified socket and IPX's own class default for dst/src, so the class could not construct itself with its documented defaults.

before: bytes(IPX(payload=b'\xde\xad\xbe\xef')) -> ValueError: 0 is not a valid Socket
after:  -> 0000002200000000...  (round-trips)
        Socket(0) -> <Socket.Unspecified: 0>

The vendor side here is a documented workaround, not a root-cause fix, and the PR says so. The Wikipedia table this crawler scrapes has disappeared from the live page independently of this issue, and requests' default User-Agent now gets a 403 from Wikipedia regardless. Both are pre-existing breakages outside this issue's scope. Rather than depend on a scrape that currently returns nothing useful, process() prepends the Unspecified = 0x0000 member unconditionally, with a comment explaining why. Those two scraper breakages are left unfixed and want their own issue.

3. LOW and latent — _missing_ without @classmethod

ResponseKind and GroupingInformation in ftp/return_code.py defined _missing_ as a plain method, so aenum's cls._missing_(value) bound the lone argument to cls:

before: ResponseKind(7) -> TypeError: _missing_() missing 1 required positional argument: 'value'
after:  ResponseKind(7) -> <ResponseKind.Unknown_7: 7>
        GroupingInformation(9) -> <GroupingInformation.Unknown_9: 9>

Every unknown value, not just 0. Fixed in the const file and in the LINE template in vendor/ftp/return_code.py. Rated LOW because these two classes are imported by no runtime code — only the codegen copy — so nothing was broken in practice.

Tests

tests/const/test_const_enum_lookup.py sweeps all 111 IntEnum classes under pcapkit.const, asserting Enum(0) matches what each registry says it should be — 105 accept, 6 correctly reject (ReturnCode and StatusCode are 3-digit codes; ConformanceRequirement, CauseCode, Parameter, MNIDSubtype are registries beginning at 1). That arithmetic reconciles with the pre-fix measurement of 102 accepting and 9 rejecting: the three fixed enums moved across.

It also asserts all 109 _missing_ overrides are classmethods, and parses the RFC 2113 packet end to end.

A second commit, for test isolation

The new sweep passed standalone but failed under full-suite ordering, because tests/cli/test_main.py leaves stubbed pcapkit.utilities.compat/exceptions in sys.modules and this sweep is the first thing to import every pcapkit.const submodule, including ones nothing else touches. Fixed with purge_modules(['pcapkit']) in setUp/setUpClass, the convention every other test file here already uses.

I verified that is a real fix rather than a coincidence — neutering the five purge_modules calls and running pytest tests/cli tests/const/test_const_enum_lookup.py:

3 failed, 6 passed, 4 errors

With the fix, the same command: 13 passed, 111 subtests passed.

Verification

Revert-proof. Restoring next(reader), removing both members and stripping the @classmethods:

9 failed, 2 passed, 107 subtests passed
  SUBFAILED(enum='pcapkit.const.ipv4.router_alert.RouterAlert')
  SUBFAILED(enum='pcapkit.const.ipx.socket.Socket')
  FAILED ...test_every_missing_is_a_classmethod
  FAILED ...test_ftp_return_code_missing_methods_are_classmethods
  FAILED ...test_parses_igmp_over_router_alert_zero

Restored: 7 passed, 111 subtests passed.

Also: test_ipx_unit.py 3 passed; test_ipv4_unit.py 12 passed / 16 subtests; full unit tier 921 passed, 8 skipped, 1859 subtests, 0 failed. mypy on the six touched source files shows only the same 11 pre-existing errors, confirmed identical against the unmodified originals; the new test file is mypy-clean.

Three lookup defects under pcapkit/const, all reproduced on the wire:

- pcapkit/const/ipv4/router_alert.py, pcapkit/vendor/ipv4/router_alert.py:
  RouterAlert(0) raised ValueError, rejecting RFC 2113's only defined
  value (0 = "Router shall examine packet"), the one IGMP/RSVP/MLD
  actually send. Root cause found by fetching IANA's live CSV directly:
  the registry ships with NO header row -- its first row IS the value-0
  entry -- so the vendor crawler's unconditional `next(reader) # header`
  silently discarded it. Removed that skip; re-ran the real
  `pcapkit-vendor ipv4.router_alert` crawler against the live registry
  and it now regenerates the const file with exactly one new member,
  `Router_shall_examine_packet = 0`, nothing else changed.

- pcapkit/const/ipx/socket.py, pcapkit/vendor/ipx/socket.py: Socket(0)
  raised ValueError, even though 0x0000 is IPX's own class default for
  the dst/src address field, so `bytes(IPX(payload=...))` crashed on
  its own defaults. Added an `Unspecified = 0x0000` member. The
  Wikipedia table the vendor crawler scrapes for this one has
  meanwhile lost the well-known-socket-ranges table entirely (only a
  generic 3-row header-format table remains under `wikitable`,
  confirmed live) and `requests`' default User-Agent now gets a 403
  from Wikipedia regardless -- both pre-existing and out of scope, so
  the vendor fix prepends the 0x0000 entry unconditionally rather than
  relying on the scrape to carry it.

- pcapkit/const/ftp/return_code.py, pcapkit/vendor/ftp/return_code.py:
  `ResponseKind._missing_` and `GroupingInformation._missing_` were
  plain methods, so aenum's `cls._missing_(value)` bound `value` to
  `cls` and every unregistered value raised TypeError instead of
  extending the enum. Added the missing `@classmethod`.

tests/const/test_const_enum_lookup.py: sweeps all 111 IntEnum classes
under pcapkit.const, asserting Enum(0) matches the registry (105
accept, 6 correctly reject -- ReturnCode, StatusCode,
ConformanceRequirement, CauseCode, Parameter, MNIDSubtype); asserts
every one of the tree's 109 `_missing_` overrides is a classmethod;
and parses the RFC 2113 repro packet end-to-end.

Verified before/after on the actual repros: RouterAlert(0) and the
28-byte IGMP-over-Router-Alert packet both raised ValueError before,
now parse; bytes(IPX(payload=b'\xde\xad\xbe\xef')) raised before, now
round-trips; ResponseKind(7)/(0) and GroupingInformation(9) raised
TypeError before, now extend correctly. tests/const/: 7 passed, 111
subtests. tests/protocols/internet/test_ipx_unit.py: 3 passed.
tests/protocols/internet/test_ipv4_unit.py: 12 passed, 16 subtests.
mypy on all six touched source files: identical pre-existing errors
only (11, all Liskov/typeshed issues predating this change).
…order

Passed standalone but failed inside the full suite: tests/const/test_const_enum_lookup.py
is the first module to import every submodule under pcapkit.const, including ones nothing
else touches (e.g. pcapkit.const.reg.apptype). When tests/cli/test_main.py runs first, its
stubbed pcapkit.utilities.compat/exceptions modules are still sitting in sys.modules, and
the fresh imports my sweep triggers pick up the stubs instead of the real files --
ImportError: cannot import name 'show_flag_values' / 'SeekError' (unknown location).

Every other module in this suite guards against exactly this by calling
tests._support.purge_modules(['pcapkit']) before touching pcapkit; this one didn't. Added
it to setUp/setUpClass in all three test classes, and moved the pcapkit.const import inside
_iter_const_int_enums() so it re-imports fresh after the purge rather than reusing a
module-level reference cached at collection time.

Reproduced with `pytest tests/cli tests/const/test_const_enum_lookup.py` (3 failed, 4
errored); passes the same way after this fix (13 passed). Full required set unaffected:
tests/const/: 7 passed, 111 subtests. tests/protocols/internet/test_ipx_unit.py: 3 passed.
tests/protocols/internet/test_ipv4_unit.py: 12 passed, 16 subtests.
@JarryShaw
JarryShaw merged commit 93fd8f8 into main Sep 19, 2026
23 checks passed
@JarryShaw
JarryShaw deleted the fix/492-const-enum-zero-values branch September 19, 2026 04:13
JarryShaw added a commit that referenced this pull request Sep 19, 2026
#503 added the missing Socket(0x0000) member, so IPX now constructs and
parses. Both entries recorded it as degrading, and this harness asserts a
recorded degrade still degrades -- so leaving them in fails, by design:

  AssertionError: True != False : internet/IPX_in_IP was recorded as
  degrading (...#492) but now reaches its target
  ('Ethernet:IPv4:IPX:Unknown'). If the defect is fixed, delete its
  KNOWN_DEGRADED entry.

That is the ratchet working. Removed from both the test copy and the
generator copy.

Verified the cases genuinely reach IPX rather than merely stopping to
fail:

  link/Novell_Inc_0x8137: reached=True chain='Ethernet:IPX:Unknown'
  internet/IPX_in_IP:     reached=True chain='Ethernet:IPv4:IPX:Unknown'

7 passed, 83 subtests, both with pycrate present and with it blocked
(CI's condition, since .[test] excludes [NGAP]).

KNOWN_DEGRADED now holds only the two SCTP NGAP cases, which are a
placeholder-payload limitation rather than a library defect.
@JarryShaw JarryShaw added the fix Pull requests that fix a defect (fix: subject prefix) label Sep 22, 2026
JarryShaw added a commit that referenced this pull request Oct 2, 2026
…ing pull requests issues

The last residual of #987 in tests/vendor/test_ipx_socket_unit.py.

- The retention of ``Registered by Xerox`` was credited to "a real ownership
  fact", which is in no maintainer comment -- it is the changelog's own phrase.
  The ruling is #775's: a proprietary protocol may expose no name of its own,
  so the company name serves as its name.
- Four lines called a pull request a "GitHub issue". Every number in the file
  was checked against the API: #492, #507, #775 and #841 are issues; #503,
  #847 and #878 are pull requests. The tests/ carve-out exempts this file from
  preferring the issue over the pull request, not from being accurate about
  which a number is.
- #847 is dropped rather than relabelled. The sentence claimed #775/#847
  converted three Socket ranges, and #847 converted none -- its own
  description puts the range branches out of scope, leaving them minting via
  extend_enum. Relabelling would have kept the false attribution. #841 stays
  where it belongs, as the temporal anchor for the branch-order work.

One ruling, one citation, in each of the three places that cite one.

tests/vendor/test_ipx_socket_unit.py passes: 10 passed, 19 subtests, exit code
0 read from the process rather than a summary line.
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

const enum lookups reject values that appear on the wire: RouterAlert(0) is RFC 2113's only defined value

1 participant