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

Add support for unicode domans to PhishingController #471

Merged
merged 1 commit into from
May 20, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 20, 2021

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 14:53
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 force-pushed the phishing-detector-punycode branch from e0d7aa6 to 6decd8e Compare May 20, 2021 15:03
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

@Gudahtt Gudahtt merged commit abc960e into develop May 20, 2021
@Gudahtt Gudahtt deleted the phishing-detector-punycode branch May 20, 2021 15:14
Gudahtt added a commit that referenced this pull request May 20, 2021
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 added a commit that referenced this pull request May 20, 2021
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 mentioned this pull request May 20, 2021
@estebanmino estebanmino mentioned this pull request May 20, 2021
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
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.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
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.
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