diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index d671228953..89c755bb26 100644 --- a/ietf/group/tests_review.py +++ b/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 = [ + "%00null@example.com", # urlencoded null character + "null@exa%00mple.com", # urlencoded null character + "\x00null@example.com", # 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)}" + ) diff --git a/ietf/group/views.py b/ietf/group/views.py index bc79599722..3529b31f68 100644 --- a/ietf/group/views.py +++ b/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 }))