Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#11907 closed (fixed)

EmailField should run strip()

Reported by: whatcould Owned by: Chris Beaven
Component: Forms Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The EmailField should run strip() to remove opening and trailing spaces before the validation is run. Copying and pasting an email can easily pick up a trailing space (I did it myself today).

The code would be something like this, from forms.fields.EmailField. (My first python, sorry if it's ugly, I'm a rubyist.)

  def clean(self, value):
      value = value.strip()
      value = super(RegexField, self).clean(value)
      return value

Attachments (3)

11907.diff (535 bytes) - added by krisneuharth 7 years ago.
Patch for #11907
11907.2.diff (1.4 KB) - added by Daniel Gonzalez Gasull 7 years ago.
Fix with test.
11907.3.diff (1.3 KB) - added by Chris Beaven 6 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by Matthias Kestenholz

Resolution: duplicate
Status: newclosed

Duplicate of #6362

comment:2 Changed 7 years ago by whatcould

Resolution: duplicate
Status: closedreopened

This is clearly not a duplicate of #6362. Email is a specific kind of data -- #6362 is the general case. (Which, of course, should also be changed, but that's not the point here.)

Emails can not have spaces, so the email. Nothing to do with stripping spaces for other field types.

If the argument against #6362 is that sometimes you might want spaces in the data, that argument is moot here, because you would never want spaces in a user-entered email address. The examples people brought up in #6362, appropriately enough, are of users entering emails (with spaces) and being confused/frustrated.

comment:3 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:4 Changed 7 years ago by krisneuharth

Owner: changed from nobody to krisneuharth
Status: reopenednew

Changed 7 years ago by krisneuharth

Attachment: 11907.diff added

Patch for #11907

comment:5 Changed 7 years ago by krisneuharth

Has patch: set
Needs documentation: set
Needs tests: set

comment:6 Changed 7 years ago by krisneuharth

Has patch: unset
Needs documentation: unset
Needs tests: unset
Owner: changed from krisneuharth to nobody

comment:7 Changed 7 years ago by Daniel Gonzalez Gasull

Owner: changed from nobody to Daniel Gonzalez Gasull
Status: newassigned

comment:8 Changed 7 years ago by Daniel Gonzalez Gasull

Has patch: set
Needs documentation: set
Needs tests: set

Changed 7 years ago by Daniel Gonzalez Gasull

Attachment: 11907.2.diff added

Fix with test.

comment:9 Changed 7 years ago by Daniel Gonzalez Gasull

Needs documentation: unset
Needs tests: unset
Resolution: fixed
Status: assignedclosed

Added fix based on krisneuharth's diff. Added test.

comment:10 Changed 7 years ago by Alex Gaynor

Resolution: fixed
Status: closedreopened

No patch was committed to Django, tickets are not fixed when a patch is uploaded.

comment:11 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:12 Changed 7 years ago by whatcould

This sounds a lot like a bug to me, James, not a feature. Emails are emails, and should be formatted as such. Why do you call the thing "email field" anyway if you let any old spaces in?

I'd wager users who try to enter emails, include an incidental space, and then can't log in -- they think it's a bug.

comment:13 Changed 7 years ago by James Bennett

Well... actually it's part of (and thus now being closed as a duplicate of) #6362, which asks for whitespace stripping to be applied before validating data in all text-based field types. And that's definitely a feature request, so once again there's no way for this to make it into 1.2. The best way to advance this is to hold on for a bit while 1.2 gets out the door, then lobby for it to get into 1.3.

comment:14 Changed 7 years ago by James Bennett

Resolution: duplicate
Status: reopenedclosed

comment:15 Changed 7 years ago by whatcould

ubernostrum, if you see my comment above (02/04/10 00:25:26) I explained why this should be marked as a separate bug, instead of being thrown into the 2-year-old black hole that is that other bug. Someone else apparently agreed at the time.

This is all no skin off my back (I was consulting on a django project that's finished) -- I was just trying to follow through on this bug. Wish it hadn't felt like pushing on a string.

-whatcould (fellow joyeur)

comment:16 Changed 6 years ago by Chris Beaven

Resolution: duplicate
Status: closedreopened

This is in the same basket as #5714 (also originally marked as a dupe of #6362).

Malcolm reopened that one, so I'm going to reopen this one.

Changed 6 years ago by Chris Beaven

Attachment: 11907.3.diff added

comment:17 Changed 6 years ago by Chris Beaven

Owner: changed from Daniel Gonzalez Gasull to Chris Beaven
Status: reopenednew
Triage Stage: AcceptedReady for checkin

comment:18 Changed 6 years ago by Adrian Holovaty

Resolution: fixed
Status: newclosed

(In [13997]) Fixed #11907 -- EmailField now runs strip() on its input. This means mistakenly including leading or trailing spaces will not cause a validation error, and clean() will remove those spaces. Thanks, krisneuharth, djansoft and SmileyChris

comment:19 Changed 6 years ago by Chris Beaven

(In [14124]) [1.2.X] Fixed #11907 -- EmailField now runs strip() on its input. This means mistakenly including leading or trailing spaces will not cause a validation error, and clean() will remove those spaces. Thanks, krisneuharth and djansoft. Backport of [13997].

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