Opened 4 years ago

Closed 3 years ago

#18761 closed Cleanup/optimization (fixed)

Strip fields' values consistently

Reported by: Vlastimil Zíma Owned by: Deni Bertovic
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 Lebedev Ilya 4 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 4 years ago by melinath

Easy pickings: set
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

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 4 years ago by Lebedev Ilya

Owner: changed from nobody to Lebedev Ilya

comment:3 Changed 4 years ago by Lebedev Ilya

Status: newassigned

Changed 4 years ago by Lebedev Ilya

Attachment: ticket_18761.diff added

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 4 years ago by Lebedev Ilya

Has patch: set

comment:5 Changed 3 years ago by Claude Paroz

Needs tests: set

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

comment:6 Changed 3 years ago by Erik Romijn

Owner: changed from Lebedev Ilya to Erik Romijn

comment:7 Changed 3 years ago by Erik Romijn

Owner: Erik Romijn deleted
Status: assignednew

comment:8 Changed 3 years ago by Deni Bertovic

Owner: set to Deni Bertovic
Status: newassigned

comment:9 Changed 3 years ago by Aymeric Augustin

Needs tests: unset

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

Resolution: fixed
Status: assignedclosed

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