From 4405d69047016c9c12b953952a27f584d5c9b67d Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Thu, 7 Dec 2023 14:16:34 +0100 Subject: [PATCH] :adhesive_bandage: [#3647] Prevent backend from crashing in case of invalid data Backport-of: #3670 --- src/openforms/formio/constants.py | 1 + .../models/submission_value_variable.py | 31 ++-- .../tests/form_logic/test_endpoint.py | 160 ++++++++++++++++++ .../tests/test_variables/test_model.py | 11 ++ src/openforms/utils/date.py | 40 ++++- 5 files changed, 223 insertions(+), 20 deletions(-) diff --git a/src/openforms/formio/constants.py b/src/openforms/formio/constants.py index 6a85f45103..69e21071ce 100644 --- a/src/openforms/formio/constants.py +++ b/src/openforms/formio/constants.py @@ -9,4 +9,5 @@ "npFamilyMembers": "object", "map": "array", "editgrid": "array", + "datetime": "datetime", } diff --git a/src/openforms/submissions/models/submission_value_variable.py b/src/openforms/submissions/models/submission_value_variable.py index 7ad1cb1d63..cf5713cd66 100644 --- a/src/openforms/submissions/models/submission_value_variable.py +++ b/src/openforms/submissions/models/submission_value_variable.py @@ -5,14 +5,14 @@ from django.core.serializers.json import DjangoJSONEncoder from django.db import models from django.utils import timezone -from django.utils.dateparse import parse_date, parse_datetime, parse_time +from django.utils.dateparse import parse_date from django.utils.functional import empty from django.utils.translation import gettext_lazy as _ from openforms.formio.service import FormioData from openforms.forms.models.form_variable import FormVariable from openforms.typing import DataMapping, JSONEncodable, JSONSerializable -from openforms.utils.date import format_date_value +from openforms.utils.date import format_date_value, parse_datetime, parse_time from openforms.variables.constants import FormVariableDataTypes from openforms.variables.service import get_static_variables @@ -358,28 +358,23 @@ def to_python(self) -> Any: return aware_date.date() maybe_naive_datetime = parse_datetime(self.value) - if maybe_naive_datetime is not None: - if timezone.is_aware(maybe_naive_datetime): - return maybe_naive_datetime.date() - return timezone.make_aware(maybe_naive_datetime).date() + if maybe_naive_datetime is None: + return - raise ValueError(f"Could not parse date '{self.value}'") # pragma: nocover + if timezone.is_aware(maybe_naive_datetime): + return maybe_naive_datetime.date() + return timezone.make_aware(maybe_naive_datetime).date() if self.value and data_type == FormVariableDataTypes.datetime: maybe_naive_datetime = parse_datetime(self.value) - if maybe_naive_datetime is not None: - if timezone.is_aware(maybe_naive_datetime): - return maybe_naive_datetime - return timezone.make_aware(maybe_naive_datetime) + if maybe_naive_datetime is None: + return - raise ValueError( - f"Could not parse datetime '{self.value}'" - ) # pragma: nocover + if timezone.is_aware(maybe_naive_datetime): + return maybe_naive_datetime + return timezone.make_aware(maybe_naive_datetime) if self.value and data_type == FormVariableDataTypes.time: - value = parse_time(self.value) - if value is not None: - return value - raise ValueError(f"Could not parse time '{self.value}'") # pragma: nocover + return parse_time(self.value) return self.value diff --git a/src/openforms/submissions/tests/form_logic/test_endpoint.py b/src/openforms/submissions/tests/form_logic/test_endpoint.py index 595bec962f..70da4d6432 100644 --- a/src/openforms/submissions/tests/form_logic/test_endpoint.py +++ b/src/openforms/submissions/tests/form_logic/test_endpoint.py @@ -1,6 +1,9 @@ +from django.test import tag + from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APIRequestFactory, APITestCase +from testfixtures import LogCapture from openforms.accounts.tests.factories import SuperUserFactory from openforms.forms.tests.factories import ( @@ -195,3 +198,160 @@ def test_updating_data_marks_step_as_applicable_again(self): data = response.json() self.assertTrue(data["submission"]["steps"][2]["isApplicable"]) + + @tag("gh-3647") + def test_sending_invalid_time_values(self): + submission = SubmissionFactory.from_components( + components_list=[ + {"type": "time", "key": "time"}, + {"type": "date", "key": "date"}, + {"type": "datetime", "key": "datetime"}, + ] + ) + + endpoint = reverse( + "api:submission-steps-logic-check", + kwargs={ + "submission_uuid": submission.uuid, + "step_uuid": submission.submissionstep_set.first().form_step.uuid, + }, + ) + self._add_submission_to_session(submission) + + with ( + self.subTest("Invalid time with good format"), + LogCapture() as log_capture, + ): + self.client.post(endpoint, data={"data": {"time": "25:00"}}) + + log_capture.check_present( + ( + "openforms.utils.date", + "INFO", + "Invalid time '25:00', falling back to 'None' instead.", + ) + ) + + with ( + self.subTest("Invalid time with bad format"), + LogCapture() as log_capture, + ): + self.client.post(endpoint, data={"data": {"time": "Invalid"}}) + + log_capture.check_present( + ( + "openforms.utils.date", + "INFO", + "Badly formatted time 'Invalid', falling back to 'None' instead.", + ) + ) + + with (self.subTest("Invalid date"), LogCapture() as log_capture): + self.client.post(endpoint, data={"data": {"date": "2020-13-46"}}) + + log_capture.check_present( + ( + "openforms.utils.date", + "INFO", + "Can't format date '2020-13-46', falling back to an empty string.", + ), + ( + "openforms.utils.date", + "INFO", + "Badly formatted datetime '2020-13-46', falling back to 'None' instead.", + ), + ) + + with (self.subTest("Invalid datetime"), LogCapture() as log_capture): + self.client.post( + endpoint, data={"data": {"datetime": "2022-13-46T00:00:00+02:00"}} + ) + + log_capture.check_present( + ( + "openforms.utils.date", + "INFO", + "Can't parse datetime '2022-13-46T00:00:00+02:00', falling back to 'None' instead.", + ) + ) + + def test_handling_none_values_in_logic(self): + submission = SubmissionFactory.from_components( + components_list=[ + {"type": "time", "key": "time"}, + {"type": "date", "key": "date"}, + {"type": "datetime", "key": "datetime"}, + {"type": "string", "key": "result"}, + {"type": "date", "key": "resultDate"}, + {"type": "datetime", "key": "resultDatetime"}, + ] + ) + FormLogicFactory.create( + form=submission.form, + json_logic_trigger={ + "and": [ + {"==": [None, {"var": "time"}]}, + {"==": [None, {"var": "date"}]}, + {"==": [None, {"var": "datetime"}]}, + ] + }, + actions=[ + { + "variable": "result", + "action": { + "type": "variable", + "value": "All the variables were None", + }, + } + ], + ) + FormLogicFactory.create( + form=submission.form, + json_logic_trigger=True, + actions=[ + { + "variable": "resultDate", + "action": { + "type": "variable", + "value": {"+": [{"var": "date"}, {"duration": "P1M"}]}, + }, + }, + { + "variable": "resultDatetime", + "action": { + "type": "variable", + "value": {"+": [{"var": "datetime"}, {"duration": "P1M"}]}, + }, + }, + ], + ) + endpoint = reverse( + "api:submission-steps-logic-check", + kwargs={ + "submission_uuid": submission.uuid, + "step_uuid": submission.submissionstep_set.first().form_step.uuid, + }, + ) + self._add_submission_to_session(submission) + + response = self.client.post( + endpoint, + data={ + "data": { + "time": "Invalid", + "date": "2020-13-46", + "datetime": "2022-13-46T00:00:00+02:00", + } + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json() + + self.assertEqual(data["step"]["data"]["result"], "All the variables were None") + self.assertEqual(data["step"]["data"]["time"], "Invalid") + self.assertEqual(data["step"]["data"]["date"], "2020-13-46") + self.assertEqual(data["step"]["data"]["datetime"], "2022-13-46T00:00:00+02:00") + self.assertNotIn("resultDate", data["step"]["data"]) + self.assertNotIn("resultDatetime", data["step"]["data"]) diff --git a/src/openforms/submissions/tests/test_variables/test_model.py b/src/openforms/submissions/tests/test_variables/test_model.py index 9203c9daf8..99dbb98b92 100644 --- a/src/openforms/submissions/tests/test_variables/test_model.py +++ b/src/openforms/submissions/tests/test_variables/test_model.py @@ -210,6 +210,17 @@ def test_to_python(self): # we'll see if that causes any issues self.assertEqual(time_value, time(11, 15)) + with self.subTest("Invalid time"): + time_var = SubmissionValueVariableFactory.create( + form_variable__user_defined=True, + form_variable__data_type=FormVariableDataTypes.time, + value="Invalid date", # Issue 3647 + ) + + time_value = time_var.to_python() + + self.assertIsNone(time_value) + def test_is_initially_prefilled_is_set(self): config = { "display": "form", diff --git a/src/openforms/utils/date.py b/src/openforms/utils/date.py index c38190f50d..d23aef2387 100644 --- a/src/openforms/utils/date.py +++ b/src/openforms/utils/date.py @@ -1,7 +1,11 @@ import logging -from datetime import date, datetime +from datetime import date, datetime, time from django.utils import timezone +from django.utils.dateparse import ( + parse_datetime as _parse_datetime, + parse_time as _parse_time, +) logger = logging.getLogger(__name__) @@ -13,7 +17,9 @@ def format_date_value(date_value: str) -> str: try: parsed_date = datetime.strptime(date_value, "%Y%m%d").date() except ValueError: - logger.info("Can't parse date %s, using empty value.", date_value) + logger.info( + "Can't format date '%s', falling back to an empty string.", date_value + ) return "" return parsed_date.isoformat() @@ -31,3 +37,33 @@ def parse_date(value: str) -> date: dt = datetime.fromisoformat(value) assert dt.tzinfo is not None, "Expected the input variable to be timezone aware!" return timezone.localtime(value=dt).date() + + +def parse_datetime(value: str) -> None | datetime: + try: + datetime_value = _parse_datetime(value) + except ValueError: + logger.info("Can't parse datetime '%s', falling back to 'None' instead.", value) + return + + if datetime_value is None: + logger.info( + "Badly formatted datetime '%s', falling back to 'None' instead.", value + ) + return + + return datetime_value + + +def parse_time(value: str) -> None | time: + try: + time_value = _parse_time(value) + except ValueError: + logger.info("Invalid time '%s', falling back to 'None' instead.", value) + return + + if time_value is None: + logger.info("Badly formatted time '%s', falling back to 'None' instead.", value) + return + + return time_value