Opened 8 years ago

Closed 8 years ago

#24531 closed Bug (fixed)

CommaSeparatedIntegerField validation not specific enough

Reported by: Bertrand Bordage Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The CommaSeparatedIntegerField validator, validate_comma_separated_integer_list, currently allows strange values such as ',', ',,', ',1,2,', '1,,2'.

Two issues here:

  • In my opinion,',' and ',,' can’t mean anything else than “no info”. One may argue that it can mean “no info, no info” and “no info, no info, no info”, but that’s weird and what it could mean about your data isn’t clear. I think this only adds confusion compared to ''.
  • Most people won’t expect this and will only apply string.split(',') on it, which may lead to unexpected empty strings in the resulting list.

Pull request 4133 proposes a fix by using a more specific regular expression to validate data.
It also introduces int_list_validator, a function to make it easier to create a similar validator with another separator than a comma. This is useful in some languages like French where comma is used as a decimal separator. Some french people may read comma-separated values as english would read 10.5.900, which is misleading. Using other separators like ; is therefore easy using this int_list_validator function.

Change History (2)

comment:1 Changed 8 years ago by Tim Graham

Component: FormsCore (Other)
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 3e64f3d:

Fixed #24531 -- Improved CommaSeparatedIntegerField validation.

',', '1,,1', ',1' etc. are no longer considered as valid
comma-separated integer lists.

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