fix: handle invalid fields properly in proc material upload form (#3784)

ietf/meeting/tests_views.py

@@ -20,6 +20,7 @@ from bs4 import BeautifulSoup
 from urllib.parse import urlparse, urlsplit, quote
 from PIL import Image
 from pathlib import Path
+from tempfile import NamedTemporaryFile
 
 from django.urls import reverse as urlreverse
 from django.conf import settings
@@ -7609,6 +7610,35 @@ class ProceedingsTests(BaseMeetingTestCase):
             )
             self.assertEqual(mat.get_href(), f'{mat.document.name}:00')
 
+    def test_add_proceedings_material_doc_invalid_ext(self):
+        """Upload proceedings materials document with disallowed extension"""
+        meeting = self._procmat_test_meeting()
+        self.client.login(username='secretary', password='secretary+password')
+        with NamedTemporaryFile('w+', suffix='.png') as invalid_file:
+            invalid_file.write('this is not a PDF file!!')
+            for mat_type in ProceedingsMaterialTypeName.objects.filter(used=True):
+                url = urlreverse(
+                    'ietf.meeting.views_proceedings.upload_material',
+                    kwargs=dict(num=meeting.number, material_type=mat_type.slug),
+                )
+                invalid_file.seek(0)  # read the file contents again
+                r = self.client.post(url, {'file': invalid_file, 'external_url': ''})
+                self.assertEqual(r.status_code, 200)
+                self.assertFormError(r, 'form', 'file', 'Found an unexpected extension: .png.  Expected one of .pdf')
+
+    def test_add_proceedings_material_doc_empty(self):
+        """Upload proceedings materials document without specifying a file"""
+        meeting = self._procmat_test_meeting()
+        self.client.login(username='secretary', password='secretary+password')
+        for mat_type in ProceedingsMaterialTypeName.objects.filter(used=True):
+            url = urlreverse(
+                'ietf.meeting.views_proceedings.upload_material',
+                kwargs=dict(num=meeting.number, material_type=mat_type.slug),
+            )
+            r = self.client.post(url, {'external_url': ''})
+            self.assertEqual(r.status_code, 200)
+            self.assertFormError(r, 'form', 'file', 'This field is required')
+
     def test_add_proceedings_material_url(self):
         """Add a URL as proceedings material"""
         meeting = self._procmat_test_meeting()
@@ -7620,6 +7650,32 @@ class ProceedingsTests(BaseMeetingTestCase):
             )
             self.assertEqual(mat.get_href(), 'https://example.com')
 
+    def test_add_proceedings_material_url_invalid(self):
+        """Add proceedings materials URL with a non-URL value"""
+        meeting = self._procmat_test_meeting()
+        self.client.login(username='secretary', password='secretary+password')
+        for mat_type in ProceedingsMaterialTypeName.objects.filter(used=True):
+            url = urlreverse(
+                'ietf.meeting.views_proceedings.upload_material',
+                kwargs=dict(num=meeting.number, material_type=mat_type.slug),
+            )
+            r = self.client.post(url, {'use_url': 'on', 'external_url': "Ceci n'est pas une URL"})
+            self.assertEqual(r.status_code, 200)
+            self.assertFormError(r, 'form', 'external_url', 'Enter a valid URL.')
+
+    def test_add_proceedings_material_url_empty(self):
+        """Add proceedings materials URL without specifying the URL"""
+        meeting = self._procmat_test_meeting()
+        self.client.login(username='secretary', password='secretary+password')
+        for mat_type in ProceedingsMaterialTypeName.objects.filter(used=True):
+            url = urlreverse(
+                'ietf.meeting.views_proceedings.upload_material',
+                kwargs=dict(num=meeting.number, material_type=mat_type.slug),
+            )
+            r = self.client.post(url, {'use_url': 'on', 'external_url': ''})
+            self.assertEqual(r.status_code, 200)
+            self.assertFormError(r, 'form', 'external_url', 'This field is required')
+
     @override_settings(MEETING_DOC_HREFS={'procmaterials': '{doc.name}:{doc.rev}'})
     def test_replace_proceedings_material(self):
         """Replace uploaded document with new uploaded document"""

ietf/meeting/views_proceedings.py

@@ -50,11 +50,11 @@ class UploadProceedingsMaterialForm(FileUploadForm):
 
     def clean(self):
         if self.cleaned_data['use_url']:
-            if not self.cleaned_data['external_url']:
+            if not self.data.get('external_url', None):
                 self.add_error('external_url', 'This field is required')
         else:
             self.cleaned_data['external_url'] = None  # make sure this is empty
-            if self.cleaned_data['file'] is None:
+            if self.files.get('file', None) is None:
                 self.add_error('file', 'This field is required')
 
 class EditProceedingsMaterialForm(forms.Form):