Skip to content

fix(rule): Require a suffix match before checking its boundary - #567

Closed
OskarEichler wants to merge 2 commits into
weppos:mainfrom
OskarEichler:codex/check-rule-suffix-before-boundary
Closed

OskarEichler wants to merge 2 commits into
weppos:mainfrom
OskarEichler:codex/check-rule-suffix-before-boundary

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Check that the input actually ends with the rule value before accepting its label boundary.

Reproduction and verification

Rule.factory("com").match?("example.net.") returns true: chomp does not remove a suffix, but the unchanged input ends in a dot. Twenty-eight assertions cover mismatched trailing-dot inputs and existing exact/subdomain/boundary behavior across normal, exception and wildcard rules.

Each patch was verified independently on current main with Ruby 4.0.6: existing rake test 93 tests, 331 assertions, zero failures/errors/skips; full RuboCop 17 files, zero offenses; git diff --check passes.

The contribution guidelines request new tests, but this contribution's task explicitly prohibits adding or modifying repository tests. Focused checks therefore remain external scratch scripts; no repository tests were changed. No production data/services were used. Other Ruby versions and upstream CI remain unverified locally.

Compatibility / breaking changes

Direct rule matching no longer reports a match solely because the input ends in a dot. This includes trailing-dot inputs that previously matched accidentally; no automatic FQDN normalization is added to Rule#match?. PublicSuffix.parse retains its existing FQDN normalization. The separate wildcard-own-value behavior in open #549 is intentionally unchanged.

@weppos

weppos commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Same feedback as #566 (comment)

@OskarEichler

Copy link
Copy Markdown
Author

This patch came from the broad automated dependency audit and I do not have personal project context or a concrete consumer failure that supplies the missing motivation. I am closing it rather than asking you to review an entirely audit-generated change.

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