RFC Verification/Hardening#108
Conversation
Greptile SummaryThis PR aligns
Confidence Score: 4/5Safe to merge for the classification correctness improvements; the new exclusions are well-sourced from IANA and all hex constants verified correctly. The main gap is that several newly excluded IPv4 and IPv6 blocks have no test entries, and the known-incomplete isDerived() detection is now load-bearing in the public-use check. The implementation is well-researched and the IANA registry alignment is accurate. The concerns are about test coverage gaps and an acknowledged limitation in isDerived() that is now silently affecting isPublicUse() results for non-canonical 6to4 addresses — previously that gap was inconsequential, but this PR makes it observable to callers. src/Version/IPv6.php (new isUnicastGlobal() chain, especially the TODO on isDerived()), tests/DataProvider/IPv4.php and tests/DataProvider/IPv6.php (missing test entries for the newly introduced address blocks). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["isPublicUse() IPv6"] --> B["isUnicastGlobal() OR\nmulticast global scope"]
B --> C{isUnicast?}
C -- No --> Z["false"]
C -- Yes --> D{isLoopback / isLinkLocal /\nisUniqueLocal / isUnspecified?}
D -- Yes --> Z
D -- No --> E{isMapped\n::ffff:0:0/96}
E -- Yes --> Z
E -- No --> F{isDocumentation\n2001:db8::/32\n3fff::/20 NEW}
F -- Yes --> Z
F -- No --> G{isBenchmarking\n2001:2::/48}
G -- Yes --> Z
G -- No --> H{isIetfProtocolAssignment\n2001::/23 minus carve-outs NEW}
H -- Yes --> Z
H -- No --> I{isDerived\n2002::/16 ⚠️ TODO}
I -- Yes --> Z
I -- No --> J{isNat64LocalUse\n64:ff9b:1::/48 NEW}
J -- Yes --> Z
J -- No --> K{isDiscardOnly\n100::/64 NEW}
K -- Yes --> Z
K -- No --> L{isDummyPrefix\n100:0:0:1::/64 NEW}
L -- Yes --> Z
L -- No --> M{isSegmentRoutingSid\n5f00::/16 NEW}
M -- Yes --> Z
M -- No --> Y["true (globally reachable)"]
|
| // The IANA special-purpose registry lists the 6to4 (derived) block | ||
| // `2002::/16` (RFC 3056) with a globally-reachable value of "N/A" | ||
| // rather than "true"; when in doubt, do what the Rust standard | ||
| // library does. | ||
| // TODO: This is broken until the Derived strategy detects | ||
| // non-canonical, "lossy" addresses. | ||
| && !$this->isDerived() |
There was a problem hiding this comment.
Known-broken
isDerived() detection is now load-bearing in isUnicastGlobal()
The TODO acknowledges that isDerived() fails to exclude "non-canonical, lossy" 6to4 addresses. Before this PR, isDerived() was not checked in isUnicastGlobal(), so that deficiency was invisible to callers of isPublicUse(). Now that !$this->isDerived() gates global-reachability, any 6to4 address that isDerived() misclassifies will silently pass through as publicly routable. It would be worth either fixing the detection in isDerived() first, or at least adding a note in the isPublicUse() docblock (and the public isDerived() docblock) warning that the 6to4 exclusion is incomplete.
No description provided.