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

Validation concessions options for top-level domains #86

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

northpowered
Copy link

Added some specific args for validate_email and validate_email_domain_part methods to give an ability for validating "bad" TLD in email addresses. Full description was added in README

  • allow_special_domains=False - ignore restrictions for SPECIAL_USE domains
  • allow_any_top_level_domain=False - ignore regex constraints for a TLD
  • allowed_top_level_domains=[] - whitelist for TLD, not matching regex from previous option

All tests passed

Default values were set such as False for saving the original validation logic without slowdown

@JoshData
Copy link
Owner

Thanks. I feel strongly about not having an option to completely disable the special use domains check: It should not be easy to have non-deliverable addresses pass validation. Is modifying the global SPECIAL_USE_DOMAIN_NAMES list not helpful for your use case?

@northpowered
Copy link
Author

Thanks. I feel strongly about not having an option to completely disable the special use domains check: It should not be easy to have non-deliverable addresses pass validation. Is modifying the global SPECIAL_USE_DOMAIN_NAMES list not helpful for your use case?

Yep, maybe I can do smth like
global SPECIAL_USE_DOMAIN_NAMES
but I`m voting also for an availability to use any domain, if developer want it :D

@JoshData
Copy link
Owner

I`m voting also for an availability to use any domain, if developer want it :D

This isn't a democracy. :) My goal is to make a good library. It's not to solve any problem that anyone happens to post here.

If you can describe why you want these things, it can shape my thinking about what makes a good library for the purpose here.

@northpowered
Copy link
Author

I`m voting also for an availability to use any domain, if developer want it :D

This isn't a democracy. :) My goal is to make a good library. It's not to solve any problem that anyone happens to post here.

If you can describe why you want these things, it can shape my thinking about what makes a good library for the purpose here.

No problems. I made some utils for a legacy private infrastructure with TLD like foo123 and local. Yeah, I`s so surprised to see non-RFC TLD, but decided to do something :)
Also maybe I should push this changes to my fork and publish the lib with the different name? I planned to use it EmailStr in Pydantic

@JoshData
Copy link
Owner

JoshData commented Sep 3, 2022

On the dev branch in 1d2e4b2 I added an option globally_deliverable which when set to False will skip some of the checks you wanted to disable. That plus clearing SPECIAL_USE_DOMAIN_NAMES should get you what you were looking for. What do you think?

@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2022

Codecov Report

Merging #86 (1d2e4b2) into main (65b2744) will increase coverage by 1.52%.
The diff coverage is 76.19%.

❗ Current head 1d2e4b2 differs from pull request most recent head dace4e7. Consider uploading reports for the commit dace4e7 to get more accurate results

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   83.90%   85.43%   +1.52%     
==========================================
  Files           1        1              
  Lines         261      302      +41     
==========================================
+ Hits          219      258      +39     
- Misses         42       44       +2     
Impacted Files Coverage Δ
email_validator/__init__.py 85.43% <76.19%> (+1.52%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@northpowered
Copy link
Author

Yep, that's good! This globally_deliverable option ignore period and regex checks in TLD, but this patch isn't clearing SPECIAL_USE_DOMAIN_NAMES as you said before :-)
To clear this check I recommend to move block

    for d in SPECIAL_USE_DOMAIN_NAMES:
        # See the note near the definition of SPECIAL_USE_DOMAIN_NAMES.
        if d == "test" and test_environment:
            continue

        if ascii_domain == d or ascii_domain.endswith("." + d):
            raise EmailSyntaxError("The domain name %s is a special-use or reserved name that cannot be used with email." % domain_i18n)

under the if globally_deliverable: (line 556 in the dev branch)

I hope I'll able to add globally_deliverable option to Pydantic email validator in the nearest future :-)

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.

None yet

3 participants