Opened 3 years ago

Last modified 3 years ago

#22229 assigned Bug

Invalid data can cause InlineFormSet.is_valid to throw ValueError

Reported by: anonymous Owned by: Jake Rothenbuhler
Component: Forms Version: 1.6
Severity: Normal Keywords: InlineFormSet
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


The InlineFormSet's foreign key field ("<input name='myformset-0-id' ...>") seems to assume that its data is valid for all of the initial data. Thus, if it is altered, the database layer will fail to convert it and throw an exception, instead of reporting that the form is invalid. For an integer foreign key, this takes form of a ValueError thrown by the int function.

I've created a test demonstrating the issue here:

Change History (3)

comment:1 Changed 3 years ago by Tim Graham

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

On master/1.7, the test code throws a ValidationError instead, but I agree this probably should cause is_valid() simply to return False.

Traceback (most recent call last):
  File "/home/tim/code/mysite/test/", line 150, in test_django_formset_int_error
  File "/home/tim/code/django/django/forms/", line 302, in is_valid
  File "/home/tim/code/django/django/forms/", line 276, in errors
  File "/home/tim/code/django/django/forms/", line 324, in full_clean
    form = self.forms[i]
  File "/home/tim/code/django/django/utils/", line 55, in __get__
    res = instance.__dict__[self.func.__name__] = self.func(instance)
  File "/home/tim/code/django/django/forms/", line 141, in forms
    forms = [self._construct_form(i) for i in xrange(self.total_form_count())]
  File "/home/tim/code/django/django/forms/", line 869, in _construct_form
    form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs)
  File "/home/tim/code/django/django/forms/", line 586, in _construct_form
    pk = to_python(pk)
  File "/home/tim/code/django/django/db/models/fields/", line 894, in to_python
    params={'value': value},
ValidationError: [u"'' value must be an integer."]

comment:2 Changed 3 years ago by Jake Rothenbuhler

Owner: changed from nobody to Jake Rothenbuhler
Status: newassigned

comment:3 Changed 3 years ago by Jake Rothenbuhler

I'm not quite sure that we want to make is_valid() return False in this case. With that behavior, the template would end up displaying an error message saying something like: "Invalid primary key for form that should have an existing instance." The user, seeing this error message, would not be able to fix it because the primary key fields wouldn't be editable. Instead, I think the error should be thrown as it currently is, resulting in a 500 response. I can't think of any situations where this error would arise from anything other than a misconfigured form initialization or a mistake in the form rendering. Is there a valid use case for catching this error that I'm missing?

If we do want to throw the error, the current ValidationError is probably misleading. I think a ValueError would be more correct.

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