Skip to content

Return nil for an absent internationalized registered domain - #32

Merged
knu merged 2 commits into
knu:masterfrom
OskarEichler:codex/handle-absent-idn-domains
Aug 30, 2026
Merged

Return nil for an absent internationalized registered domain#32
knu merged 2 commits into
knu:masterfrom
OskarEichler:codex/handle-absent-idn-domains

Conversation

@OskarEichler

Copy link
Copy Markdown
Contributor

Summary

Only decode a registered domain when one exists. This also removes the redundant IP-address special case, whose domain is already nil.

Reproduction and verification

DomainName.new("com").domain is nil, but domain_idn calls gsub on nil and raises NoMethodError. Five focused cases cover public suffixes and a real internationalized registered domain. Another 9,433 checks cover every ordinary suffix rule in the existing snapshot plus three IP 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. domain_idn now returns nil instead of raising when domain is nil. Existing registered-domain Unicode decoding and IP-address nil results are preserved.

@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, this fix looks correct. I reproduced the domain_idn failure for a public suffix and verified that this patch applies cleanly on the current local master and passes the full test suite.

Before merging, could you add repository regression tests covering an absent registered domain (for example, DomainName.new("com").domain_idn returning nil) and a normal internationalized registered domain? The external checks described in the PR are useful, but committed tests will protect this behavior going forward.

@OskarEichler

Copy link
Copy Markdown
Contributor Author

Added the repository regression for DomainName.new("com").domain_idn returning nil. The existing IDN test already covers a normal internationalized registered domain and still verifies 日本語ドメイン名example.日本. The focused test file passes with 11 tests and 605 assertions.

@knu
knu merged commit 811f6a8 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