Repository navigation
ACOMMONS-164 Improve IP classification and allow querying without reparsing - #571
Conversation
Let callers parse an address once and query multiple predicates on the immutable result. Keep public static predicates as compatibility wrappers and migrate tests from the package-private classification API.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
0a1e678 to
591a56f
Compare
Use the standard library to parse IPv4 and mixed hexadecimal/decimal IPv6 spellings. Guard numeric inputs against DNS lookup, require four-component IPv4 and ASCII CIDR prefixes, and restore the native IPv6 value of JDK-converted mapped addresses.
… payload Apply IPv4 classification consistently to dotted and pure-hex mapped and historical translated addresses. Project only ranges wholly contained within either family, retain native CIDR prefix semantics, and document translated payload classification as an explicit policy.
Classify the base IPv6 address independently of its interface selector. Accept named, numeric, and empty zones without interface lookup, preserve CIDR prefixes, and reject IPv4 zones.
Expose multicast as an independent classification category for ranges wholly contained in 224.0.0.0/4 or ff00::/8. Keep reserved-range classification unchanged.
Give analyzers a common suppression policy for loopback, any-address, broadcast, documentation, and multicast ranges. Private and link-local addresses remain reportable; callers can parse once and query the policy on the result.
591a56f to
b2d76c1
Compare
teemu-rytilahti-sonarsource
left a comment
teemu-rytilahti-sonarsource
left a comment
There was a problem hiding this comment.
Great improvements, thanks @lijun-chen-sonarsource! 👍
Please update the PR title to be something more descriptive and see my minor nits about name of isS1313Exempt & test coverage, otherwise this looks good to go.
| "fc99:1234::42, true", | ||
| "fc00::1, true", | ||
| "fc00::/7, true", | ||
| "::ffff:10.0.0.1, true", |
There was a problem hiding this comment.
Move this into its own category (as this is not a v6 unique local), feel free to add other RFC1918 addresses.
| /** | ||
| * True if the literal is the IPv4 limited-broadcast address {@code 255.255.255.255} (or {@code /32}). | ||
| * IPv6 has no broadcast equivalent, so every IPv6 literal returns false. | ||
| * IPv6 literals return false except mapped and translated addresses, which use the IPv4 payload classification. |
There was a problem hiding this comment.
Payload classification? 🤔
There was a problem hiding this comment.
39aae00 now uses "embedded IPv4". Payload was indeed confusing.
lijun-chen-sonarsource
changed the title
|
Code Review ✅ Approved 2 closed / 2 findings🟡 Medium risk · Reworking IP parsing and address classifications could cause analyzers to misclassify literals, missing security findings or flagging benign addresses. Adds ✅ 2 closed✅ Security: Leading-zero IPv4 octets now parsed as decimal, so S6329 misses them
Closed: Cross-PR: sonar-php S1313 migration won't compile and multicast test fails
Verification Rules 1/1 met Repos 5 inspected · PRs 1 inspectedFunctional validation No results Rules 1 passed 2 not applicable
Broader impact Repos 5 inspected PRs 1 inspectedCross-repo 5 repositories inspected 0 open impacts
Cross-PR 1 PR inspected 0 open impacts
Auto-approval Not enabledFor more verification details, comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Expose
parse(String)returning an immutableParsedIp, so analyzers candistinguish invalid input and query multiple classifications without reparsing.
Existing public static predicates remain available.
addresses by their embedded IPv4 address.
InetAddress.getByName, with lexical guardsto prevent DNS lookup. Strip IPv6 zone suffixes to avoid interface lookup.
Reject leading-zero decimal octets because some consumers interpret them
as octal, while the JDK interprets them as decimal.
classification.
whether an IP address is allowed to be hardcoded.
Summary by Gitar
ParsedIp.isMulticastcovering IPv4224.0.0.0/4and IPv6ff00::/8, included inisS1313ExemptIpAddressClassifier.parsewith guarded numeric parsing that rejects ambiguous leading-zero octetsClassificationenum andisInternetRoutableThis will update automatically on new commits.