Opened 3 years ago

Closed 2 years ago

#18761 closed Cleanup/optimization (fixed)

Strip fields' values consistently

Reported by: vzima Owned by: deni
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Currently, some fields strips values during validation and some do not. The state does not even clearly represents whether whitespace is valid part of field's value.

Field Strips Should strip
CharField no no
IntegerField yes yes
FloatField yes yes
DecimalField yes yes
BaseTemporalField yes yes
RegexField no no
EmailField yes yes
URLField no yes
BooleanField no ?
NullBooleanField no ?
IPAddressField no yes
GenericIPAddressField no yes
SlugField no yes

Comments:

  • Spaces are not allowed in URLField, IPAddressField, GenericIPAddressField and SlugField
  • Fairly unclear are boolean fields. IMHO if value to validate is string of whitespace characters, it should validate to None/False as when represented by TextInput user will see empty field.

I have not check localised form fields but I expect more fields that can be safely trimmed.

Maybe a good long-term solution would be list of filters (like validators) which would perform transformation to python object.

Attachments (1)

ticket_18761.diff (1.3 KB) - added by Melevir 3 years ago.
Added stripping for URLField, IPAddressField, GenericIPAddressField, SlugField. I have two questions: is it all that should be done? Why no tests are required?

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 years ago by melinath

  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I've actually gotten this as a feature request on projects I've worked on in the wild... I assumed that django just wasn't stripping any field values of whitespace. But it is, either implicitly or explicity. It would be good to be consistent. So it would make sense (to me) to strip whitespace on !URLField, !IPAddressField, !GenericIPAddressField, and SlugField - maybe leave the BooleanFields for later/never, since the case isn't clear.

comment:2 Changed 3 years ago by Melevir

  • Owner changed from nobody to Melevir

comment:3 Changed 3 years ago by Melevir

  • Status changed from new to assigned

Changed 3 years ago by Melevir

Added stripping for URLField, IPAddressField, GenericIPAddressField, SlugField. I have two questions: is it all that should be done? Why no tests are required?

comment:4 Changed 3 years ago by Melevir

  • Has patch set

comment:5 Changed 2 years ago by claudep

  • Needs tests set

Note that this has been fixed for IPAddressField/GenericIPAddressField in #17751.

comment:6 Changed 2 years ago by erikr

  • Owner changed from Melevir to erikr

comment:7 Changed 2 years ago by erikr

  • Owner erikr deleted
  • Status changed from assigned to new

comment:8 Changed 2 years ago by deni

  • Owner set to deni
  • Status changed from new to assigned

comment:9 Changed 2 years ago by aaugustin

  • Needs tests unset

comment:10 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

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

In 7d050e8e9ce0e08aefe035a7e203c764fbfb544b:

Merge pull request #1113 from denibertovic/master

Fixed #18761 -- Added whitespace stripping to URLField and SlugField.

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