Opened 7 months ago
Last modified 6 days ago
#36809 assigned New feature
Allow EmailValidator to customize error messages depending on the part that failed validation
| Reported by: | Daniel E Onetti | Owned by: | Natalia Bidart |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | EmailValidator |
| Cc: | Mike Edmunds, jaffar Khan | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently, the EmailValidator only provides the full 'value' in the params dictionary when a ValidationError is raised. However, in many use cases, developers need to customize error messages based specifically on the domain part of the email (e.g., "The domain %(domain_part)s is not allowed").
This change adds 'domain_part' to the params dictionary in EmailValidator.__call__, bringing it in line with how other validators provide decomposed parts of the validated value. This allows for more granular and helpful error messages for end users.
I have already prepared a patch with tests and verified that it passes all style (flake8) and functional checks.
Change History (17)
comment:1 by , 7 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
follow-up: 4 comment:2 by , 7 months ago
follow-up: 12 comment:3 by , 7 months ago
| Has patch: | set |
|---|---|
| Keywords: | EmailValidator added |
| Needs documentation: | set |
| Needs tests: | set |
| Owner: | changed from to |
| Patch needs improvement: | set |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Cleanup/optimization |
Hello Daniel! Thanks for the report, for taking the time to prepare a patch, and for following the contributing guide. I appreciate that.
I agree that the use case is valid and that being able to reference the domain in error messages can be genuinely useful. That said, I am not convinced that simply adding domain_part to ValidationError.params is the best fit for Django core. It solves the immediate problem, but does so in a way that feels quite specific rather than structural. It also risks committing to an API that only partially reflects how EmailValidator actually works.
Looking at the other validators in this module, there is a clear pattern of separating parsing, normalization, and validation into distinct steps or methods, even when the final error surface remains simple. EmailValidator is somewhat inconsistent here: it already decomposes the value into user and domain components, but does so inline inside __call__, with no clear extension points. Adding a single extra param addresses the symptom rather than the structure.
I think a more Django aligned approach would be to re-evaluate EmailValidator as a whole and give it clearer, overridable validation hooks, while keeping full backward compatibility. For example, introducing explicit methods like validate_recipient() and validate_domain(), called from __call__, would allow customization via subclassing without re-parsing the email or wrapping errors. The existing validate_domain_part() could be kept for compatibility and potentially deprecated later, since it currently returns a boolean.
So in short, I agree with the motivation, but I think the solution needs to be broader and more structural than the current proposal in order to align better with Django's existing validator patterns.
comment:4 by , 7 months ago
Replying to Kundan Yadav:
hey can i work on it or is it already being worked on by you
Hello Kundan, there is an initial PR already for this issue by Daniel, so I have re-assigned this to them,
comment:5 by , 7 months ago
| Summary: | Allow EmailValidator to return 'domain_part' in ValidationError params → Allow EmailValidator to customize error messages depending on the part that failed validation |
|---|
comment:6 by , 7 months ago
| Cc: | added |
|---|
Natalia's proposal would also simplify implementation of #27029 EAI address validation. (Or help developers who need that functionality sooner subclass EmailValidator themselves.)
Django already uses "recipient" to mean a complete email address, possibly including a friendly display name (e.g., django.core.mail.EmailMessage.all_recipients()). The official RFC 5322 term for the part before the @ is local-part, but other common terms are user (e.g., existing EmailValidator code) or username (e.g., Python's email.headerregistry.Address.username). I'd maybe go with validate_username() and validate_domain() to align with Python's email package.
comment:7 by , 7 months ago
Thank you Mike for the validation! I appreciate it. Your naming proposal sounds great!
comment:8 by , 5 months ago
| Cc: | added |
|---|
I want to work on this ticket as the current owner seem to no longer active.
I think it is more genuine way to separate checks of username and domain as Natalia proposed. I defined two methods validate_username() and validate_domain() as:
def validate_username(self, user_part):
if not self.user_regex.match(user_part):
raise ValidationError(self.message, code=self.code, params={"value": user_part})
def validate_domain(self, domain_part):
if domain_part not in self.domain_allowlist and not self.domain_regex.match(domain_part):
raise ValidationError(self.message, code=self.code, params={"value": domain_part})
then inside call, I called these two methods and removed the username and domain checks as it looks no longer necessary:
def __call__(self, value):
# The maximum length of an email is 320 characters per RFC 3696
# section 3.
if not value or "@" not in value or len(value) > 320:
raise ValidationError(self.message, code=self.code, params={"value": value})
user_part, domain_part = value.rsplit("@", 1)
self.validate_username(user_part)
self.validate_domain(domain_part)
I think it will be a more developer-friendly way. Please let me know whether to create a PR.
comment:9 by , 5 months ago
I checked docs/ref/validators.txt, and there is no description about methods of classes, so I don't think documentation changes are needed.
comment:10 by , 5 months ago
I opened a PR https://github.com/django/django/pull/20616 based on above description.
There are some linter failures, If the modified changes are acceptable then I will polish the PR.
comment:11 by , 5 months ago
| Owner: | changed from to |
|---|
follow-up: 14 comment:12 by , 5 months ago
Natalia, in attempting to review jaffar Khan's PR, I realized that while I agree with these statements in theory, I'm having trouble putting them into practice:
Replying to Natalia Bidart:
… Looking at the other validators in this module, there is a clear pattern of separating parsing, normalization, and validation into distinct steps or methods, even when the final error surface remains simple.
EmailValidatoris somewhat inconsistent here: it already decomposes the value into user and domain components, but does so inline inside__call__, with no clear extension points. Adding a single extra param addresses the symptom rather than the structure.
I think a more Django aligned approach would be to re-evaluate
EmailValidatoras a whole and give it clearer, overridable validation hooks, while keeping full backward compatibility. For example, introducing explicit methods likevalidate_recipient()andvalidate_domain(), called from__call__, would allow customization via subclassing without re-parsing the email or wrapping errors. …
I couldn't tell which validators you were referring to that separate parsing, normalization and validation. EmailValidator follows the pattern of the RegexValidators: it has some (overridable) regex and list properties, and it does pretty much everything in its __call__() method. Unlike the RegexValidators, it also has one extension point method: validate_domain_part() allows replacing the domain validation logic (but not domain_allowlist exceptions or ValidationError construction). I believe the intent was to support stricter domain validation via subclassing (e.g., for users who felt DomainNameValidator was too lax or wanted to disallow numeric domain literals).
If a refactored EmailValidator looks something like:
class EmailValidator: def __call__(self, value): ... # pre-validation omitted username, domain = value.rsplit("@", 1) # or self.parse_parts(value) ? self.validate_username(username, value) self.validate_domain(domain, value) def validate_domain(self, domain, value): if domain not in self.domain_allowlist and not some_other_logic_on(domain): raise ValidationError(self.message, code=self.code, params={"value": value})
… then the original ticket request to have access to the domain in the error message does seem to require wrapping errors (or duplicating logic from the superclass):
class EmailValidatorWithBetterErrorMessages(EmailValidator): def validate_domain(self, domain, value): try: super().validate_domain(domain, value) except ValidationError as error: # change "code" and add domain to the error params raise ValidationError(self.message, code="invalid-domain", params={"value": value, "domain": domain}) from error
… and substituting the domain validation logic requires duplicating some of the superclass code:
class EmailValidatorWithCustomDomainValidation(EmailValidator): def validate_domain(self, domain, value): if domain in self.domain_allowlist: return # duplicate allowlist logic from superclass if not idna.utils.is_valid_domain(domain): raise ValidationError(self.message, code=self.code, params={"value": value})
… which makes me think I've misunderstood what you were envisioning in reworking EmailValidator to improve subclassing hooks.
comment:13 by , 5 months ago
Thanks Mike for the review. I think defining subclass is not a good option which makes duplication of code as you describe. Defining explicit methods like I did will makes the code more readable and will avoid duplication.
In my current PR I made some changes like the methods validate_domain() and validate_username() will return boolean so we need to wrap the methods inside call. Now I am confused about do I have to make these two methods to raise ValidationError then no need for wrapping inside call or left as it is?
follow-up: 16 comment:14 by , 6 days ago
| Description: | modified (diff) |
|---|---|
| Needs documentation: | unset |
| Needs tests: | unset |
| Owner: | changed from to |
| Patch needs improvement: | unset |
| Type: | Cleanup/optimization → New feature |
Replying to Mike Edmunds:
I couldn't tell which validators you were referring to that separate parsing, normalization and validation.
I made this reasoning a while ago, and since you asked, I kept snoozing the email. Sorry! But I think I meant something like this:
URLValidatorstarts with a parsing stage (value.split("://")for the scheme,urlsplit(value)into components), then normalizes (.lower()on the scheme), then validates each piece (scheme inschemes, hostname length, IPv6 in the netloc), raising as it goes.DecimalValidatoralso has a parsing phase (value.as_tuple()into digits/exponent), then normalizes (derivesdigits/decimals/whole_digits), then validates each against the limits, raising a distinct code per failure from itsmessagesdict.FileExtensionValidatoris smaller but IMO has the same pattern: parse+normalize (Path(value.name).suffix[1:].lower()), then validate againstallowed_extensions.
Granted those keep the concerns inline in __call__() and are not split by methods, but I think the "stages" are defined. The BaseValidator family is the one that turns them into dedicated methods: clean() is the normalize stage, compare() is the validate stage (the bool decision), and __call__() owns the raise.
[...] what you were envisioning in reworking EmailValidator to improve subclassing hooks.
I don't think I had a specific implementation in mind when I wrote that, but last night (and a lot of today, this was more time consuming than anticipated, but it brought :sparkles:) I spent some time putting together a more concrete plan. I went looking for precedents and used your use cases as a guide (thanks for those BTW). Some thoughts:
- The hooks are a bool decision A
validate_domain()that only raises fuses the decision and the error into one method; fine for replacing the logic, but for yourEmailValidatorWithBetterErrorMessagesit forces exactly the wrapping you flagged. So the default hook returns a bool and__call__()owns the raise. The key addition: each hook receives the fullvalue, so a subclass that does want a custom message can overridevalidate_domain()and even potentially raise a completeValidationErrorstraight from the hook (withvalueinparams, custom code, etc), no wrapping. I deprecatedvalidate_domain_part()in favor of the nwevalidate_domain()and addedvalidate_username()(the names align with Python'semail.headerregistry.Address, per your suggestion).
- The default error
codestays"invalid". There's a singlecodefor the whole validator, so today every failure raises"invalid"and there's no way to give only the domain part its own code. Themessagesdict idiom I wanted to borrow (DecimalValidator, anddefault_error_messagesacross forms and model fields) does give each failure its own code, but those validators were built with distinct codes from scratch; bringing it toEmailValidatorwould mean the domain failure starts emitting a new code instead of"invalid", a backwards-incompatible change with no clean deprecation path (I couldn't find a precedent for changing an already-emitted code value, with or without deprecation -- ideas welcome here!). What the new hooks do give users an opt-in: because they receivevalue, a subclass can raise its own per-part code fromvalidate_domain()(e.g.code="invalid_domain") without changing the default for everyone else.
So what's left puts the failed username / domain in params, deprecates validate_domain_part() in favor of validate_domain(), and adds validate_username(); both returning a bool by default but receiving the full address: https://github.com/django/django/pull/21568
follow-up: 17 comment:15 by , 6 days ago
Apparently, a solution for ticket #27029 would look like this (llm-generated) following my PR:
@deconstructible class EmailValidator: message = _("Enter a valid email address.") code = "invalid" ... user_regex = _lazy_re_compile( # dot-atom r"(^[-!#$%&'*+/=?^_`{}|~0-9A-Z]+(\.[-!#$%&'*+/=?^_`{}|~0-9A-Z]+)*\Z" ... ) + # RFC 6531: a UTF-8 local part, i.e. the dot-atom class plus any non-ASCII. + smtputf8_user_regex = _lazy_re_compile( + <something something> + r'*"\Z)', + re.IGNORECASE, + ) domain_allowlist = ["localhost"] - def __init__(self, message=None, code=None, allowlist=None): + def __init__(self, message=None, code=None, allowlist=None, allow_smtputf8=False): if message is not None: self.message = message if code is not None: self.code = code if allowlist is not None: self.domain_allowlist = allowlist + self.allow_smtputf8 = allow_smtputf8 def validate_username(self, username, value): - return bool(self.user_regex.match(username)) + regex = self.smtputf8_user_regex if self.allow_smtputf8 else self.user_regex + return bool(regex.match(username)) def validate_domain(self, domain, value): if self.domain_regex.match(domain): return True + if self.allow_smtputf8: + # Accept an internationalized (U-label) domain via its IDNA form. + try: + domain.encode("idna") + return True + except UnicodeError: + pass literal_match = self.literal_regex.match(domain) ...
comment:16 by , 6 days ago
Replying to Natalia Bidart:
[...] So what's left puts the failed
username/domaininparams, deprecatesvalidate_domain_part()in favor ofvalidate_domain(), and addsvalidate_username(); both returning a bool by default but receiving the full address: https://github.com/django/django/pull/21568
That looks great to me. I left a couple of questions and comments in the PR.
Also, one option for enabling distinct codes (without going through deprecation) might be something like:
class EmailValidator(...): code = "invalid" code_username = code code_domain = code def __call__(value): ... if not self.validate_username(user_part, value): raise ValidationError(..., code=self.code_username, ...)
…so a custom subclass could keep all the built-in validation logic and just override the codes:
class EmailValidatorWithDistinctCodes(EmailValidator): code_username = "invalid-username" code_domain = "invalid-domain"
comment:17 by , 6 days ago
Replying to Natalia Bidart's LLM:
Apparently, a solution for ticket #27029 would look like this (llm-generated) following my PR:
@deconstructible class EmailValidator: [...] + # RFC 6531: a UTF-8 local part, i.e. the dot-atom class plus any non-ASCII. + smtputf8_user_regex = _lazy_re_compile( + <something something> + r'*"\Z)', + re.IGNORECASE, + ) [...] - def __init__(self, message=None, code=None, allowlist=None): + def __init__(self, message=None, code=None, allowlist=None, allow_smtputf8=False): [...] + self.allow_smtputf8 = allow_smtputf8 [...]: def validate_domain(self, domain, value): if self.domain_regex.match(domain): return True + if self.allow_smtputf8: + # Accept an internationalized (U-label) domain via its IDNA form. + try: + domain.encode("idna") + return True + except UnicodeError: + pass literal_match = self.literal_regex.match(domain) ...
If I may speak directly to your LLM for a moment (or to anyone thinking about copying and pasting that code into a PR for #27029): (1) Naming-wise, let's try to avoid confusing the rfc6531 "smtputf8" SMTP protocol extension with rfc6532 internationalized email headers (which allows utf8 in email addresses, among other things). Although those RFCs are related, the EmailValidator is about addresses not SMTP. (2) That <something something> in your regex is doing some heavy lifting. (3) The domain_name_regex borrowed from DomainNameValidator already allows IDNA U-labels. You've either managed to reintroduce a variation on the dead code that was removed in 54059125956789ad4c19b77eb7f5cde76eec0643, or you've broken IDNA 2008 domains by insisting on Python's IDNA 2003 encoding. 😀
hey can i work on it or is it already being worked on by you