Opened 2 years ago

Closed 13 months ago

Last modified 13 months ago

#20631 closed Bug (fixed)

Increase the default max_length of EmailField to 254

Reported by: pmartin Owned by: nobody
Component: Database layer (models, ORM) Version: master
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 Changed 2 years ago by pmartin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Fixed with this pull request:

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

comment:2 Changed 2 years ago by russellm

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 2 years ago by mjtamlyn

  • Cc marc.tamlyn@… added
  • Has patch set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Accepted to Unreviewed

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 Changed 2 years ago by pmartin

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 follow-up: Changed 2 years ago by 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.

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.

comment:6 in reply to: ↑ 5 Changed 2 years ago by pmartin

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 Changed 2 years ago by pmartin

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 Changed 2 years ago by timo

  • Triage Stage changed from Unreviewed to Accepted

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 Changed 2 years ago by timo

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

comment:10 Changed 13 months ago by timo

  • Has patch set

comment:11 Changed 13 months ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 13 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 7fd55c3481a004afb049e15ae3b8c93ce8bf0603:

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

Thanks pmartin for the report.

comment:13 Changed 13 months ago by Tim Graham <timograham@…>

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