Fixed #22229 -- Added primary key validation to BaseModelFormSet._construct_form().

Author: jdufresne

django/forms/models.py

@@ -590,20 +590,37 @@ def _get_to_python(self, field):
         return field.to_python
 
     def _construct_form(self, i, **kwargs):
-        if self.is_bound and i < self.initial_form_count():
-            pk_key = "%s-%s" % (self.add_prefix(i), self.model._meta.pk.name)
-            pk = self.data[pk_key]
-            pk_field = self.model._meta.pk
-            to_python = self._get_to_python(pk_field)
-            pk = to_python(pk)
-            kwargs['instance'] = self._existing_object(pk)
-        if i < self.initial_form_count() and 'instance' not in kwargs:
-            kwargs['instance'] = self.get_queryset()[i]
-        if i >= self.initial_form_count() and self.initial_extra:
+        pk_required = False
+        if i < self.initial_form_count():
+            pk_required = True
+            if self.is_bound:
+                pk_key = '%s-%s' % (self.add_prefix(i), self.model._meta.pk.name)
+                try:
+                    pk = self.data[pk_key]
+                except KeyError:
+                    # The primary key is missing. The user may have tampered
+                    # with POST data.
+                    pass
+                else:
+                    to_python = self._get_to_python(self.model._meta.pk)
+                    try:
+                        pk = to_python(pk)
+                    except ValidationError:
+                        # The primary key exists but is an invalid value. The
+                        # user may have tampered with POST data.
+                        pass
+                    else:
+                        kwargs['instance'] = self._existing_object(pk)
+            else:
+                kwargs['instance'] = self.get_queryset()[i]
+        elif self.initial_extra:
             # Set initial values for extra forms
             with suppress(IndexError):
                 kwargs['initial'] = self.initial_extra[i - self.initial_form_count()]
-        return super()._construct_form(i, **kwargs)
+        form = super()._construct_form(i, **kwargs)
+        if pk_required:
+            form.fields[self.model._meta.pk.name].required = True
+        return form
 
     def get_queryset(self):
         if not hasattr(self, '_queryset'):

tests/model_formsets/tests.py

@@ -1655,6 +1655,50 @@ def test_prevent_change_outer_model_and_create_invalid_data(self):
         # created.
         self.assertQuerysetEqual(Author.objects.all(), ['<Author: Charles>', '<Author: Walt>'])
 
+    def test_validation_without_id(self):
+        AuthorFormSet = modelformset_factory(Author, fields='__all__')
+        data = {
+            'form-TOTAL_FORMS': '1',
+            'form-INITIAL_FORMS': '1',
+            'form-MAX_NUM_FORMS': '',
+            'form-0-name': 'Charles',
+        }
+        formset = AuthorFormSet(data)
+        self.assertEqual(
+            formset.errors,
+            [{'id': ['This field is required.']}],
+        )
+
+    def test_validation_with_child_model_without_id(self):
+        BetterAuthorFormSet = modelformset_factory(BetterAuthor, fields='__all__')
+        data = {
+            'form-TOTAL_FORMS': '1',
+            'form-INITIAL_FORMS': '1',
+            'form-MAX_NUM_FORMS': '',
+            'form-0-name': 'Charles',
+            'form-0-write_speed': '10',
+        }
+        formset = BetterAuthorFormSet(data)
+        self.assertEqual(
+            formset.errors,
+            [{'author_ptr': ['This field is required.']}],
+        )
+
+    def test_validation_with_invalid_id(self):
+        AuthorFormSet = modelformset_factory(Author, fields='__all__')
+        data = {
+            'form-TOTAL_FORMS': '1',
+            'form-INITIAL_FORMS': '1',
+            'form-MAX_NUM_FORMS': '',
+            'form-0-id': 'abc',
+            'form-0-name': 'Charles',
+        }
+        formset = AuthorFormSet(data)
+        self.assertEqual(
+            formset.errors,
+            [{'id': ['Select a valid choice. That choice is not one of the available choices.']}],
+        )
+
     def test_validation_with_nonexistent_id(self):
         AuthorFormSet = modelformset_factory(Author, fields='__all__')
         data = {