fix: validate review_requests_history params (#9281)

* test: test null chars in GET params

* fix: validate GET params

Author: jennifer-richards

ietf/group/tests_review.py

@@ -943,3 +943,42 @@ def test_requests_history_filter_page(self):
         self.assertNotContains(r, 'Assigned')
         self.assertNotContains(r, 'Accepted')
         self.assertNotContains(r, 'Completed')
+
+    def test_requests_history_invalid_filter_parameters(self):
+        # First assignment as assigned
+        review_req = ReviewRequestFactory(state_id="assigned", doc=DocumentFactory())
+        group = review_req.team
+        url = urlreverse(
+            "ietf.group.views.review_requests_history",
+            kwargs={"acronym": group.acronym},
+        )
+        invalid_reviewer_emails = [
+            "%[email protected]",  # urlencoded null character
+            "null@exa%00mple.com",  # urlencoded null character
+            "\x00[email protected]",  # literal null character
+            "null@ex\x00ample.com",  # literal null character
+        ]
+        for invalid_email in invalid_reviewer_emails:
+            r = self.client.get(
+                url + f"?reviewer_email={invalid_email}"
+            )
+            self.assertEqual(
+                r.status_code, 
+                400,
+                f"should return a 400 response for reviewer_email={repr(invalid_email)}"
+            )
+
+        invalid_since_choices = [
+            "forever",  # not an option
+            "all\x00",  # literal null character
+            "a%00ll",  # urlencoded null character
+        ]
+        for invalid_since in invalid_since_choices:
+            r = self.client.get(
+                url + f"?since={invalid_since}"
+            )
+            self.assertEqual(
+                r.status_code, 
+                400,
+                f"should return a 400 response for since={repr(invalid_since)}"
+            )

ietf/group/views.py

@@ -51,7 +51,13 @@
 from django.contrib.auth.decorators import login_required
 from django.db.models import Count, F, OuterRef, Prefetch, Q, Subquery, TextField, Value
 from django.db.models.functions import Coalesce
-from django.http import HttpResponse, HttpResponseRedirect, Http404, JsonResponse
+from django.http import (
+    HttpResponse,
+    HttpResponseRedirect,
+    Http404,
+    JsonResponse,
+    HttpResponseBadRequest,
+)
 from django.shortcuts import render, redirect, get_object_or_404
 from django.template.loader import render_to_string
 from django.urls import reverse as urlreverse
@@ -96,11 +102,9 @@
 from ietf.review.policies import get_reviewer_queue_policy
 from ietf.review.utils import (can_manage_review_requests_for_team,
                                can_access_review_stats_for_team,
-
                                extract_revision_ordered_review_requests_for_documents_and_replaced,
                                assign_review_request_to_reviewer,
                                close_review_request,
-
                                suggested_review_requests_for_team,
                                unavailable_periods_to_list,
                                current_unavailable_periods_for_reviewers,
@@ -686,13 +690,30 @@ def history(request, acronym, group_type=None):
                       "can_add_comment": can_add_comment,
                   }))
 
+
+class RequestsHistoryParamsForm(forms.Form):
+    SINCE_CHOICES = (
+        (None, "1 month"),
+        ("3m", "3 months"),
+        ("6m", "6 months"),
+        ("1y", "1 year"),
+        ("2y", "2 years"),
+        ("all", "All"),
+    )
+
+    reviewer_email = forms.EmailField(required=False)
+    since = forms.ChoiceField(choices=SINCE_CHOICES, required=False)
+
 def review_requests_history(request, acronym, group_type=None):
     group = get_group_or_404(acronym, group_type)
     if not group.features.has_reviews:
         raise Http404
 
-    reviewer_email = request.GET.get("reviewer_email", None)
+    params = RequestsHistoryParamsForm(request.GET)
+    if not params.is_valid():
+        return HttpResponseBadRequest("Invalid parameters")
 
+    reviewer_email = params.cleaned_data["reviewer_email"] or None
     if reviewer_email:
         history = ReviewAssignment.history.model.objects.filter(
             review_request__team__acronym=acronym,
@@ -702,19 +723,7 @@ def review_requests_history(request, acronym, group_type=None):
             review_request__team__acronym=acronym)
         reviewer_email = ''
 
-    since_choices = [
-        (None, "1 month"),
-        ("3m", "3 months"),
-        ("6m", "6 months"),
-        ("1y", "1 year"),
-        ("2y", "2 years"),
-        ("all", "All"),
-    ]
-    since = request.GET.get("since", None)
-    
-    if since not in [key for key, label in since_choices]:
-        since = None
-
+    since = params.cleaned_data["since"] or None
     if since != "all":
         date_limit = {
             None: datetime.timedelta(days=31),
@@ -731,7 +740,7 @@ def review_requests_history(request, acronym, group_type=None):
                       "group": group,
                       "acronym": acronym,
                       "history": history,
-                      "since_choices": since_choices,
+                      "since_choices": params.SINCE_CHOICES,
                       "since": since,
                       "reviewer_email": reviewer_email
                   }))