[Django] #28555: Inconsistent empty value for EmailField(blank=True, null=True) due to strip after check for empty value
Django
after check for empty value
---------------------------------------+------------------------
Reporter: Olav Morken | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
Hi,
I just noticed that if I enter a single space in an EmailField in a
ModelForm where the backing model field is {{{EmailField(blank=True,
null=True)}}}, the value is stored as an empty string. On the other hand,
if I don't enter anything in the field, it is stored as a {{{None}}}
value.
Looking at the code
(https://github.com/django/django/blob/1.11.4/django/forms/fields.py#L234-L241),
I think the cause is that it first checks for empty values, and then
strips the string.
To reproduce this behavior, try something like:
{{{#!python
# models.py
from django.db import models
class SomeUser(models.Model):
address = models.EmailField(null=True, blank=True)
# views.py
import pprint
from django import forms
from django.http import HttpResponse
from django.views.decorators.csrf import csrf_exempt
from .models import SomeUser
class SomeUserForm(forms.ModelForm):
class Meta:
fields = [ 'address' ]
model = SomeUser
@csrf_exempt
def test_view(request):
form = SomeUserForm(request.POST)
some_user = form.save()
return HttpResponse(pprint.pformat(some_user.address) + "\n")
}}}
Testing this with {{{curl}}} gives something like this:
{{{
$ curl -XPOST -d address= http://127.0.0.1:1253/test
None
curl -XPOST -d address=%20 http://127.0.0.1:1253/test
u''
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28555>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
after check for empty value
Reporter: Olav Morken | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:1>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Josh Schneier):
* status: new => assigned
* owner: nobody => Josh Schneier
* has_patch: 0 => 1
* version: 1.11 => master
Comment:
[https://github.com/django/django/pull/9019 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:2>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* cc: Tim Martin (added)
* needs_better_patch: 0 => 1
Comment:
The change looks good to me, but it should be squashed into a single
commit.
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:3>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
Comment:
Thanks for reviewing. Squashing commits can be done easily enough when
merging so you don't need to check "Patch needs improvement" for that.
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:4>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Josh Schneier):
Replying to [comment:3 Tim Martin]:
> The change looks good to me, but it should be squashed into a single
commit.
In this case I intentionally did not rebase down because of the additional
fix/patch I added. I'm not sure if it's necessary and wanted to leave the
history for the reviewer to help with that determination.
Thanks for the review!
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:5>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Martin):
* stage: Accepted => Ready for checkin
Comment:
I think the `non_string` unit test makes it pretty clear why you reworked
it the way you did. At least, I was able to follow without looking through
all the patches.
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:6>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"48c394a6fc2594891f766293afec8f86d63e1015" 48c394a6]:
{{{
#!CommitTicketReference repository=""
revision="48c394a6fc2594891f766293afec8f86d63e1015"
Fixed #28555 -- Made CharField convert whitespace-only values to the
empty_value when strip is enabled.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:7>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
I was wondering if this fix could be eligible for a backport to 1.11.x. It
fixes a bug in the new features of:
https://docs.djangoproject.com/en/dev/releases/1.11/#forms
> The new CharField.empty_value attribute allows specifying the Python
value to use to represent “empty”.
https://docs.djangoproject.com/en/dev/releases/1.11/#miscellaneous
> In model forms, CharField with null=True now saves NULL for blank values
instead of empty strings.
The [https://docs.djangoproject.com/en/dev/internals/release-process
/#supported-versions documentation] says the following about fixing bugs
in newly introduced features:
> Major functionality bugs in new features of the latest stable release.
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:8>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1d1a56c59983ae40675aff2c737bdde8f988e5e9" 1d1a56c]:
{{{
#!CommitTicketReference repository=""
revision="1d1a56c59983ae40675aff2c737bdde8f988e5e9"
[1.11.x] Fixed #28555 -- Made CharField convert whitespace-only values to
the empty_value when strip is enabled.
Backport of 48c394a6fc2594891f766293afec8f86d63e1015 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:9>
Django
after check for empty value
Reporter: Olav Morken | Owner: Josh Schneier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"3d2c3905a606f373d1b4ad386ad8343112f384f7" 3d2c3905]:
{{{
#!CommitTicketReference repository=""
revision="3d2c3905a606f373d1b4ad386ad8343112f384f7"
Refs #28555 -- Forwardported 1.11.6 release note.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28555#comment:10>