resurrecting an old friend, whitespace in forms, #6362

[frantisek holop]

good day,

a recent technical support incident conducted remotely and
involving a lot of back and forth of "huh? but i have entered
what you sent me" left me my head scratching.

the reason turned out to be a trailing space in the username of
the django admin loginform (thank god for nginx's "$request_body"
log_format parameter).

this of course sent me on an archeological journey into the lands
of stackoverflow, blogs, and finally #6362 and this mailing list.
it has been some 5 years now since the decision on that
controversial ticket.

i also went through the whole emotianal rollercoaster:

how come, so many batteries are included, but when it comes to
this essential POST best practice, i needed in every single
webform i have ever made, and now i have to do it myself?
error-prone and not DRY. especially for the admin login form,
it is a usability issue.

vs

the BDFL is right, silently discarding user input is just wrong.
just use a package like happyforms[1], or pick a stackoverflow
answer and be done with it.


but wait, then HTML is also wrong, because it silently folds all
whitespace into 1 piece of space, we are all used to this. even
if the user entered whitespace is saved, pushing it back onto the
web will silently corrupt it (unless taken care of). i am not
saying this requirement does not exist for someone, somewhere,
but i have yet to see a site in the wild that needs this (hello,
ascii art people). whitespace in fields was always reserved for
government sites :)


it seems to me that there is a vocal group (majority?) that would
welcome a simple switch to make whitespace go away _now_, instead
of waiting for that perfect solution lurking in the future along
the lines of a generic normalize kwarg, or a flag on _every_
{Char,Text}Field on the model or overriding form.fields
attributes like required.

apps that need to preserve the whitespace are the exception,
not the rule, and that is why i would prefer not to start every
project by overriding BaseForm._clean_fields[2].

so i would like to present another idea for a possibe solution, a
proposal i have not seen so far: to have a global setting like
FORM_STRIP_FIELDS=True or some such and then roughly:

diff --git a/django/forms/forms.py b/django/forms/forms.py
index c9b8cf2..aab737a 100644
--- a/django/forms/forms.py
+++ b/django/forms/forms.py
@@ -8,6 +8,7 @@ from collections import OrderedDict
import copy
import datetime

+from django.conf import settings
from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
from django.forms.fields import Field, FileField
from django.forms.utils import flatatt, ErrorDict, ErrorList
@@ -355,6 +356,8 @@ class BaseForm(object):
if isinstance(field, FileField):
initial = self.initial.get(name, field.initial)
value = field.clean(value, initial)
+ elif isinstance(value, basestring) and settings.FORM_STRIP_FIELDS:
+ value = field.clean(value.strip())
else:
value = field.clean(value)
self.cleaned_data[name] = value

i know, it is a big hammer, but for me, it is like the timezone,
or csrf, i'd like to just set it, and forget about it.

-f

[1] https://pypi.python.org/pypi/happyforms
[2] http://chriskief.com/2012/12/21/trim-spaces-in-django-forms/
--
nobody can be exactly like me. even i have trouble doing so.

[Collin Anderson]

Hi frantisek,

I've also ran into a number of problems with extra whitespace in forms. Pretty annoying.

I also find settings pretty annoying. :) We were just talking on another thread [1] about the possibility of packaging packaging django.forms as a standalone package, but global settings are one of the main roadblocks to doing that.

Here are a few more possible solutions:

1.  Include some javascript on your page that .trim()s everything, maybe onsubmit.

2.  form = Form({k: v.strip() for k, v in request.POST.items()})

3.  Use a custom form subclass that strips the incoming values in __init__

4.  call strip() on model.save() or a pre_save signal.

[1] https://groups.google.com/d/msg/django-developers/WF3My-cZNtY/V0Z8QkQaOnAJ

Collin

[Tom Christie]

Trimming at `request.POST` or at the `Form` layer absolutely isn't sensible, and a `normalize` argument is probably just overcomplicating things, but I got the impression from that ticket that nobody would have any great issue with someone issuing a patch with a `trim_whitespace` flag on CharFields and TextFields.

The only thing beside then to resolve would be if we'd be okay with *its default being True*, despite the (edge case) backward incompatibility that'd cause. That seems to me to be simple and desirable behavior, doesn't need any extra settings, and you'd only need to override that argument in the bizarre cases where you *don't* want to strip leading and trailing whitespace.

Granted that ticket's been around for an *awfully* long time, but it's only a case of someone actually acting on it, and seeking a consensus on a sensible option, so...

Cheers,

  Tom

[frantisek holop]

Tom Christie, 03 Feb 2015 12:53:

> Trimming at `request.POST` or at the `Form` layer absolutely isn't
> sensible, and a `normalize` argument is probably just overcomplicating
> things, but I got the impression from that ticket that nobody would have
> any great issue with someone issuing a patch with a `trim_whitespace` flag
> on CharFields and TextFields.

i'd be more than happy with a flag like that, provided
it is True by default...

i don't feel strongly about any of the solutions as
long as i don't have to change all my models to add a
flag i always need...

i think wontfix-ing the ticket sent the wrong
signal, and people stopped trying to seek a consensus
on something that is perceived as a dead-end..

-f
--
in an atomic war, all men will be cremated equal.

[Russell Keith-Magee]

On Wed, Feb 4, 2015 at 6:49 AM, frantisek holop <min...@obiit.org> wrote:

Tom Christie, 03 Feb 2015 12:53:
> Trimming at `request.POST` or at the `Form` layer absolutely isn't
> sensible, and a `normalize` argument is probably just overcomplicating
> things, but I got the impression from that ticket that nobody would have
> any great issue with someone issuing a patch with a `trim_whitespace` flag
> on CharFields and TextFields.

Jacob specifically said a good patch for this feature would be considered when he wontfixed #6362.

 

i'd be more than happy with a flag like that, provided
it is True by default...

I can tell you that this *won't* happen - firstly because it will be backwards incompatible for every Django installation out there, but also because throwing away data that the user actually entered should be an opt-in, not opt-out behavior.

 

i don't feel strongly about any of the solutions as
long as i don't have to change all my models to add a
flag i always need...

 

You wouldn't be changing it on a model - it would be on the form.

i think wontfix-ing the ticket sent the wrong
signal, and people stopped trying to seek a consensus
on something that is perceived as a dead-end..

Read the comment where #6362 was closed WONTFIX, and you'll see Jacob left the door open for a "strip"-style approach, and specifically asked of for a patch implementing that behavior. WONTFIX is a single flag - sometimes you need to read the context as well. 

Yours,

Russ Magee %-)

[Tom Christie]

> it will be backwards incompatible for every Django installation out there, but also because throwing away data that the user actually entered should be an opt-in, not opt-out behavior.

If adequately called out I think there'd be a valid case that the current and future issues it'll be causing to applications right now would outweigh the risk of compatibility breakages. I can see a couple of cases where I might not want stripped whitespace, but only in slightly contrived and edge case situations.

Validating and normalizing input at the field level in a way that supports the common case by default seems like a good plan to me. I'm not sure I see any great difference between a user entering "3.1" in an integer field and having 3 returned is so very different from having "hello " return "hello" on a char field. And we're not of course throwing anything away at the `request.POST` or `Form` layer - it's only once we're validating an individual field that the normalization is being applied.

I don't have an great investment in this either way around, but I think it's a valid user-focused improvement worth considering *if* someone actually puts together a pull request for it.

[frantisek holop]

Russell Keith-Magee, 04 Feb 2015 08:26:

> Jacob specifically said a good patch for this feature would be considered
> when he wontfixed #6362.

for me, leaving a ticket open is a better signal for
inviting discussion and patches. when i see a wontfix
closed ticket in other projects it usually means "end
of story, dont bother" and/or "this is not an issue
that needs fixing". but that's just me :)

> because throwing away data that the user actually entered should be an
> opt-in, not opt-out behavior.

data: yes, leading/trailing whitespace: no.

this is the critical point where we disagree. when it
comes to that whitespace, it should be opt-out.

the web also seems to disagree with you. try any of the
forms starting at google search, usernames in login
forms, contact forms, they all throw away
leading/trailing whitespace -- and it's one less
problem their helpdesks must deal with.

leading/trailing whitespace is a usability issue, and a
mistake people make who do not realise how "unforgiving"
computers are when presented with
'username ' != 'username'. for them whitespace is "air".

but i think the point is made and clear. of course
it's possible to deal with this in django, it's just
more hassle than it could be and right now i feel
punished. i think many django projects simply
ignore the issue and delegate the blame to the user.


i am thinking of raising a ticket about the admin's
loginform not stripping its username, esp when adding a
user is actually protected against whitespace.
would that make sense? it is trivial to fix :)

-f
--
underneath all these clothes i am completely naked!

[Tom Christie]

> leaving a ticket open is a better signal for inviting discussion and patches.

There's been over 22,000 issues raised in Django's history. It's understandable and desirable that maintainers close off tickets aggressively if they don't think they should be tackled.

I wouldn't get too hung up on the process there - a sound argument and a bit of proactive work is all that's needed to convince folks to take the time to reassess something like that issue. Personally I think there's a good case to be made for this one so from my point of view, so I'd say go ahead, make the proposal, or better still a pull request, make the argument and *then* see if it is possible to reach a consensus.

There's clearly absolutely no guarantee it'd be accepted (there's two core members in this thread making different judgement calls on it, so it's *far* from cut & dried) but you can be sure that folks are willing to put the time in to listen to anyone willing to put in a bit of legwork.

Cheers,

  Tom :)

[Carl Meyer]

I agree with all of what Tom says.

Carl

[Shai Berger]

I agree with this as well; I'd argue, though, that there is a difference
between leading and trailing whitespace. Leading whitespace is usually visible
and, to my mind, much more likely to be intentional.

Shai.

[Collin Anderson]

Hi All,

I'd be ok with a well thought-out strip-by-default.

- I think most of my problems have been with trailing whitespace on CharFields.

- I once have seen a minor unintentional leading whitespace. I think I also may have once used a leading whitespace for sorting purposes, but I'd be ok with even having leading whitespace stripped by default.

I can't think of many cases where trailing whitespace has been an issue for TextFields. Has this been an issue for people? I could imagine some people would want a trailing newline on TextFields. However, peeking at what other frameworks do, I'd be ok with this by default too :)

Also, did we decide if the Model-field-layer or Form-field-layer would be better?

Collin

[charettes]

Like Russ I think this should be at the form level.

Since both db.CharField and db.TextField use a form.CharField the stripping implementation should be done there.

I commented here because I recently fixed a related bug in an application I maintain where a trailing white-space was not striped from an URLField form input. Since browsers automatically strip trailing white-space in urls (e.g. href=" example.com/path " == href="example.com/path") the anchor in the widget we provide was pointing to the correct resource when clicked by the user. However, when our system appended a querystring (By using the urllib and urlparse modules) for analytics purposes (e.g "example.com/path ?param=value") the white-space ended up making the whole URL invalid since the "/path " lead to a 404.

Simon

[Curtis Maloney]

I'm certainly leaning on the side of "strip by default" because, like many others, I feel it's the expected default -- users just don't get it -- and have been bitten by it before.

I think I'll have a PR for this later tonight...

--

Curtis

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ff91cec7-90a2-4db3-8747-97d333ad218b%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Tim Chase]

[slightly reordered]

On 2015-02-04 18:25, Collin Anderson wrote:
> Also, did we decide if the Model-field-layer or Form-field-layer
> would be better?

I think the Form-field layer is definitely the place for it. If I do

my_model.my_text_field = " leading and trailing "

I expect that value to make it in there, spaces and all. But if it
comes from a form, the form-field should default to assuming that
leading/trailing whitespace is an accident. There have been plenty
of times that I've copied/pasted data and missed that
leading/trailing whitespace was included. E.g.: copying from Excel
is notoriously bad at including trailing newlines.

> I can't think of many cases where trailing whitespace has been an
> issue for TextFields. Has this been an issue for people? I could
> imagine some people would want a trailing newline on TextFields.

My password-generator/manager can include spaces in passwords which
occasionally come up at the beginning/end of the resulting password.
Since it auto-types the value for me, I don't usually give it a
thought. But it has stung me once or twice when a site has a password
field that eats the whitespace. Thus stung, I tend to no longer
include spaces in my generated passwords these days. But again, at
the form-level, a TextInput could strip while a PasswordInput could
retain, and the underlying models.CharField would preserve whatever
the form hands to it.

My $0.01 on the matter,

-tkc