Opened 14 years ago

Closed 13 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 by Sasha Romijn, 14 years ago

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

comment:2 by Sasha Romijn, 14 years ago

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 by Sasha Romijn, 14 years ago

Owner: changed from Sasha Romijn to nobody
Status: assignednew

comment:4 by Sasha Romijn, 14 years ago

milestone: 1.3

comment:5 by aledr, 14 years ago

Cc: alexandre@… added

Just figured out that the correct should be:

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

Is it possible to change in ticket description?

Thanks.

in reply to:  5 ; comment:6 by aledr, 14 years ago

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.

in reply to:  6 comment:7 by JohnDoe, 14 years ago

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 by Julien Phalip, 14 years ago

Severity: Normal
Type: Bug

comment:9 by Aymeric Augustin, 13 years ago

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