Data entered in raw_id_fields needs better checking

[Karen Tracey]

I stumbled across this while trying to verify the patch for #8648. If you have a ForeignKey listed in raw_id_fields and enter an incorrect value in it, the admin code will likely thrown an exception. In the case I ran across in #8648 the incorrect value is an integer with no associated related object. The admin code attempts to re-display the form with an error message about the value being invalid, but the raw id widget in an attempt to be helpful and display the printable representation of the object referred to in the form generates an exception when it assumes it can get the associated object.

A 2nd way to cause a (different) exception is to enter something that isn't an integer at all. In this case I got:

Environment:

Request Method: POST
Request URL: http://lbox:8000/admin/crossword/clues/2518/
Django Version: 1.0-beta_2-SVN-8769
Python Version: 2.5.1
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.admin',
 'django.contrib.sites',
 'django.contrib.humanize',
 'xword.crossword']
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.middleware.doc.XViewMiddleware')


Traceback:
File "/home/kmt/tmp/django/trunk/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/home/kmt/tmp/django/trunk/django/contrib/admin/sites.py" in root
  173.                 return self.model_page(request, *url.split('/', 2))
File "/home/kmt/tmp/django/trunk/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/home/kmt/tmp/django/trunk/django/contrib/admin/sites.py" in model_page
  192.         return admin_obj(request, rest_of_url)
File "/home/kmt/tmp/django/trunk/django/contrib/admin/options.py" in __call__
  196.             return self.change_view(request, unquote(url))
File "/home/kmt/tmp/django/trunk/django/db/transaction.py" in _commit_on_success
  238.                 res = func(*args, **kw)
File "/home/kmt/tmp/django/trunk/django/contrib/admin/options.py" in change_view
  571.             if form.is_valid():
File "/home/kmt/tmp/django/trunk/django/forms/forms.py" in is_valid
  120.         return self.is_bound and not bool(self.errors)
File "/home/kmt/tmp/django/trunk/django/forms/forms.py" in _get_errors
  111.             self.full_clean()
File "/home/kmt/tmp/django/trunk/django/forms/forms.py" in full_clean
  218.                     value = field.clean(value)
File "/home/kmt/tmp/django/trunk/django/forms/models.py" in clean
  527.             value = self.queryset.get(pk=value)
File "/home/kmt/tmp/django/trunk/django/db/models/query.py" in get
  295.         clone = self.filter(*args, **kwargs)
File "/home/kmt/tmp/django/trunk/django/db/models/query.py" in filter
  481.         return self._filter_or_exclude(False, *args, **kwargs)
File "/home/kmt/tmp/django/trunk/django/db/models/query.py" in _filter_or_exclude
  499.             clone.query.add_q(Q(*args, **kwargs))
File "/home/kmt/tmp/django/trunk/django/db/models/sql/query.py" in add_q
  1191.                             can_reuse=used_aliases)
File "/home/kmt/tmp/django/trunk/django/db/models/sql/query.py" in add_filter
  1135.         self.where.add((alias, col, field, lookup_type, value), connector)
File "/home/kmt/tmp/django/trunk/django/db/models/sql/where.py" in add
  48.                 params = field.get_db_prep_lookup(lookup_type, value)
File "/home/kmt/tmp/django/trunk/django/db/models/fields/__init__.py" in get_db_prep_lookup
  202.             return [self.get_db_prep_value(value)]
File "/home/kmt/tmp/django/trunk/django/db/models/fields/__init__.py" in get_db_prep_value
  337.         return int(value)

Exception Type: ValueError at /admin/crossword/clues/2518/
Exception Value: invalid literal for int() with base 10: 'akdakdf'

Not sure whether this should be one or multiple tickets, but I'll start with one under the assumption that "making admin safe from bad input placed in a raw id widget" can be viewed as one task, even if bad input today can lead to different errors depending on what flavor of 'bad' you feed to admin.

Specifically not marking this 1.0 since an easy workaround is to just not enter invalid values in these fields. Might be nice to fix it post-1.0, though, to save people from surprising results due to typos and such.

[slowik]

This bug is not case of admin site but form validating. I receive sort of this when I try submit ModelForm with choices to integer field like this

birth_begin=models.PositiveSmallIntegerField('',choices=gen_birth_choices(),null=True)

with no choice of birth

error:

/python/django/core/handlers/base.py in get_response 
...
                response = callback(request, *callback_args, **callback_kwargs) ...
▶ Local vars 
/python/django/contrib/auth/decorators.py in __call__ 
...
            return self.view_func(request, *args, **kwargs) ...
▶ Local vars 
/apps/users/views.py in search_for 
...
                form.save() ...
▶ Local vars 
/python/django/forms/models.py in save 
...
        return save_instance(self, self.instance, self._meta.fields, fail_message, commit) ...
▶ Local vars 
/python/django/forms/models.py in save_instance 
...
        instance.save() ...
▶ Local vars 
/python/django/db/models/base.py in save 
...
        self.save_base(force_insert=force_insert, force_update=force_update) ...
▶ Local vars 
/python/django/db/models/base.py in save_base 
...
                    values = [(f, None, f.get_db_prep_save(raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks] ...
▶ Local vars 
/python/django/db/models/fields/__init__.py in get_db_prep_save 
...
        return self.get_db_prep_value(value) ...
▶ Local vars 
/python/django/db/models/fields/__init__.py in get_db_prep_value 
...
        return int(value) 

Exception Type:	ValueError
Exception Value:	invalid literal for int() with base 10: ''
Exception Location:	/python/django/db/models/fields/__init__.py in get_db_prep_value, line 671
Python Executable:	/usr/bin/python
Python Version:	2.5.2

[Karen Tracey]

I am not convinced you are running into the same problem this ticket was opened to resolve. You don't give enough details of what you are doing, but the error you are seeing could be the result of having blank=True but not null=True on your model field. django-users would be a better place to try to resolve what you are seeing, this ticket was really opened to address what I believe is specifically an admin problem, not a general forms validation problem.

[adroffne@…]

I just discovered this raw_id_fields bug and also believe it is lacking Form validation. The raw-id field should raise a ValidationError when the contents is not the right data type.

I have a ManyToMany model that has a through model. An Inline admin class links the through model to the parent model. In my case, the primary key is declared to be a CharField, a US ZIP code, not a plain integer key.

The raw_id_fields fix must determine the primary key datatype of the model, type(instance.pk), first. Then, its Form field raises ValidationError when TypeError is thrown.

[Karen Tracey]

#9484 was a dup.

[jfw]

I had a similar issue with raw_id_fields: if users type into the field, sometimes they'll leave extra commas or spaces. Extra spaces are fine, though should be stripped--extra commas makes the query die by trying to convert " " or "" to an int. Attached is a patch to at least fix that. I agree that it should have better checking as to whether or not the values are valid for the particular key, but I'm not sure (yet) how to do that without assuming that the key is always an integer.

[(none)]

Milestone post-1.0 deleted

[Jacob]

(In [10233]) Fixed #8746: Check data in raw_id_fields more closely. Thanks, dgouldin

[Silvio J. Gutierrez]

Seems like only M2M fields were fixed. Someone submitted #9484 reporting this, but the ticket was closed and linked to this fix. ForeignKey fields with raw_id_fields enabled will produce an exception if anything but an int, or no value, is entered.

Error:

Django Version: 1.1 beta 1 SVN-10934

Traceback:
File "/var/lib/python-support/python2.6/django/core/handlers/base.py" in get_response
  92.                 response = callback(request, *callback_args, **callback_kwargs)
File "/var/lib/python-support/python2.6/django/contrib/admin/sites.py" in root
  480.                 return self.model_page(request, *url.split('/', 2))
File "/var/lib/python-support/python2.6/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/var/lib/python-support/python2.6/django/contrib/admin/sites.py" in model_page
  499.         return admin_obj(request, rest_of_url)
File "/var/lib/python-support/python2.6/django/contrib/admin/options.py" in __call__
  1088.             return self.add_view(request)
File "/var/lib/python-support/python2.6/django/db/transaction.py" in _commit_on_success
  240.                 res = func(*args, **kw)
File "/var/lib/python-support/python2.6/django/contrib/admin/options.py" in add_view
  715.             if form.is_valid():
File "/var/lib/python-support/python2.6/django/forms/forms.py" in is_valid
  120.         return self.is_bound and not bool(self.errors)
File "/var/lib/python-support/python2.6/django/forms/forms.py" in _get_errors
  111.             self.full_clean()
File "/var/lib/python-support/python2.6/django/forms/forms.py" in full_clean
  240.                     value = field.clean(value)
File "/var/lib/python-support/python2.6/django/forms/models.py" in clean
  981.             value = self.queryset.get(**{key: value})
File "/var/lib/python-support/python2.6/django/db/models/query.py" in get
  299.         clone = self.filter(*args, **kwargs)
File "/var/lib/python-support/python2.6/django/db/models/query.py" in filter
  498.         return self._filter_or_exclude(False, *args, **kwargs)
File "/var/lib/python-support/python2.6/django/db/models/query.py" in _filter_or_exclude
  516.             clone.query.add_q(Q(*args, **kwargs))
File "/var/lib/python-support/python2.6/django/db/models/sql/query.py" in add_q
  1675.                             can_reuse=used_aliases)
File "/var/lib/python-support/python2.6/django/db/models/sql/query.py" in add_filter
  1614.                 connector)
File "/var/lib/python-support/python2.6/django/db/models/sql/where.py" in add
  56.                 obj, params = obj.process(lookup_type, value)
File "/var/lib/python-support/python2.6/django/db/models/sql/where.py" in process
  269.                 params = self.field.get_db_prep_lookup(lookup_type, value)
File "/var/lib/python-support/python2.6/django/db/models/fields/__init__.py" in get_db_prep_lookup
  210.             return [self.get_db_prep_value(value)]
File "/var/lib/python-support/python2.6/django/db/models/fields/__init__.py" in get_db_prep_value
  361.         return int(value)

Exception Type: ValueError at /admin/articles/video/add/
Exception Value: invalid literal for int() with base 10: 'adfas'

[Alex Gaynor]

Please file a new ticket for this.

[Jacob]

Milestone 1.1 deleted