Skip to content

Recognize IPv4-embedded IPv6 address forms - #33

Merged
knu merged 3 commits into
knu:masterfrom
OskarEichler:codex/parse-ipv4-embedded-ipv6
Aug 30, 2026
Merged

Recognize IPv4-embedded IPv6 address forms#33
knu merged 3 commits into
knu:masterfrom
OskarEichler:codex/parse-ipv4-embedded-ipv6

Conversation

@OskarEichler

Copy link
Copy Markdown
Contributor

Summary

Allow dotted-decimal tails in the existing IPv6 detection patterns, then continue delegating actual address validation and normalization to IPAddr.

Reproduction and verification

DomainName.new("::ffff:192.0.2.1") is classified as a hostname with a domain/TLD instead of an IP address. Four initial cases and 290 additional assertions cover mapped/translation/documentation prefixes, bracketed/unbracketed forms, case normalization, URI hosts, cookie equality and invalid addresses.

Verified independently on Ruby 4.0.6: existing rake test suite 11 tests, 642 assertions, zero failures/errors. Supplementary RuboCop Lint comparison has four baseline offenses and four after, no additions. Targeted syntax and git diff --check pass. The 9,565 generated suffix rules were also compared to the checked-in data source; no suffix-data update is included.

Focused checks are external scripts. No repository tests were added or modified because this contribution's task explicitly prohibits test-file changes. Other Ruby versions and upstream CI remain unverified locally. No network requests or real user data are needed for these reproductions.

Compatibility / breaking changes

No API or dependency changes. IPv4-embedded IPv6 forms are now classified as IP addresses with bracketed URI hosts and no registered domain/TLD/superdomain. Invalid dotted IPv6 forms recognized by the pattern raise IPAddr::InvalidAddressError instead of being treated as hostnames. Pure IPv4 and other IPv6 forms retain existing behavior.

@mergify

mergify Bot commented Aug 27, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@knu

knu commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Thanks, the implementation direction looks correct. I applied the patch to the current local master and verified mapped and embedded IPv4 forms, bracketed and unbracketed input, URI-host normalization, invalid input, and the full test suite.

Before merging, could you add these cases as repository regression tests? In particular, please cover mapped and non-mapped IPv4-embedded IPv6 addresses, bracketed input, normalized uri_host, and an invalid dotted IPv6 candidate. This parsing boundary is subtle enough that external-only checks are not sufficient for future maintenance.

@OskarEichler

Copy link
Copy Markdown
Contributor Author

Added repository coverage for mapped and non-mapped embedded IPv4 forms, bracketed input, normalized hostname and uri_host values, nil domain/TLD, and an invalid dotted candidate raising IPAddr::InvalidAddressError. The focused file passes with 11 tests and 620 assertions.

Comment thread lib/domain_name.rb Outdated
Comment on lines +90 to +91
when /\A([0-9A-Fa-f:.]*:[0-9A-Fa-f:.]*:[0-9A-Fa-f:.]*)\z/,
/\A\[([0-9A-Fa-f:.]*:[0-9A-Fa-f:.]*:[0-9A-Fa-f:.]*)\]\z/

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this pattern is overly lax. Dot-separated decimals should only appear in the last component, I guess?

@OskarEichler OskarEichler Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Tightened the pattern in 86726e4 so dotted decimals are accepted only as the final IPv6 component, while the pure-hex form remains unchanged. I also added a regression for a dotted component in the middle; the focused suite passes with 11 tests / 621 assertions.

@knu
knu merged commit 2401b64 into knu:master Aug 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants