feat: better reject null characters in forms (ietf-tools#7472)

* feat: subclass ModelMultipleChoiceField to reject nuls

* refactor: Use custom ModelMultipleChoiceField

* fix: handle value=None

Author: jennifer-richards

ietf/doc/views_ballot.py

@@ -38,6 +38,7 @@
 from ietf.message.utils import infer_message
 from ietf.name.models import BallotPositionName, DocTypeName
 from ietf.person.models import Person
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.mail import send_mail_text, send_mail_preformatted
 from ietf.utils.decorators import require_api_key
 from ietf.utils.response import permission_denied
@@ -931,7 +932,7 @@ def approve_ballot(request, name):
 
 
 class ApproveDownrefsForm(forms.Form):
-    checkboxes = forms.ModelMultipleChoiceField(
+    checkboxes = ModelMultipleChoiceField(
         widget = forms.CheckboxSelectMultiple,
         queryset =  RelatedDocument.objects.none(), )
 

ietf/doc/views_draft.py

@@ -52,6 +52,7 @@
 from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of
 from ietf.utils.textupload import get_cleaned_text_file_content
 from ietf.utils import log
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.response import permission_denied
 from ietf.utils.timezone import datetime_today, DEADLINE_TZINFO
 
@@ -390,9 +391,9 @@ def replaces(request, name):
                    ))
 
 class SuggestedReplacesForm(forms.Form):
-    replaces = forms.ModelMultipleChoiceField(queryset=Document.objects.all(),
-                                              label="Suggestions", required=False, widget=forms.CheckboxSelectMultiple,
-                                              help_text="Select only the documents that are replaced by this document")
+    replaces = ModelMultipleChoiceField(queryset=Document.objects.all(),
+                                        label="Suggestions", required=False, widget=forms.CheckboxSelectMultiple,
+                                        help_text="Select only the documents that are replaced by this document")
     comment = forms.CharField(label="Optional comment", widget=forms.Textarea, required=False, strip=False)
 
     def __init__(self, suggested, *args, **kwargs):
@@ -1601,7 +1602,7 @@ class ChangeStreamStateForm(forms.Form):
     new_state = forms.ModelChoiceField(queryset=State.objects.filter(used=True), label='State' )
     weeks = forms.IntegerField(label='Expected weeks in state',required=False)
     comment = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional comment for the document history.", strip=False)
-    tags = forms.ModelMultipleChoiceField(queryset=DocTagName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False)
+    tags = ModelMultipleChoiceField(queryset=DocTagName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False)
 
     def __init__(self, *args, **kwargs):
         doc = kwargs.pop("doc")

ietf/doc/views_review.py

@@ -52,7 +52,7 @@
 from ietf.utils.textupload import get_cleaned_text_file_content
 from ietf.utils.mail import send_mail_message
 from ietf.mailtrigger.utils import gather_address_lists
-from ietf.utils.fields import MultiEmailField
+from ietf.utils.fields import ModelMultipleChoiceField, MultiEmailField
 from ietf.utils.http import is_ajax
 from ietf.utils.response import permission_denied
 from ietf.utils.timezone import date_today, DEADLINE_TZINFO
@@ -68,7 +68,7 @@ def clean_doc_revision(doc, rev):
     return rev
 
 class RequestReviewForm(forms.ModelForm):
-    team = forms.ModelMultipleChoiceField(queryset=Group.objects.all(), widget=forms.CheckboxSelectMultiple)
+    team = ModelMultipleChoiceField(queryset=Group.objects.all(), widget=forms.CheckboxSelectMultiple)
     deadline = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" })
 
     class Meta:

ietf/doc/views_search.py

@@ -69,6 +69,7 @@
 from ietf.person.models import Person
 from ietf.person.utils import get_active_ads
 from ietf.utils.draft_search import normalize_draftname
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.log import log
 from ietf.doc.utils_search import prepare_document_table, doc_type, doc_state, doc_type_name, AD_WORKLOAD
 from ietf.ietfauth.utils import has_role
@@ -100,7 +101,7 @@ class SearchForm(forms.Form):
             ("ad", "AD"), ("-ad", "AD (desc)"), ),
         required=False, widget=forms.HiddenInput)
 
-    doctypes = forms.ModelMultipleChoiceField(queryset=DocTypeName.objects.filter(used=True).exclude(slug__in=('draft', 'rfc', 'bcp', 'std', 'fyi', 'liai-att')).order_by('name'), required=False)
+    doctypes = ModelMultipleChoiceField(queryset=DocTypeName.objects.filter(used=True).exclude(slug__in=('draft', 'rfc', 'bcp', 'std', 'fyi', 'liai-att')).order_by('name'), required=False)
 
     def __init__(self, *args, **kwargs):
         super(SearchForm, self).__init__(*args, **kwargs)

ietf/liaisons/forms.py

@@ -32,7 +32,7 @@
 from ietf.person.models import Email
 from ietf.person.fields import SearchableEmailField
 from ietf.doc.models import Document
-from ietf.utils.fields import DatepickerDateField
+from ietf.utils.fields import DatepickerDateField, ModelMultipleChoiceField
 from ietf.utils.timezone import date_today, datetime_from_date, DEADLINE_TZINFO
 from functools import reduce
 
@@ -200,7 +200,7 @@ def get_results(self):
         return results
 
 
-class CustomModelMultipleChoiceField(forms.ModelMultipleChoiceField):
+class CustomModelMultipleChoiceField(ModelMultipleChoiceField):
     '''If value is a QuerySet, return it as is (for use in widget.render)'''
     def prepare_value(self, value):
         if isinstance(value, QuerySetAny):
@@ -215,12 +215,12 @@ def prepare_value(self, value):
 class LiaisonModelForm(forms.ModelForm):
     '''Specify fields which require a custom widget or that are not part of the model.
     '''
-    from_groups = forms.ModelMultipleChoiceField(queryset=Group.objects.all(),label='Groups',required=False)
+    from_groups = ModelMultipleChoiceField(queryset=Group.objects.all(),label='Groups',required=False)
     from_groups.widget.attrs["class"] = "select2-field"
     from_groups.widget.attrs['data-minimum-input-length'] = 0
     from_contact = forms.EmailField()   # type: Union[forms.EmailField, SearchableEmailField]
     to_contacts = forms.CharField(label="Contacts", widget=forms.Textarea(attrs={'rows':'3', }), strip=False)
-    to_groups = forms.ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False)
+    to_groups = ModelMultipleChoiceField(queryset=Group.objects,label='Groups',required=False)
     to_groups.widget.attrs["class"] = "select2-field"
     to_groups.widget.attrs['data-minimum-input-length'] = 0
     deadline = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={"autoclose": "1" }, label='Deadline', required=True)

ietf/meeting/forms.py

@@ -28,7 +28,13 @@
 from ietf.message.models import Message
 from ietf.name.models import TimeSlotTypeName, SessionPurposeName
 from ietf.person.models import Person
-from ietf.utils.fields import DatepickerDateField, DurationField, MultiEmailField, DatepickerSplitDateTimeWidget
+from ietf.utils.fields import (
+    DatepickerDateField,
+    DatepickerSplitDateTimeWidget,
+    DurationField,
+    ModelMultipleChoiceField,
+    MultiEmailField,
+)
 from ietf.utils.validators import ( validate_file_size, validate_mime_type,
     validate_file_extension, validate_no_html_frame)
 
@@ -551,7 +557,7 @@ class SwapTimeslotsForm(forms.Form):
         queryset=TimeSlot.objects.none(),  # default to none, fill in when we have a meeting
         widget=forms.TextInput,
     )
-    rooms = forms.ModelMultipleChoiceField(
+    rooms = ModelMultipleChoiceField(
         required=True,
         queryset=Room.objects.none(),  # default to none, fill in when we have a meeting
         widget=CsvModelPkInput,
@@ -617,7 +623,7 @@ class TimeSlotCreateForm(forms.Form):
     )
     duration = TimeSlotDurationField()
     show_location = forms.BooleanField(required=False, initial=True)
-    locations = forms.ModelMultipleChoiceField(
+    locations = ModelMultipleChoiceField(
         queryset=Room.objects.none(),
         widget=forms.CheckboxSelectMultiple,
     )

ietf/nomcom/forms.py

@@ -21,6 +21,7 @@
 from ietf.person.models import Email
 from ietf.person.fields import (SearchableEmailField, SearchableEmailsField,
                                 SearchablePersonField, SearchablePersonsField )
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.mail import send_mail
 from ietf.mailtrigger.utils import gather_address_lists
 
@@ -719,9 +720,9 @@ def set_nomcom(self, nomcom, person, instances=None):
                                                                   required= self.feedback_type.slug != 'comment',
                                                                   help_text='Hold down "Control", or "Command" on a Mac, to select more than one.')
             if self.feedback_type.slug == 'comment':
-                self.fields['topic'] = forms.ModelMultipleChoiceField(queryset=self.nomcom.topic_set.all(),
-                                                                      help_text='Hold down "Control" or "Command" on a Mac, to select more than one.',
-                                                                      required=False,)
+                self.fields['topic'] = ModelMultipleChoiceField(queryset=self.nomcom.topic_set.all(),
+                                                                help_text='Hold down "Control" or "Command" on a Mac, to select more than one.',
+                                                                required=False,)
         else:
             self.fields['position'] = forms.ModelChoiceField(queryset=Position.objects.get_by_nomcom(self.nomcom).filter(is_open=True), label="Position")
             self.fields['searched_email'] = SearchableEmailField(only_users=False,help_text="Try to find the candidate you are classifying with this field first. Only use the name and email fields below if this search does not find the candidate.",label="Candidate",required=False)
@@ -847,7 +848,7 @@ class Meta:
 class NominationResponseCommentForm(forms.Form):
     comments = forms.CharField(widget=forms.Textarea,required=False,help_text="Any comments provided will be encrypted and will only be visible to the NomCom.", strip=False)
 
-class NomcomVolunteerMultipleChoiceField(forms.ModelMultipleChoiceField):
+class NomcomVolunteerMultipleChoiceField(ModelMultipleChoiceField):
     def label_from_instance(self, obj):
         year = obj.year()
         return f'Volunteer for the {year}/{year+1} Nominating Committee'

ietf/secr/sreq/forms.py

@@ -13,6 +13,7 @@
 from ietf.meeting.models import ResourceAssociation, Constraint
 from ietf.person.fields import SearchablePersonsField
 from ietf.person.models import Person
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.html import clean_text_field
 from ietf.utils import log
 
@@ -57,7 +58,7 @@ def __init__(self,*args,**kwargs):
         self.fields['group'].widget.choices = choices
 
 
-class NameModelMultipleChoiceField(forms.ModelMultipleChoiceField):
+class NameModelMultipleChoiceField(ModelMultipleChoiceField):
     def label_from_instance(self, name):
         return name.desc
 
@@ -159,7 +160,7 @@ def __init__(self, group, meeting, data=None, *args, **kwargs):
             self.fields['resources'].widget = forms.MultipleHiddenInput()
             self.fields['timeranges'].widget = forms.MultipleHiddenInput()
             # and entirely replace bethere - no need to support searching if input is hidden
-            self.fields['bethere'] = forms.ModelMultipleChoiceField(
+            self.fields['bethere'] = ModelMultipleChoiceField(
                 widget=forms.MultipleHiddenInput, required=False,
                 queryset=Person.objects.all(),
             )

ietf/submit/forms.py

@@ -39,6 +39,7 @@
 from ietf.submit.parsers.xml_parser import XMLParser
 from ietf.utils import log
 from ietf.utils.draft import PlaintextDraft
+from ietf.utils.fields import ModelMultipleChoiceField
 from ietf.utils.text import normalize_text
 from ietf.utils.timezone import date_today
 from ietf.utils.xmldraft import InvalidXMLError, XMLDraft, XMLParseError
@@ -793,7 +794,7 @@ class EditSubmissionForm(forms.ModelForm):
     rev = forms.CharField(label='Revision', max_length=2, required=True)
     document_date = forms.DateField(required=True)
     pages = forms.IntegerField(required=True)
-    formal_languages = forms.ModelMultipleChoiceField(queryset=FormalLanguageName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False)
+    formal_languages = ModelMultipleChoiceField(queryset=FormalLanguageName.objects.filter(used=True), widget=forms.CheckboxSelectMultiple, required=False)
     abstract = forms.CharField(widget=forms.Textarea, required=True, strip=False)
 
     note = forms.CharField(label=mark_safe('Comment to the Secretariat'), widget=forms.Textarea, required=False, strip=False)

ietf/utils/fields.py

@@ -14,7 +14,7 @@
 
 from django import forms
 from django.db import models # pyflakes:ignore
-from django.core.validators import validate_email
+from django.core.validators import ProhibitNullCharactersValidator, validate_email
 from django.core.exceptions import ValidationError
 from django.utils.dateparse import parse_duration
 
@@ -353,3 +353,20 @@ def update_dimension_fields(self, *args, **kwargs):
             super().update_dimension_fields(*args, **kwargs)
         except FileNotFoundError:
             pass  # don't do anything if the file has gone missing
+
+
+class ModelMultipleChoiceField(forms.ModelMultipleChoiceField):
+    """ModelMultipleChoiceField that rejects null characters cleanly"""
+    validate_no_nulls = ProhibitNullCharactersValidator()
+
+    def clean(self, value):
+        try:
+            for item in value:
+                self.validate_no_nulls(item)
+        except TypeError:
+            # A TypeError probably means value is not iterable, which most commonly comes up
+            # with None as a value. If it's something more exotic, we don't know how to test
+            # for null characters anyway. Either way, trust the superclass clean() method to 
+            # handle it.
+            pass
+        return super().clean(value)