fix(vendor): retire the IPX socket scrape, its data source is gone (#507) - #511
Conversation
) Wikipedia removed the well-known-socket-number table in revision 1371327031 (2026-08-25), and rejects `requests`' default User-Agent with 403 besides. So the crawler could not fetch the page, and there was nothing left to parse if it could. - drop `LINK`, so `_request()` short-circuits and never touches the network; `bs4` and `re` imports go with it - add `DATA` (15 rows) and `RANGES` (5 rows), each entry carrying its source in-line: Novell's own *IPX Addressing* Table 2 for the 0x045x sockets, and RFC 1132 / RFC 1791 for 0x8060 / 0x9091-0x9093 - record in a `# NOTE:` why this is retirement rather than a re-point: the registry was Novell's and is closed, so a crawler has nothing to pick up - add `tests/vendor/test_ipx_socket_unit.py`, which pins byte-for-byte reproduction of the committed constant file `pcapkit/const/ipx/socket.py` regenerates byte-identically -- 15 members, same names, values and order, `Socket(0x0000)` intact. 9 passed / 16 subtests; unit tier 943 passed / 1968 subtests, +9/+16 against baseline.
ee020b3 to
010dc03
Compare
Review: CHANGES NEEDED — prose and citations, not codeThe three load-bearing claims all hold under independent measurement. What needs fixing is the sourcing prose plus one docs line, and none of it touches Verified. Byte identity: regenerated by constructing Needs changing:
On the On Could not verify: the Minimum to merge: items 1-5 and the |
… runnable in CI Addresses review feedback on #511. - pyproject.toml: add requests and beautifulsoup4 to the `test` extra. The nine tests pinning the byte-for-byte regeneration skipped in CI, so the only guard on the hand-maintained registry never ran on any CI path. Adding requests alone is not enough: pcapkit/vendor/__init__.py imports every crawler unconditionally and eight of them import bs4 at module scope, so the tests fail with ModuleNotFoundError rather than run. - tests: widen the skip guard from requests to requests + bs4 to match, and pin member *names* in test_unlisted_sockets_still_resolve. Every branch of the generated _missing_ returns `value`, so asserting the value alone passed with three of the five range branches deleted. - vendor/ipx/socket.py: correct the per-entry sourcing. IANA's xns-protocol-types and Xerox XSIS 028112 Appendix D source 0x0001-0x0003 and three of the five ranges, so only 0x0457 and 0x4003 rest on the archived revision alone. Record that Socket.TCP_over_IPXF is misnamed (RFC 1791 s3 assigns 0x9091 to TCP over plain IPX and advises against IPXF), that Novell's Table 2 has seven rows and two are absent, and that the 0x4000-0x4FFF bound contradicts Novell's 0x7FFF. None renamed or added: that would end the byte-identity proof. - docs: note that the crawler no longer crawls, and point the footnote at the archived revision rather than a live anchor. pcapkit/const/ipx/socket.py is unchanged (md5 7548ec44c461a5c1d30d57751ae54f42).
Review feedback addressed — pushed
|
test extra |
result |
|---|---|
| as this PR stood | 9 skipped — every reason "requests not installed (vendor extra)" |
+ requests only |
9 failed — ModuleNotFoundError: No module named 'bs4' |
+ requests, beautifulsoup4 |
9 passed, 16 subtests |
Both are added, plain — no [socks], no [html5lib], since no unit-tier test goes through a proxy or parses HTML. The skip guard is widened from requests to requests and bs4 so a deps-less checkout still skips rather than errors.
The suite baseline in this PR's description is not what CI measures
CI installs .[test], and a clean .[test] venv gives 869 passed / 82 skipped / 1910 subtests, going to 878 / 73 / 1926 with this change — a delta of exactly +9 passed, −9 skipped, +16 subtests. The 82 skips are the optional engines (dpkt, scapy, pyshark, pypcapfile, cryptography, emoji), none of which .[test] installs. Higher counts quoted earlier came from an environment carrying all extras; the figures above are what unit-tests.yml actually runs.
The name-pinning hardening, with its proof
test_unlisted_sockets_still_resolve now pins member.name as well as the value, because every branch of the generated _missing_ returns value — so asserting the value alone passes with range branches deleted. Proof: dropping the 0x0BB9 <= value <= 0xFFFF masking branch gives
8 failed, 1 passed, 6 subtests passed
SUBFAILED 0x0BB9, 0x4000, 0x4FFF, 0x7FFF, 0x8000, 0x8061, 0x9094, 0xFFFF
— exactly the eight sockets that branch covered. Restored afterwards, md5 re-verified.
Corrections to the review and to this description
117vendor autoclass entries is 119. 119.. autoclass::underdocs/source/pcapkit/vendor/; 117 carry:members:(the crawler-class count meant), and the other two areVendor/VendorMetawith:no-members:atdefault.rst:10,35.ipx.rst:39was the sole:members:mismatch among the 23 LINK-less crawlers and is fixed here.- The
LLC_4argument's premise was wrong. RFC 1132 has four references, not two:[1]IEEE 802.2,[2]Novell IPX,[3]RFC 791,[4]RFC 826 ARP. The conclusion holds — its[4]is ARP, and the markup idcite_ref-RFC1132_4-1proves the marker is Wikipedia's footnote — but the stated reason should not be repeated. Also, the spaces inLLC [ 4 ]are not in the page: bareget_text()yieldsLLC[4], and the spaces are an artefact of the old scrape joining cell strings and picking up thecite-bracketspans as separate tokens. - "Will still lose every socket the scrape used to supply" is not what happens — the next regeneration fails outright and the constant file is simply never rewritten. The 403 fetch failure comes first (
Vendor._requestretries, then raises, andvendor/__main__.py:59turns it into aVendorRuntimeWarning); theIndexErroratvendor/ipx/socket.py:82is only reachable once the fetch is fixed. Silent loss would need the page to regain a fourthwikitable. - Per-entry sourcing is stronger than this PR claimed. IANA's live
xns-protocol-typesregistry and Xerox XSIS 028112 (December 1981) Appendix D both source0x0001–0x0003and three of the five ranges, so only0x0457and0x4003rest on the archived revision alone. The octal reading of Appendix D is settled by arithmetic rather than OCR: 3000 →0x0BB8, 3001 →0x0BB9, octal 40–77 →0x0020–0x003F, all matching the transcribed bounds, where decimal 40–77 would give0x0028–0x004Dand does not.
Recorded in code, deliberately not fixed
Socket.TCP_over_IPXF (0x9091) is misnamed: RFC 1791 assigns it in §3, "Running TCP Over IPX", which opens "Unlike UDP, TCP runs directly over IPX" and closes "Hence, running TCP over IPXF is not recommended." The value is right; the name came from the archived revision and describes the one arrangement its own RFC advises against. UDP over IPXF for 0x9092 is accurate. Also absent with good sourcing: 0x9001 NLSP and 0x9004 IPXWAN (Novell's Table 2 has seven rows; 0x9004 is reserved by RFC 1634 line 552, RFC 1551 line 532 verbatim, and RFC 1362 line 253), and an entry for all-ones 0xFFFF, which XSIS reserves to mean "all". And (0x4000, 0x4FFF) is contradicted by Novell's "between 0x4000 and 0x7FFF are dynamic sockets", not merely rounded.
None renamed or added: each would end the byte-identity proof, so they belong in their own review.
… runnable in CI Addresses review feedback on #511. - pyproject.toml: add requests and beautifulsoup4 to the `test` extra. The nine tests pinning the byte-for-byte regeneration skipped in CI, so the only guard on the hand-maintained registry never ran on any CI path. Adding requests alone is not enough: pcapkit/vendor/__init__.py imports every crawler unconditionally and eight of them import bs4 at module scope, so the tests fail with ModuleNotFoundError rather than run. - tests: widen the skip guard from requests to requests + bs4 to match, and pin member *names* in test_unlisted_sockets_still_resolve. Every branch of the generated _missing_ returns `value`, so asserting the value alone passed with three of the five range branches deleted. - vendor/ipx/socket.py: correct the per-entry sourcing. IANA's xns-protocol-types and Xerox XSIS 028112 Appendix D source 0x0001-0x0003 and three of the five ranges, so only 0x0457 and 0x4003 rest on the archived revision alone. Record that Socket.TCP_over_IPXF is misnamed (RFC 1791 s3 assigns 0x9091 to TCP over plain IPX and advises against IPXF), that Novell's Table 2 has seven rows and two are absent, and that the 0x4000-0x4FFF bound contradicts Novell's 0x7FFF. None renamed or added: that would end the byte-identity proof. - docs: note that the crawler no longer crawls, and point the footnote at the archived revision rather than a live anchor. pcapkit/const/ipx/socket.py is unchanged (md5 7548ec44c461a5c1d30d57751ae54f42).
dd888da to
1da80b1
Compare
Re-review of
|
test extra |
reproduced |
|---|---|
| as it stood | 9 skipped, 1 warning in 0.02s |
+ plain requests only |
9 failed, 2 warnings in 8.23s — traceback bottoms out at exactly pcapkit/vendor/reg/linktype.py:17 |
+ requests and beautifulsoup4 |
9 passed, 2 warnings, 16 subtests passed |
So overriding the original review's prescription was correct: plain requests genuinely does not work. It also checked something the claim never addressed — pytest categorises all nine as FAILED rather than ERROR even though the exception fires inside setUp(), matching the wording used here.
It found the undetected mutation, as asked
The three masked range branches are each a strict subset of an earlier always-checked-first branch, so they are unreachable at runtime. Widening 0x0020 <= value <= 0x003F to 0x0025 <= value <= 0x0999 — still fully masked — is completely invisible to test_unlisted_sockets_still_resolve alone: 1 passed, 14 subtests passed.
The whole-file run does catch it, but via test_regeneration_reproduces_the_committed_constant_file re-deriving from unmutated source and diffing byte-for-byte — a different mechanism than the name-pinning under review. So the hardening has a real blind spot for masked-branch bounds, while the suite as a whole does not; and a realistic edit (change RANGES in the vendor module, then regenerate) would additionally trip test_range_order_is_preserved's exact tuple compare regardless of masking. Recorded rather than changed.
Two corrections, one of them in committed code — both now fixed in 1da80b122
1. The bs4 importer count was wrong in its own context, and the confusion is worth explaining because three different numbers were all correct.
| tree | unguarded module-scope bs4 imports |
|---|---|
main |
8 — both ipx/packet.py and ipx/socket.py still scrape |
| #518's branch | 7 — #518 retires ipx/packet.py's scrape |
| this branch | 7 — this PR retires ipx/socket.py's scrape |
Both branches reach seven by removing a different file. An earlier comment of mine asserted eight as a correction to someone measuring seven — that was measured against main, so it was true there and wrong as a correction. The figure is now seven in pyproject.toml and tests/vendor/test_ipx_socket_unit.py, verified on this branch. The mechanism is unaffected either way: one unguarded import (reg/linktype.py, pulled in via reg/__init__.py) is sufficient to reproduce the failure, and it does.
2. "RFC 1132 has four references" is wrong — it has eight. Fetched from rfc-editor.org and the IETF datatracker independently: [1] IEEE 802.2, [2] Novell IPX, [3] RFC 791, [4] RFC 826/ARP, [5] ISO DIS 8473, [6] Xerox XNSG 068504, [7] RFC 1042, [8] Cohen. The sub-claim that [4] is RFC 826/ARP is correct, and the conclusion holds on ground the reviewer verified directly — the raw wikitext reuses <ref name="RFC1132" /> at the LLC row and the rendered anchor is cite_ref-RFC1132_4-1, where the _4 is Wikipedia's own footnote position and -1 its second use on the page, nothing to do with RFC 1132's bibliography. My "four references" line conflated two unrelated numbering systems and was wrong on the count as well. GitHub-conversation text only, not committed.
Verified more strongly than this PR claimed
The PR worried that Appendix D of Xerox XSIS 028112 was OCR-garbled and settled the octal radix by arithmetic instead. The reviewer installed poppler and read the scanned 1981 document directly — page 58, PDF page 67 — and it is fully legible: "socket numbers zero and all ones are reserved to mean unknown and all respectively", "Well-known socket numbers have the range 1 to 3000 decimal", "All other socket numbers are ephemeral", and a table header reading "Well-Known Socket (octal)" outright, with rows 1/2/3/40-77 exactly as transcribed. So three of the five ranges rest on a directly-legible primary source, and the arithmetic workaround is no longer load-bearing. Novell's live page independently confirms the other two bounds, including that (0x4000, 0x4FFF) really is contradicted by Novell's own 0x4000-0x7FFF.
Also confirmed exactly: 119 autoclass with 117 :members: and 2 :no-members:; bare get_text() yielding LLC[4]; the 0x9004 wording in RFC 1634 and 1551 as "protocol exchange" against RFC 1362's "information exchange"; 17 unconditional crawler imports; byte-identity at md5 7548ec44c461a5c1d30d57751ae54f42, 3298 bytes, with git diff origin/main on that file empty and the whole vendor-module diff being comments only.
On TCP_over_IPXF, agreed — with a better reason than mine
RFC 1791 §3 confirmed verbatim: titled "Running TCP Over IPX", opening "Unlike UDP, TCP runs directly over IPX", closing "Hence, running TCP over IPXF is not recommended." The value 0x9091 is right and only the label is wrong. Defer the rename — and the sharper argument is that even a deprecated alias changes extend_enum's generated output, which would end the byte-identity proof that is this PR's entire evidentiary basis. Its own change, later.
Not independently verified
The full-suite delta (869→878 passed, 82→73 skipped, 1910→1926 subtests) was not re-run, under host-care constraints after an earlier OOM incident; the nine-test delta is confirmed instead. The exhaustive "0 hits" sweep across all ten RFCs for 0x0457/0x4003 was not replicated — the three bearing on 0x9004 were spot-checked and matched exactly.
|
✅ GOOD TO MERGE — cross-reviewed twice, the |
…#519) Closes #519. #501 fixed one wrong exception name and forty stale `Args:` labels by hand. #519 showed the class was not exhausted. Re-deriving its census turned up one finding that is not a docstring defect at all, plus a third class the issue never mentions. `TCP._read_mode_sack` documented `ProtocolError: If length is **NOT** multiply of 8 plus 2` and never checked it. The tempting reading -- and the first one taken here -- is that the clause was stale, like the other five phantoms. It is the opposite, and three things settle it: RFC 2018 gives SACK a 2-octet header followed by 8-octet edge pairs; both neighbouring readers validate their own lengths, `_read_mode_sackpmt` on `!= 2` and `_read_mode_echo` on `!= 6`, with the same message; and nothing upstream enforced it either, since the schema's `ListField` consumes as many whole 8-octet items as it finds and ignores the tail. So the docstring was right and the code was wrong. - tcp.py: add the `(length - 2) % 8` check to `_read_mode_sack`, raising `ProtocolError` with the message its two siblings already use. - httpv2.py: drop the `Raises: ProtocolError` clause from the five `_read_http_*` readers that cannot raise it. `_read_http_none` shows the mechanism -- its `raise` is commented out and replaced by a `ProtocolWarning` on the next line. All 11 `_read_http_*` methods carried the clause; the 6 that genuinely raise keep it. - tcp.py, twice: un-indent a `Returns:` header that sat inside the `Args:` block. This is the class the issue missed, and it loses documentation rather than merely misnaming it -- napoleon parses the header as a parameter, so the page rendered `:param Returns:` and carried no `:returns:` at all. Measured through `GoogleDocstring` directly, before and after. - frame.py, schema/application/httpv2.py, schema/misc/pcapng.py: correct four `Args:` labels naming a parameter that does not exist. None of the four takes `**kwargs`, so each is a hard `TypeError`, not a cosmetic slip: `Frame.register(code=..., module=IPv4)` really does raise `TypeError: got an unexpected keyword argument 'module'`, while `protocol=` is accepted. - hip.py: document the parameters `_make_param_reg_response` and `_make_param_route_dst` omit, in the wording their own siblings `_make_param_reg_failed` and `_make_param_route_via` already use. - tests: `test_tcp_sack_length_unit.py` pins the length rule, accepted and rejected cases both. `test_docstring_contract.py` walks every function under `pcapkit/` and checks each documented name against the real signature, each documented exception against the real `raise` statements, and each section header against its indentation. It imports no pcapkit, reading source under its own root, so no editable install can shadow what it measures. Census re-derived at this base rather than taken from the issue, which counted against an older one: phantom Raises: 6 -> 0 remaining Args: naming a missing parameter 13 -> 9 remaining, all allowlisted swallowed section headers 3 -> 1 remaining, allowlisted The `Args:` figure is 13 here and was 14 when #519 was written: #511 retired the IPX socket scrape and renamed that crawler's `soup` parameter to `data`, incidentally fixing one. The nine that remain are in files owned by other in-flight changes and are recorded in `KNOWN_DEFECTS` with a reason each, under a test asserting every one still reproduces -- so the list cannot rot into a description of bugs that are gone. That test has already earned its keep: it failed on exactly the IPX entry during the rebase, which is how the stale entry was found and removed. Be clear about what is provable. A docstring label is not executed, so the corrections cannot fail a test individually -- the contract test is what pins them, by deriving the answer from the code instead of snapshotting today's wrongness. Verified by reintroducing each class into a scratch tree: baseline exit 0, phantom `Raises:` exit 1, bad `Args:` label exit 1, re-indented `Returns:` exit 1. The two `Raises:` assertions are complementary rather than redundant, and `_read_http_none` proves it -- `any(header.flags)` in its body makes the conservative reachability check judge it possibly-raising, so only the commented-out-raise check catches it. Measured: reachability exit 0, commented raise exit 1. The SACK change is the one with a real behavioural fails-before. Lengths 3, 11, 14, 17, 19 and 25 all parsed clean on `c8fd97bcd` with the SACK option present in `tcp.info['options']`, and all six now raise `ProtocolError: TCP: [OptNo 5] invalid format`. Two things deliberately not done. All 12 functions carrying both `**kwargs` and an `Args:` section without documenting `**kwargs` are in hip.py -- 438 of 450 document it elsewhere -- and that set includes the two siblings this change copies its wording from, so fixing 4 of the 12 is what would make hip.py inconsistent. And the SACK check implements exactly the documented rule, not RFC 2018's one-to-four block bound; `length=2` is degenerate and still parses, recorded in the test as a decision. Tests: 5 passed / 10 subtests (contract), 3 passed / 10 subtests (SACK), 25 passed / 19 subtests (tier guard), 81 passed / 73 subtests (transport and schema units, unchanged by the new check). An independent second scanner, written from scratch against the same baseline, reproduced all three counts (6 / 13 / 3) and the residuals (0 / 9 / 1), and turned up three things now recorded in the test module: - `_documented_names` justified its relative-indent measurement by saying `__doc__` is dedented at compile time. True of `__doc__` and irrelevant here, since this module reads `ast.get_docstring(..., clean=False)`, which preserves raw source indentation -- measured [0, 8, 12, 12] against [0, 0, 4, 4] for the dedented forms. The implementation was right for a different reason (nesting depth moves the absolute column); the rationale is corrected rather than left wrong in a checker for wrong rationales. - `_raised_names` resolves only `Name` and `Attribute` raise targets, and `ast.walk` attributes a nested `def`'s raise to the enclosing function. Both can only miss a phantom, never fail a correct docstring, and both are now documented with their measurements: 856 `Name` + 2 `Attribute` + 0 `Subscript` across 858 raise targets, and 11 documented functions with a nested `def`, 3 raising inside it. The second is the correct answer rather than a gap -- `_read_param_locator_set` documents `ProtocolError` and raises none itself, but calls a `_read_locator` helper that does. - One reported defect was a false positive and is recorded as a trap: `Raw.__post_init__` documents `error` and `alias`, has neither in its signature, and assigns a local `alias` -- but forwards `**kwargs` to `read`, which declares both as keyword-only and uses them. Reporting it would ask for correct documentation to be deleted.
…#519) (#535) Closes #519. #501 fixed one wrong exception name and forty stale `Args:` labels by hand. #519 showed the class was not exhausted. Re-deriving its census turned up one finding that is not a docstring defect at all, plus a third class the issue never mentions. `TCP._read_mode_sack` documented `ProtocolError: If length is **NOT** multiply of 8 plus 2` and never checked it. The tempting reading -- and the first one taken here -- is that the clause was stale, like the other five phantoms. It is the opposite, and three things settle it: RFC 2018 gives SACK a 2-octet header followed by 8-octet edge pairs; both neighbouring readers validate their own lengths, `_read_mode_sackpmt` on `!= 2` and `_read_mode_echo` on `!= 6`, with the same message; and nothing upstream enforced it either, since the schema's `ListField` consumes as many whole 8-octet items as it finds and ignores the tail. So the docstring was right and the code was wrong. - tcp.py: add the `(length - 2) % 8` check to `_read_mode_sack`, raising `ProtocolError` with the message its two siblings already use. - httpv2.py: drop the `Raises: ProtocolError` clause from the five `_read_http_*` readers that cannot raise it. `_read_http_none` shows the mechanism -- its `raise` is commented out and replaced by a `ProtocolWarning` on the next line. All 11 `_read_http_*` methods carried the clause; the 6 that genuinely raise keep it. - tcp.py, twice: un-indent a `Returns:` header that sat inside the `Args:` block. This is the class the issue missed, and it loses documentation rather than merely misnaming it -- napoleon parses the header as a parameter, so the page rendered `:param Returns:` and carried no `:returns:` at all. Measured through `GoogleDocstring` directly, before and after. - frame.py, schema/application/httpv2.py, schema/misc/pcapng.py: correct four `Args:` labels naming a parameter that does not exist. None of the four takes `**kwargs`, so each is a hard `TypeError`, not a cosmetic slip: `Frame.register(code=..., module=IPv4)` really does raise `TypeError: got an unexpected keyword argument 'module'`, while `protocol=` is accepted. - hip.py: document the parameters `_make_param_reg_response` and `_make_param_route_dst` omit, in the wording their own siblings `_make_param_reg_failed` and `_make_param_route_via` already use. - tests: `test_tcp_sack_length_unit.py` pins the length rule, accepted and rejected cases both. `test_docstring_contract.py` walks every function under `pcapkit/` and checks each documented name against the real signature, each documented exception against the real `raise` statements, and each section header against its indentation. It imports no pcapkit, reading source under its own root, so no editable install can shadow what it measures. Census re-derived at this base rather than taken from the issue, which counted against an older one: phantom Raises: 6 -> 0 remaining Args: naming a missing parameter 13 -> 9 remaining, all allowlisted swallowed section headers 3 -> 1 remaining, allowlisted The `Args:` figure is 13 here and was 14 when #519 was written: #511 retired the IPX socket scrape and renamed that crawler's `soup` parameter to `data`, incidentally fixing one. The nine that remain are in files owned by other in-flight changes and are recorded in `KNOWN_DEFECTS` with a reason each, under a test asserting every one still reproduces -- so the list cannot rot into a description of bugs that are gone. That test has already earned its keep: it failed on exactly the IPX entry during the rebase, which is how the stale entry was found and removed. Be clear about what is provable. A docstring label is not executed, so the corrections cannot fail a test individually -- the contract test is what pins them, by deriving the answer from the code instead of snapshotting today's wrongness. Verified by reintroducing each class into a scratch tree: baseline exit 0, phantom `Raises:` exit 1, bad `Args:` label exit 1, re-indented `Returns:` exit 1. The two `Raises:` assertions are complementary rather than redundant, and `_read_http_none` proves it -- `any(header.flags)` in its body makes the conservative reachability check judge it possibly-raising, so only the commented-out-raise check catches it. Measured: reachability exit 0, commented raise exit 1. The SACK change is the one with a real behavioural fails-before. Lengths 3, 11, 14, 17, 19 and 25 all parsed clean on `c8fd97bcd` with the SACK option present in `tcp.info['options']`, and all six now raise `ProtocolError: TCP: [OptNo 5] invalid format`. Two things deliberately not done. All 12 functions carrying both `**kwargs` and an `Args:` section without documenting `**kwargs` are in hip.py -- 438 of 450 document it elsewhere -- and that set includes the two siblings this change copies its wording from, so fixing 4 of the 12 is what would make hip.py inconsistent. And the SACK check implements exactly the documented rule, not RFC 2018's one-to-four block bound; `length=2` is degenerate and still parses, recorded in the test as a decision. Tests: 5 passed / 10 subtests (contract), 3 passed / 10 subtests (SACK), 25 passed / 19 subtests (tier guard), 81 passed / 73 subtests (transport and schema units, unchanged by the new check). An independent second scanner, written from scratch against the same baseline, reproduced all three counts (6 / 13 / 3) and the residuals (0 / 9 / 1), and turned up three things now recorded in the test module: - `_documented_names` justified its relative-indent measurement by saying `__doc__` is dedented at compile time. True of `__doc__` and irrelevant here, since this module reads `ast.get_docstring(..., clean=False)`, which preserves raw source indentation -- measured [0, 8, 12, 12] against [0, 0, 4, 4] for the dedented forms. The implementation was right for a different reason (nesting depth moves the absolute column); the rationale is corrected rather than left wrong in a checker for wrong rationales. - `_raised_names` resolves only `Name` and `Attribute` raise targets, and `ast.walk` attributes a nested `def`'s raise to the enclosing function. Both can only miss a phantom, never fail a correct docstring, and both are now documented with their measurements: 856 `Name` + 2 `Attribute` + 0 `Subscript` across 858 raise targets, and 11 documented functions with a nested `def`, 3 raising inside it. The second is the correct answer rather than a gap -- `_read_param_locator_set` documents `ProtocolError` and raises none itself, but calls a `_read_locator` helper that does. - One reported defect was a false positive and is recorded as a trap: `Raw.__post_init__` documents `error` and `alias`, has neither in its signature, and assigns a local `alias` -- but forwards `**kwargs` to `read`, which declares both as keyword-only and uses them. Reporting it would ask for correct documentation to be deleted.
1 participant
Closes #507.
Both halves of the issue re-verified against the live page
requestsUser-Agent403 body:
Please set a user-agent and respect our robot policy https://w.wiki/4wJS.Live page (revid 1372814585) carries 1
wikitable, headers['Octets', 'Field']— the IPX header format table.0x0451andNetWare Core Protocolappear nowhere in the HTML. TheSocket_numbersection still exists but now holds two sentences of prose and no table.Removal revision is 1371327031 (2026-08-25), comment "…deleted some irrelevant technical tables".
One correction to the issue's framing
#507 says the next regeneration "will still lose every socket the scrape used to supply". Measured, it does not — it fails outright (
IndexErroratvendor/ipx/socket.py:82), andvendor/__main__.py:59catches and warns, so the constant file is simply never rewritten. Silent loss would require the page to regain a fourthwikitable.Why retirement rather than pinning an archived revision
The archived revision was verified viable on the data side —
oldid=1368657333returns 200 with 4wikitables,table[3]headers['Socket number', 'Protocol'], 19 body rows, and running the oldprocess()against it reproduces today's constant file exactly.It is not viable on the code side. The User-Agent is applied in
pcapkit/vendor/default.py:404—page = requests.get(self.LINK), no headers — so any non-NoneLINK, archived or not, takes the same 403. Fixing that belongs in the shared base class and affects three other crawlers (see below), so it is deliberately out of scope here.The policy argument settles it, and it is recorded in the script: the registry was Novell's and is closed. Novell's own documentation still says "developers can contact Novell to reserve well-known sockets" — and there is no longer a Novell to contact, so a crawler has nothing to pick up on its next run.
Per-entry sourcing
novell.com/documentation/nw6p/ipx_enu/data/hvvqznoa.html, 200, still live) —0x451NCP,0x452SAP,0x453RIP,0x455NetBIOS,0x456Diagnostics. This is also the citation the Wikipedia article itself used.0x80600x9091TCP,0x9092UDP,0x9093IPXFStated honestly in the script:
0x0001,0x0002,0x0003,0x0457,0x4003rest on the archived revision alone. RFCs 1132, 1362, 1377, 1551, 1552, 1553, 1634, 1791, 1973 and 2043 were swept for those codes — 0 hits in all ten.0x0000is not a registry entry but IPX's own field default, per #492/#503.No loss — proven by byte identity
pcapkit/const/ipx/socket.pyregenerates byte-identically: same md5, 15 members, same names, values and order.Socket(0x0000)→Unspecified, so #503's member is intact.Byte identity was chosen deliberately as the strongest available proof, which meant keeping the
LLC_4member name and its#: LLC [ 4 ]comment — a Wikipedia footnote artefact (<ref name="RFC1132" />rendering as[4]). Cleaning that up would rename a public member, so it is flagged with a# NOTE:instead.Verification
Regeneration completes with both
requests.getandrequests.Session.requestpatched to raise, confirming no network path remains.tests/vendor/test_ipx_socket_unit.py— 9 tests, 16 subtests — pinsLINK is None, byte-for-byte reproduction of the committed constant, the full 15-member list, registry/enum agreement,Socket(0),RANGESorder, and rejection of out-of-range values.The tests are mutation-tested, because the first attempt was vacuous —
purge_modulesre-imports from disk, so in-memory mutations were discarded and every mutation "passed". Redone by mutating the file on disk with a hash-checked restore: dropping0x9093fails 2 tests, dropping0x0000fails 3, renamingLLC_4fails 2, reorderingRANGESfails 2.Delta is exactly the +9 tests / +16 subtests added.
isort -l100 -ppcapkit --check-onlyclean, matching whatcron-vendor.ymlruns.Follow-ups this surfaced, deliberately not fixed here
pcapkit/vendor/ipx/packet.pyis broken identically — same article, same removal revision,find_all('table', class_='wikitable')[1]at:82against a page that now has one table.vendor/vlan/priority_level.py,vendor/ftp/return_code.py,vendor/ipx/packet.py. The fix is a descriptive User-Agent atvendor/default.py:404.pcapng/*crawlers point at a dead URL —www.ietf.org/staging/draft-tuexen-opsawg-pcapng-02.html404s regardless of User-Agent. Working replacements:ietf.org/archive/id/…anddatatracker.ietf.org/doc/html/….