SplitArrayField doesn't validate properly with remove_trailing_nulls=True

[Matthijs]

I created an ArrayField(models.CharField(), size=3) on one of my models and created a form for it using the SplitArrayField. My requirements where that the array field was optional to fill in and that you could also just fill in one of the CharFields and leave the others blank. So according to ​the docs I was to set the CharField in my SplitArrayField to required=True and remove_trailing_nulls=True on the SplitArrayField.

SplitArrayField(
    forms.CharField(required=True, max_length=148),
    size=3,
    widget=SplitArrayWidget(forms.TextInput(attrs={'size': '148'}), 3),
    remove_trailing_nulls=True,
    required=False
)

To my surprise the form did not validate properly and I was able to submit inputs that exceeded the max_length specified in my form.
Looking at the def clean method of the SplitArrayField I suspect a problem there.

try:
    cleaned_data.append(self.base_field.clean(item))
except ValidationError as error:
    errors.append(
        prefix_validation_error(
            error,
            self.error_messages["item_invalid"],
            code="item_invalid",
            params={"nth": index + 1},
        )
    )
    cleaned_data.append(None)
    ...

If there is any ValidationError the error is added to errors, at the same time None is appended to the cleaned_data.
This causes issues with the self._remove_trailing_nulls later on.

cleaned_data, null_index = self._remove_trailing_nulls(cleaned_data)
if null_index is not None:
    errors = errors[:null_index]
errors = list(filter(None, errors))

cleaned_data is passed to self._remove_trailing_nulls and because remove_trailing_nulls=True, null_index will be 0 thus errors is reassigned to an empty list.

I wrote a test case showcasing this issue:

from django import forms
from django.contrib.postgres.forms import SplitArrayField

class TestForm1(forms.Form):
    field = SplitArrayField(
        forms.CharField(max_length=5, required=True),
        size=3,
        remove_trailing_nulls=True,
        required=False,
    )

class TestForm2(forms.Form):
    field = forms.CharField(max_length=5, required=True)

class SplitArrayFieldTest(TestCase):
    def test_validation_error(self):
        invalid_value = 'invalid'
        form1 = TestForm1(data={'field': [invalid_value, '', '']})
        form2 = TestForm2(data={'field': invalid_value})

        self.assertFalse(form1.is_valid())  #is_valid() should return False, but returns True
        self.assertFalse(form2.is_valid())

[Sarah Boyce]

Thank you! Was able to replicate on main
Here is a test if someone needs.

tests/postgres_tests/test_array.py

@@ -1339 +1339 @@
             ],
         )
 
+    def test_invalid_char_length_with_remove_trailing_nulls(self):
+        field = SplitArrayField(
+            forms.CharField(max_length=2),
+            size=3,
+            remove_trailing_nulls=True,
+        )
+        with self.assertRaises(exceptions.ValidationError) as cm:
+            field.clean(["abc", "", ""])
+        self.assertEqual(
+            cm.exception.messages,
+            [
+                "Item 1 in the array did not validate: Ensure this value has at most 2 "
+                "characters (it has 3).",
+            ],
+        )
+
     def test_splitarraywidget_value_omitted_from_data(self):

[Calvin Vu]

When I include Sarah's test in test_array.py and run:

python runtests.py postgres_tests.test_array

The test passes, does this mean that the bug has been resolved?

[Calvin Vu]

Replying to Calvin Vu:

When I include Sarah's test in test_array.py and run the tests with

python runtests.py postgres_tests.test_array

The test passes, does this mean that the bug has been resolved?

Nevermind, ignore this

[Jacob Walls]

​new PR

[Sarah Boyce <42296566+sarahboyce@…>]

In a417c0e:

Fixed #35449 -- Fixed validation of array items in SplitArrayField when remove_trailing_nulls=True.