Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PublicSuffixMatcherLoader#getDefault #621

Merged

Conversation

joegallo
Copy link
Contributor

@joegallo joegallo commented Mar 5, 2025

The code of getDefault reads like it's supposed to load the entries from PUBLIC_SUFFIX_LIST by default, but it doesn't actually work that way in practice. I think this is a bug. This works correctly on 4.x, but the list there already starts with a leading slash:

This was sneaking past the tests, because the test for getDefault only checks that it isn't null, but a non-null nearly empty default PublicSuffixMatcher is still provided in the case of the url not existing, and the tests for PublicSuffixMatcher themselves reimplement the default loading logic (correctly, since they rely on ClassLoader#getResource rather than Class#getResource) and so they weren't running into the bug either.

joegallo added 3 commits March 5, 2025 16:53
since is the being resolved via Class#getResource, the path to the
file in question is absolute, not relative.
@ok2c ok2c merged commit c687247 into apache:master Mar 6, 2025
10 checks passed
@ok2c
Copy link
Member

ok2c commented Mar 6, 2025

@joegallo That you for contributing this fix.

ok2c pushed a commit that referenced this pull request Mar 6, 2025
@ok2c
Copy link
Member

ok2c commented Mar 6, 2025

Cherry-picked to 5.4.x

@joegallo joegallo deleted the fix-public-suffix-matcher-loader-get-default branch March 6, 2025 13:37
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