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

E-mail field checked twice for correct format #238

Closed
christianwgd opened this issue Mar 21, 2023 · 11 comments
Closed

E-mail field checked twice for correct format #238

christianwgd opened this issue Mar 21, 2023 · 11 comments

Comments

@christianwgd
Copy link

Hi, when I enter an invalid email address during registration, I get the error message "Enter a valid email address" twice.

I digged a little bit deeper and found that one comes from the standard django email validator (django.core.validators.EmailValidator) and the other one is added by the HTML5EmailValidator (django_registration.validators.HTML5EmailValidator).

Using Python3.10, Django 4.1.7 and django_registration 3.3

@laurentiurosu-04
Copy link

Hi,

I'm experiencing the same issue on my project, any luck in resolving it?

Thanks!

@christianwgd
Copy link
Author

I've created a minimal example to demonstrate the error and maybe analyse it: https://github.com/christianwgd/django_registration_issue_238

It creates the following output:
Bildschirm­foto 2023-03-21 um 13 08 49

@ubernostrum
Copy link
Owner

I'll try to figure this out. Unfortunately, Django's validators aren't really designed for two validators to both error with the same error message :/

@christianwgd
Copy link
Author

Hey, thanks for bothering. To me it seems like the HTML5 validator doesn't do a more accurate check on the email address than the standard django email validator. But maybe I don't fully understand, what the purpose of the HTML5 validator is.
So maybe it's possible to subclass the django email validator to add the missing HTML5 check there?

@christianwgd
Copy link
Author

christianwgd commented Mar 24, 2023

My current workaround for this is to remove the django email validator from the list of validators, since it seems to have a more strict regex.
If you can give me a hint on why you introduced the HTML5 validator, I can give it a try to bring those two together.
And if the HTML5 validator is checking on some special cases, we could try to respond only to them and try to find a more suitable error message for that (so not check the whole thing again).

@ubernostrum
Copy link
Owner

ubernostrum commented Mar 28, 2023

The main point of the HTML5 validator, for this specific app, is to ensure that no "valid" email address value is ever accepted which contains more than one @ character. This makes splitting the local-part and domain very easy for applying the homograph/confusable check (see #175 for background).

I also prefer the HTML5 validator's regex because it gets uniformity across the frontend and backend -- django-registration enforces the same thing an HTML5 input type="email" does -- and because it disallows a lot of the uselessly complex things people can "technically" do in an email address according to the RFCs, but which just make trouble for sites that try to handle them.

@ubernostrum
Copy link
Owner

Also I'm mildly curious to know how this is getting around the client-side validation -- since it's an input type="email" the browser ought to be applying the HTML5 email rules and not even allowing you to submit the form for the Django backend to see it.

@christianwgd
Copy link
Author

So you think the right way to go is what I've done in my workaround and use only the HTML5 validator and remove the standard django validator? As far as I've tested this seems to be a sufficient fix.

@ubernostrum
Copy link
Owner

I think that might be what ends up happening. I'm thinking through what will be in the next release right now (there are a couple other things I want to get in), as well as whether it's time to start planning a major version bump at some point to try to handle yet more custom-user edge cases...

@ubernostrum
Copy link
Owner

So as @christianwgd suggested, the least-breaking thing to do here is just to not apply the default Django validator. I've pushed a commit which makes that change, and it'll be included in the next release (3.4), due out Soon™.

Note that this is, amusingly, not a backwards-incompatible change because it doesn't change the set of accepted addresses.

@ubernostrum
Copy link
Owner

django-registration 3.4 has been released, containing this fix.

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

No branches or pull requests

3 participants