ModelFormSet with queryset accepts invalid POST data for outer models and create unexpected empty data.

[Hiroki Kiyohara]

ModelFormSet with queryset argument will accept invalid POST data and create unexpected empty data.
Invalid data means that data won't be appear in queryset=..., like this.

        author = Author.objects.create(pk=1, name='Walt Whitman')
        other_author = Author.objects.create(pk=2, name='Charles Baudelaire')
        AuthorFormSet = modelformset_factory(Author, fields="__all__", extra=0)

        data = {
            'form-TOTAL_FORMS': '2',  # the number of forms rendered
            'form-INITIAL_FORMS': '2',  # the number of forms with initial data
            'form-MAX_NUM_FORMS': '',  # the max number of forms
            'form-0-id': str(author.id),
            'form-0-name': 'Walt Whitman',
            'form-1-id': str(other_author.id),  # Specifying outer model of bellow queryset.
            'form-1-name': 'Changed name',
        }
        # This formset is only for Walt Whitman
        # and should not accept POST data for other_author
        formset = AuthorFormSet(data=data,
                                queryset=Author.objects.filter(id__in=(author.id,)))

This formset should ignore unexpected form-1-id': str(other_author.id) value, a value not in queryset.
The value in other_author won't be changed, but this formset will create unexpected new empty data (which has Author.name = "").

Why

Internally...

1. In Formsets, forms corresponding to invalid data will be instantiated with instance=None (due to this line ​https://github.com/django/django/blob/master/django/forms/models.py#L597)
2. ModelForm will set form.instance as empty instance (self.instance = opts.model())
3. Saving formset.save() will save this unexpected invalid data

Why it's problem

It will create unexpected empty data and may cause IntegrityError.
For instance, if Artist.name field has unique constraint, and invalid POST data come sometimes,
it will create empty Artist data more than twice and cause UNIQUE constraint error.

PS

Is this recognized as bug? or should I specify more extra args for formset?
I searched similar tickets but not existed.
(Just ticket I found is #26142 to prevent to create new instance for FormSet, It may prevent this problem too but not correct way to solve).

[Hiroki Kiyohara]

Opened a Pull Request ​https://github.com/django/django/pull/7462

[Tim Graham]

My first instinct is that this is a duplicate of #26142 but the fact that the extra instances are created without the supplied data leaves a question as to whether or not the current behavior is actually useful. As #27416 says, extra=0 isn't really meant to prohibit the creation of new instances. Without digging into the details, I'd say it might be appropriate to fix the current behavior to create instances with the provided data (as a separate ticket) and then in this ticket add a new modelformset_factory() parameter to prohibit the creation of new instances.

[Hiroki Kiyohara]

Thanks Tim for your quick response.

And I agree with you, If we want to prohibit the creation of new instances, the ticket #26142 is the best.
Actually #26142 will solve a latest problem on my working project (Creation of unexpected instance by invalid POST param).
As Tim explained, now the second question is whether invalid POST data should create new empty instance or not.

I'm thinking it's not expected behavior. Because it will create unvalidated empty instance. not so explicit.
But I'm not strongly sure. so I want some others opinion or I'll take time to re-think about it.

I deleted extra=0 argument on the PR (now it seems unrelated thing).
​https://github.com/django/django/pull/7462/commits/20d4b8073c0f028ee23feb68f42cdd58549d58fa

[Tim Graham]

I did some testing and the patch looks reasonable, although I'd like another set of eyes.

Using the test in your patch, I didn't see the "empty data" object you described. Without the fix, the new object has name='Changed name', doesn't it?

[Hiroki Kiyohara]

Thank you. I want to check not to exist the "empty data", so the test has self.assertEqual(Author.objects.count(), 2).
Without chanes of the code, It will be 3, (author, other_author, and unexpected empty data). The other_author.name won't be "Changed name" even we don't chande codes.

I'll check something I said is correct again later.
If you feel the test or changes is hard to understand, I'll improve so please give me any advices.

[Hiroki Kiyohara]

Without changes of the code, It will be 3, (author, other_author, and unexpected empty data). The other_author.name won't be "Changed name"

I checked that it's correct.

[Tim Graham <timograham@…>]

In 181f492a:

Fixed #27416 -- Prevented ModelFormSet from creating objects for invalid PKs in data.

[Hiroki Kiyohara]

Thank you.