Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#22630 closed Bug (needsinfo)

models.EmailField allows a dict to be saved without raising an error resulting in Python dict strings saved to the DB

Reported by: joe@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: email orm field validators
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We've been using the EmailField and recently found a bug where invalid email text was being saved. Here's an example of the text:

{u'verified': True, u'email': u'example@…', u'primary': True}

It looks like what's happening is the email validator splits on the @ sign and somehow the two sides validate.

Change History (7)

comment:1 by Baptiste Mispelon, 10 years ago

Resolution: needsinfo
Status: newclosed

Hi,

I can't seem to be able to reproduce your issue.
Here's the code I tried (both under master and 1.4):

>>> from django import forms
>>> 
>>> class EmailForm(forms.Form):
...     email = forms.EmailField()
... 
>>> 
>>> f = EmailForm({'email': "{u'verified': True, u'email': u'example@…', u'primary': True}"})
>>> f.is_valid()
False

(this uses forms but the same is true for models since they use the same validator)

I'll mark this ticket as "needs info". Can you reopen it with some information on how to reproduce the issue (ideally, a small piece of code that demonstrates your issue).

Thanks.

comment:2 by joe@…, 10 years ago

I am able to reproduce this by assigning a dict to the email. Also, please note this is a bug with the Django ORM and not (that I know of) an issue with the forms (though if they use the same validator this could possibly be an issue).

https://gist.github.com/joestump/9b76fd94378c960ed7c3

So the issue appears to be that there's no type checking combined with the blind split on the @ sign.

comment:3 by joe@…, 10 years ago

Summary: EmailField allows invalid text to be saved as long as an email address is inside the textmodels.EmailField allows a dict to be saved without raising an error resulting in Python dict strings saved to the DB

comment:4 by joe@…, 10 years ago

Using your example you can see it's failing on the forms as well:

In [14]: f = EmailForm({'email': {'test': 'foo', 'email': 'joe@example.com'}})

In [15]: f.is_valid()
Out[15]: True

comment:5 by Tim Graham, 10 years ago

In the example gist, you haven't actually invoked model validation before you saved the object. Does adding email.full_clean() before save() change anything?

comment:6 by joe@…, 10 years ago

I don't believe that will fix this issue given that the simple f.is_valid() returns True for a dict, but I will test and report back.

comment:7 by joe@…, 10 years ago

So that does raise an error:

In [1]: from wasatch.models import User

In [2]: from accounts.models import AlternateEmail

In [3]: email = AlternateEmail(user=User.objects.get(pk=1))

In [4]: email.email = {'verified': True, 'email': 'joe@example.com'}

In [5]: email.full_clean()
---------------------------------------------------------------------------
ValidationError                           Traceback (most recent call last)
/Users/jstump/Dropbox/jstump/.virtualenvs/sprint.ly/lib/python2.7/site-packages/django/core/management/commands/shell.pyc in <module>()
----> 1 email.full_clean()

/Users/jstump/Dropbox/jstump/.virtualenvs/sprint.ly/lib/python2.7/site-packages/django/db/models/base.pyc in full_clean(self, exclude)
    822 
    823         if errors:
--> 824             raise ValidationError(errors)
    825 
    826     def clean_fields(self, exclude=None):

ValidationError: {'email': [u'Enter a valid e-mail address.']}

Still not sure why that form is marked as valid though.

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