Opened 7 years ago

Closed 5 years ago

#13646 closed Bug (wontfix)

Wrong RE in comma_separated_int_list_re

Reported by: aledr Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: CommaSeparatedIntegerField
Cc: alexandre@… Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The actual RE validator in comma_separated_int_list_re for CommaSeparatedIntegerField will allow some kind of following values:

',,,' '1,,2,3,'

It is

comma_separated_int_list_re = re.compile('^[\d,]+$')

and should be:

comma_separated_int_list_re = re.compile('^[\d,]*\d$')

Change History (9)

comment:1 Changed 7 years ago by Erik Romijn

Keywords: CommaSeparatedIntegerField added; comma separated removed
milestone: 1.3
Needs tests: set
Owner: changed from nobody to Erik Romijn
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by Erik Romijn

Component: UncategorizedDatabase layer (models, ORM)
Needs tests: unset
Triage Stage: AcceptedDesign decision needed

Oh, just noticed this actually seems to be by design:

From tests/modeltests/model_forms/models.py:

>>> f = CommaSeparatedIntegerForm({'field': '1,,2'})
>>> f.is_valid()
True

So, this seems to be an intentional feature, and not a bug.
I'll mark this as design decision needed, for someone more experienced to have a look at it.

comment:3 Changed 7 years ago by Erik Romijn

Owner: changed from Erik Romijn to nobody
Status: assignednew

comment:4 Changed 7 years ago by Erik Romijn

milestone: 1.3

comment:5 Changed 7 years ago by aledr

Cc: alexandre@… added

Just figured out that the correct should be:

re.compile('^(\d,)*\d$')

Is it possible to change in ticket description?

Thanks.

comment:6 in reply to:  5 ; Changed 7 years ago by aledr

Replying to aledr:

Just figured out that the correct should be:

re.compile('!^(\d,)*\d$')

Is it possible to change in ticket description?

Thanks.

Wrong again, sorry... It's not possible to use '^' in this RE, It will always fail.

After tests done, correct is:

re.compile('(\d,)*\d$')

It will only allow integers, not nulls or commas or anything else comma separated.

comment:7 in reply to:  6 Changed 7 years ago by JohnDoe

Replying to aledr:

Replying to aledr:

Just figured out that the correct should be:

re.compile('!^(\d,)*\d$')

Is it possible to change in ticket description?

Thanks.

Wrong again, sorry... It's not possible to use '^' in this RE, It will always fail.

After tests done, correct is:

re.compile('(\d,)*\d$')

It will only allow integers, not nulls or commas or anything else comma separated.

What you're doing here doesn't make much sense.

IF this is how it should be designed, the answer is

re.compile('^(\d+,)*\d+$')

Still, there's no final design decision.

comment:8 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:9 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset
Resolution: wontfix
Status: newclosed
UI/UX: unset

There are tests for the current behavior, so it appears to be intentional.

I don't think it's worth breaking backwards compatibility.

It's easy to write your own subclass with a different regex.

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