#6967 closed (fixed)
ModelForms doesn't validate CHOICES
| Reported by: | Owned by: | Jacob | |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Keywords: | ||
| Cc: | simon@…, carl@…, ville@…, pat.j.anderson@…, web@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
A ModelForm with a CharField, that has CHOICES set, presents to the user only the specified CHOICES. But it does not _validate_ against the CHOICES, which allows a crafted response to write arbitrary values to the field.
Attachments (3)
Change History (23)
by , 18 years ago
| Attachment: | choices-fix.patch added |
|---|
comment:1 by , 18 years ago
comment:2 by , 18 years ago
| Cc: | added |
|---|
comment:3 by , 17 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:4 by , 17 years ago
| milestone: | → 1.0 |
|---|
by , 17 years ago
| Attachment: | 6967-tests.diff added |
|---|
Some tests for choices on CharField and IntegerField.
comment:5 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 17 years ago
comment:7 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:8 by , 17 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
The test was insufficient, unfortunately. Field.formfield is going to need to cope with other model field types that set arbitrary arguments in their defaults dict which would make sense for the form field they expect to use, but won't make sense to TypedChoiceField. The only built-in examples of this are PositiveIntegerField and PositiveSmallIntegerField, which pass a min_value argument.
We could pop off min_value as max_length is being popped now, but it's just fixing the symptom. A custom model field could set anything.
comment:9 by , 17 years ago
I fell across this just now too. For now I just changed my local copy of the pop() line to:
kwargs = kwargs.get('choices_kwargs', {})
Still not the nicest solution, but it isn't as bad as the current one.
comment:10 by , 17 years ago
| Has patch: | set |
|---|
comment:11 by , 17 years ago
| Cc: | added |
|---|
comment:13 by , 17 years ago
| Cc: | added |
|---|
comment:14 by , 17 years ago
| Cc: | added |
|---|
comment:15 by , 17 years ago
| Cc: | added |
|---|
comment:17 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
comment:18 by , 17 years ago
I'm getting this bug as well (running latest trunk as of two minutes ago). Here is my traceback:
Validating models...
Unhandled exception in thread started by <function inner_run at 0x6aef70>
Traceback (most recent call last):
File "/Library/Python/2.5/site-packages/django/core/management/commands/runserver.py", line 48, in inner_run
self.validate(display_num_errors=True)
File "/Library/Python/2.5/site-packages/django/core/management/base.py", line 249, in validate
num_errors = get_validation_errors(s, app)
File "/Library/Python/2.5/site-packages/django/core/management/validation.py", line 28, in get_validation_errors
for (app_name, error) in get_app_errors().items():
File "/Library/Python/2.5/site-packages/django/db/models/loading.py", line 131, in get_app_errors
self._populate()
File "/Library/Python/2.5/site-packages/django/db/models/loading.py", line 58, in _populate
self.load_app(app_name, True)
File "/Library/Python/2.5/site-packages/django/db/models/loading.py", line 74, in load_app
models = import_module('.models', app_name)
File "/Library/Python/2.5/site-packages/django/utils/importlib.py", line 35, in import_module
__import__(name)
File "/Users/stavros/Code/Django/head/main/models.py", line 215, in <module>
class AdType(models.Model):
File "/Users/stavros/Code/Django/head/main/models.py", line 221, in AdType
price = models.DecimalField(_("price"), max_digits=6, decimal_places=2, min_value=0)
File "/Library/Python/2.5/site-packages/django/db/models/fields/__init__.py", line 589, in __init__
Field.__init__(self, verbose_name, name, **kwargs)
TypeError: __init__() got an unexpected keyword argument 'min_value'
I'm not sure what this means, but I can't launch my server.
comment:19 by , 17 years ago
I'm sorry, disregard my post above. I shouldn't post bug reports while sleepless...
Attached patch appears to fix this, but needs tests.