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 , 14 years ago
Keywords: | CommaSeparatedIntegerField added; comma separated removed |
---|---|
milestone: | → 1.3 |
Needs tests: | set |
Owner: | changed from | to
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Needs tests: | unset |
Triage Stage: | Accepted → Design decision needed |
comment:3 by , 14 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:4 by , 14 years ago
milestone: | 1.3 |
---|
follow-up: 6 comment:5 by , 14 years ago
Cc: | added |
---|
Just figured out that the correct should be:
re.compile('^(\d,)*\d$')
Is it possible to change in ticket description?
Thanks.
follow-up: 7 comment:6 by , 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.
comment:7 by , 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 , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:9 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
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.
Oh, just noticed this actually seems to be by design:
From tests/modeltests/model_forms/models.py:
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.