Opened 5 months ago
Last modified 9 days ago
#35449 assigned Bug
SplitArrayField doesn't validate properly with remove_trailing_nulls=True
Reported by: | Matthijs | Owned by: | Calvin Vu |
---|---|---|---|
Component: | contrib.postgres | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | Calvin Vu | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
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())
Change History (11)
comment:1 by , 5 months ago
Summary: | SplitArrayField doesn't validate properly (django.contrib.postgres.forms) → SplitArrayField doesn't validate properly with remove_trailing_nulls=True |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 5 months ago
Has patch: | set |
---|
comment:4 by , 4 months ago
Patch needs improvement: | set |
---|
comment:5 by , 3 weeks ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:6 by , 3 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
follow-up: 9 comment:7 by , 3 weeks ago
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?
comment:8 by , 3 weeks ago
Cc: | added |
---|
comment:9 by , 2 weeks ago
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_arrayThe test passes, does this mean that the bug has been resolved?
Nevermind, ignore this
comment:11 by , 9 days ago
Patch needs improvement: | set |
---|
Thank you! Was able to replicate on main
Here is a test if someone needs.
tests/postgres_tests/test_array.py