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

Backport "Add support for unicode domans to PhishingController" to v8.x #472

Merged
merged 1 commit into from
May 20, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 20, 2021

This is a backport of #471 to v8.x

The PhishingController now supports unicode domains. Previously it
supported plain ASCII domains (including punycode domains), but any
domains with non-ASCII unicode characters would always be considered
as safe.

Tests have been added to ensure the `test` and `bypass` methods both
normalize input to punycode.

Note that any whitelisted unicode domains will now be ignored. We could
fix this with a migration, but it's exceedingly unlikely that any
unicode domains could have been whitelisted in the first place, because
we only offer that ability for blocked domains. And unicode domains
were never blocked.

The `eth-phishing-detect` package was updated to make a pre-existing
test work correctly. Previously the "should bypass a given domain" test
was broken because the example unsafe domain was not blocked in the
version of `eth-phishing-detect` used. It is blocked in the current
version, and an assertion has been added to ensure these tests are
working correctly.
@Gudahtt Gudahtt requested a review from a team as a code owner May 20, 2021 15:18
@Gudahtt Gudahtt changed the base branch from develop to 8.x May 20, 2021 15:18
@Gudahtt
Copy link
Member Author

Gudahtt commented May 20, 2021

I'll just merge this - the diff is identical to #471

@Gudahtt Gudahtt merged commit be670e6 into 8.x May 20, 2021
@Gudahtt Gudahtt deleted the backport-unicode-phishing-support branch May 20, 2021 15:20
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.

1 participant