Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#20631 closed Bug (fixed)

Increase the default max_length of EmailField to 254

Reported by: Pablo Martín Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: schemamigration
Cc: marc.tamlyn@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

How you can see in the next link the max length of an email is 254 chars. Now we have only 75 chars...

http://stackoverflow.com/questions/386294/what-is-the-maximum-length-of-a-valid-email-address

Related commit:

https://github.com/django/django/commit/bfcda7781a886ab2b7b41937c0f49c088f58a3d7

The max length does not change directly, because "At present we do not have the ability to make schema changes in core and so we cannot change the underlying database field of any given model field" (Marc Tamlyn)

First steps https://github.com/django/django/pull/1291 (this pull request is closed)

Change History (13)

comment:1 by Pablo Martín, 11 years ago

Fixed with this pull request:

https://github.com/django/django/pull/1292

comment:2 by Russell Keith-Magee, 11 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted

Accepting the ticket, because I agree this is a problem.

However, if we're going to introduce a new setting for this, I don't think I agree making the EMAIL_MAX_LENGTH a configurable parameter is the right approach. The only reasonable values are 75 (the historical value) and 254 (the RFC-correct value). We should be looking at a way to migrate people to a new default, not provide a long term configuration hook.

The discussion around custom User models briefly hit on this topic - I'd be more in favor of seeing a temporary, boolean setting like ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES that allows a slow transition over a couple of releases.

It's also worth thinking about how this would integrate with the 'checksetup' command recently added to trunk. This command was recently added (and may soon be renamed to check or verify) specifically to handle an analgous problem with TEST_RUNNER. There's no database migration issue there, but there is an issue with getting test suites correctly named so that the Django 1.6 test discovery mechanism will find all the tests in a suite. We might be able to use the same tool to good advantage here.

comment:3 by Marc Tamlyn, 11 years ago

Cc: marc.tamlyn@… added
Has patch: set
Needs tests: set
Patch needs improvement: set
Triage Stage: AcceptedUnreviewed

I think that the change to the email validator is a good idea - the regex should be more accommodating.

I am definitely -1 on introducing a new setting, and I think that at this time we should not be changing the defaults for existing field types until we can change it in the model layer as well. The documentation for EmailField (https://docs.djangoproject.com/en/dev/ref/models/fields/#emailfield) already gives warnings about this.

In any case, it needs tests.

What Russ says about using the check command - I wonder whether it would be worth in the future looking to extend it to have a "recommendations" mode - highlights things like slug fields with no uniqueness condition, email fields which don't have the length overridden etc. --best-practices basically.

comment:4 by Pablo Martín, 11 years ago

Ok no problem, I can change EMAIL_MAX_LENGTH settings to ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES settings.

But mjtamlyn do you like the ALLOW_RFC_COMPLIANT_EMAIL_ADDRESSES settings?

comment:5 by Marc Tamlyn, 11 years ago

Personally I don't think it's worth making changes to the form field now when we can make changes to the model field once migrations lands (which is on its way). I'd love it to be right, but I don't think it's worth having an intermediary step when the "real" fix is probably going to be in Django 1.7 anyway.

As a side point, I believe (correct me if I'm wrong) that if you extend the model field then model forms will get that form field with the correct length, which is a pretty major use case.

in reply to:  5 comment:6 by Pablo Martín, 11 years ago

Replying to mjtamlyn:

Personally I don't think it's worth making changes to the form field now when we can make changes to the model field once migrations lands (which is on its way). I'd love it to be right, but I don't think it's worth having an intermediary step when the "real" fix is probably going to be in Django 1.7 anyway.

I updated the pull request, I had improve the code. Now the pull request is very very simple.

I have changed the form field because now (django master) if you have a forms.EmailField in a form the max length is not validated. Of the same way that in dbfield.EmailField. Something like this:

    class MyUserForm(forms.ModelForm):

        email_no_validate = forms.EmailField()

        class Meta:
            model = User
            fields = ('email', 'email_no_validate')


comment:7 by Pablo Martín, 11 years ago

I can change the name of the setting (and the logic) this is trivial. But please tell me some if you like it before....

comment:8 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted

Duplicate of #11365 which was closed as won't fix, but I guess we've decided to address it now.

I agree that it would be easiest to wait for schema migrations to land and then make this change when that happens.

comment:9 by Tim Graham, 11 years ago

Has patch: unset
Keywords: schemamigration added
Needs tests: unset
Patch needs improvement: unset
Summary: The max length of the email is 254Increase the default max_length of EmailField to 254

comment:10 by Tim Graham, 10 years ago

Has patch: set

comment:11 by Claude Paroz, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 7fd55c3481a004afb049e15ae3b8c93ce8bf0603:

Fixed #20631 -- Increased the default EmailField max_length to 254.

Thanks pmartin for the report.

comment:13 by Tim Graham <timograham@…>, 10 years ago

In 1f8bb95cc2286a882e0f7a4692f77b285d811d11:

Corrected domain max length for EmailValidator; refs #20631.

Thanks MarkusH for the report.

Note: See TracTickets for help on using tickets.
Back to Top