Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#35483 closed Bug (fixed)

ModelMultipleChoiceField with CharField key throws ValueError if data contains NUL (0x00) characters

Reported by: Jennifer Richards Owned by: lotvall
Component: Forms Version: 4.2
Severity: Normal Keywords: ModelMultipleChoiceField null nul
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In a form using a ModelMultipleChoiceField with a model that has a CharField primary key, I'm getting server errors when a request submits input with a nul (0x00) character. This is resulting from a ValueError raised by the postgresql backend. It happens whether the data is submitted as a query string or in the request body.

I've tried adding an explicit ProhibitNullCharactersValidator to the field, but this fails because the field's clean method runs validators after evaluating whether the input is a member of the queryset.

A fairly minimal example that exhibits this is:

class MyModel(models.Model):
    slug = models.CharField(primary_key=True)

class MyForm(forms.Form):
    field = forms.ModelMultipleChoiceField(queryset=MyModel.objects.all())

def my_view(request):
    form = MyForm(data=request.GET)
    if form.is_valid():
        return HttpResponse("yay")
    return HttpResponse("boo", status=400)

With that running, the following triggers the error:

$ curl 'http://localhost:8000/my-view?field=hi%00'

The output from the Django dev server is

ERROR: django.request:241: Internal Server Error: /my-view
Traceback (most recent call last):
  File "/home/dev/.local/lib/python3.9/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/home/dev/.local/lib/python3.9/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/workspace/ietf/doc/views_search.py", line 98, in my_view
    if form.is_valid():
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/forms.py", line 201, in is_valid
    return self.is_bound and not self.errors
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/forms.py", line 196, in errors
    self.full_clean()
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/forms.py", line 433, in full_clean
    self._clean_fields()
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/forms.py", line 445, in _clean_fields
    value = field.clean(value)
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/models.py", line 1590, in clean
    qs = self._check_values(value)
  File "/home/dev/.local/lib/python3.9/site-packages/django/forms/models.py", line 1623, in _check_values
    pks = {str(getattr(o, key)) for o in qs}
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/models/query.py", line 398, in __iter__
    self._fetch_all()
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/models/query.py", line 1881, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/models/query.py", line 91, in __iter__
    results = compiler.execute_sql(
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1562, in execute_sql
    cursor.execute(sql, params)
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/backends/utils.py", line 102, in execute
    return super().execute(sql, params)
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/dev/.local/lib/python3.9/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
ValueError: A string literal cannot contain NUL (0x00) characters.
[27/May/2024 08:02:37] "GET /my-view?field=hi%00 HTTP/1.0" 500 180131

As a workaround, replacing forms.ModelMultipleChoiceField with

class MyModelMultipleChoiceField(forms.ModelMultipleChoiceField):
    validate_no_nulls = validators.ProhibitNullCharactersValidator()
    def clean(self, value):
        for item in value:
            self.validate_no_nulls(item)
        return super().clean(value)

correctly handles the same input, with only

[27/May/2024 08:04:22] "GET /my-view?field=hi%00 HTTP/1.0" 400 3

in the server log.

I'm seeing this with Django 4.2.13 using the postgresql backend with psycopg2 2.99 against PostgreSQL 14.6.

Change History (7)

comment:1 by Sarah Boyce, 6 months ago

Triage Stage: UnreviewedAccepted

Replicated, thank you for the report!

My failing test if it's useful

  • tests/model_forms/tests.py

    diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py
    index 3f927cb053..16bdbf8d21 100644
    a b class ModelMultipleChoiceFieldTests(TestCase):  
    22272227        f = forms.ModelMultipleChoiceField(queryset=Writer.objects.all())
    22282228        self.assertNumQueries(1, f.clean, [p.pk for p in persons[1:11:2]])
    22292229
     2230    def test_model_multiple_choice_null_characters(self):
     2231        f = forms.ModelMultipleChoiceField(queryset=ExplicitPK.objects.all())
     2232        msg = "Null characters are not allowed."
     2233        with self.assertRaisesMessage(ValidationError, msg):
     2234            f.clean(["\x00something"])
     2235
    22302236    def test_model_multiple_choice_run_validators(self):
    22312237        """
    22322238        ModelMultipleChoiceField run given validators (#14144).

Relates to #28201.

comment:2 by lotvall, 6 months ago

Has patch: set
Owner: changed from nobody to lotvall
Status: newassigned

comment:3 by Sarah Boyce, 5 months ago

Patch needs improvement: set

comment:4 by lotvall, 5 months ago

Patch needs improvement: unset

comment:5 by Sarah Boyce, 5 months ago

Triage Stage: AcceptedReady for checkin

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

Resolution: fixed
Status: assignedclosed

In 38ad710:

Fixed #35483 -- Added NUL (0x00) character validation to ModelChoiceFields.

Applied the ProhibitNullCharactersValidator to ModelChoiceField and ModelMultipleChoiceField.

Co-authored-by: Viktor Paripás <viktor.paripas@…>
Co-authored-by: Vasyl Dizhak <vasyl@…>
Co-authored-by: Arthur Vasconcelos <vasconcelos.arthur@…>

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 5 months ago

In b8983dc:

[5.1.x] Fixed #35483 -- Added NUL (0x00) character validation to ModelChoiceFields.

Applied the ProhibitNullCharactersValidator to ModelChoiceField and ModelMultipleChoiceField.

Co-authored-by: Viktor Paripás <viktor.paripas@…>
Co-authored-by: Vasyl Dizhak <vasyl@…>
Co-authored-by: Arthur Vasconcelos <vasconcelos.arthur@…>

Backport of 38ad710aba885ad26944ff5708ce1a02a446d2d3 from main.

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