From 130a5268d2c258f0abbbf708deaf01d337f449c1 Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Fri, 27 Dec 2024 15:47:16 +0000 Subject: [PATCH 1/3] add validator --- exporter/applications/forms/parties.py | 3 ++ exporter/core/validators.py | 12 +++++++ .../applications/forms/test_parties.py | 33 +++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/exporter/applications/forms/parties.py b/exporter/applications/forms/parties.py index 836c9826f9..823ebfeed3 100644 --- a/exporter/applications/forms/parties.py +++ b/exporter/applications/forms/parties.py @@ -11,6 +11,7 @@ from core.forms.widgets import Autocomplete from exporter.core.constants import CaseTypes, FileUploadFileTypes from exporter.core.services import get_countries +from exporter.core.validators import SpecialCharacterStringValidator from lite_content.lite_exporter_frontend import strings from lite_content.lite_exporter_frontend.applications import PartyForm, PartyTypeForm from lite_forms.common import country_question @@ -222,6 +223,7 @@ class PartyNameForm(BaseForm): 80, f"End user name should be 80 characters or less", ), + SpecialCharacterStringValidator(), ], ) @@ -300,6 +302,7 @@ class PartyAddressForm(BaseForm): address = forms.CharField( widget=forms.Textarea(attrs={"rows": "10"}), error_messages={"required": "Enter an address"}, + validators=[SpecialCharacterStringValidator()], ) country = forms.ChoiceField( choices=[("", "Select a country")], error_messages={"required": "Select the country"} diff --git a/exporter/core/validators.py b/exporter/core/validators.py index f7960cad7b..2b0345e942 100644 --- a/exporter/core/validators.py +++ b/exporter/core/validators.py @@ -1,5 +1,6 @@ from datetime import date from dateutil.relativedelta import relativedelta +import re from django.core.exceptions import ValidationError from django.utils import timezone @@ -52,3 +53,14 @@ def __init__(self, message, **kwargs): def __call__(self, value): if value > (date.today() + self.relativedelta): raise ValidationError(self.message) + + +class SpecialCharacterStringValidator: + message = "There's a invalid special charactor in this field." + regex_string = r"^[\000-\031]" + + def __call__(self, value): + if value: + match_regex = re.compile(self.regex_string) + if bool(match_regex.match(value)): + raise ValidationError(self.message) diff --git a/unit_tests/exporter/applications/forms/test_parties.py b/unit_tests/exporter/applications/forms/test_parties.py index 723bec4b5d..df1548a2dd 100644 --- a/unit_tests/exporter/applications/forms/test_parties.py +++ b/unit_tests/exporter/applications/forms/test_parties.py @@ -94,6 +94,17 @@ def test_consignee_name_form(data, valid, errors): False, {"name": [f"End user name should be 80 characters or less"]}, ), + ( + {"name": "\x02 control chars not allowed"}, + False, + {"name": ["There's a invalid special charactor in this field."]}, + ), + ({"name": "control \x1A is allowed"}, True, None), + ( + {"name": "\x00 control chars not allowed"}, + False, + {"name": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, + ), ), ) def test_end_user_name_form(data, valid, errors): @@ -168,6 +179,17 @@ def test_consignee_website_form(data, valid, errors): ({"address": "this\r\nis\r\ninvalid", "country": "aus"}, True, None), ({"address": "this_is_not", "country": "aus"}, True, None), ({"address": "this\w\ais\a\ainvalid", "country": "aus"}, True, None), + ( + {"address": "\x02 control chars not allowed", "country": "aus"}, + False, + {"address": ["There's a invalid special charactor in this field."]}, + ), + ({"address": "control \x1A is allowed", "country": "aus"}, True, None), + ( + {"address": "\x00 control chars not allowed", "country": "aus"}, + False, + {"address": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, + ), ), ) @patch("exporter.applications.forms.parties.get_countries") @@ -193,6 +215,17 @@ class Request: ({"address": "", "country": ""}, False, {"address": ["Enter an address"], "country": ["Select the country"]}), ({"address": "This-is-a-valid-address", "country": "aus"}, True, None), ({"address": "this\r\nis\r\ninvalid", "country": "aus"}, True, None), + ( + {"address": "\x02 control chars not allowed", "country": "aus"}, + False, + {"address": ["There's a invalid special charactor in this field."]}, + ), + ({"address": "control \x1A is allowed", "country": "aus"}, True, None), + ( + {"address": "\x00 control chars not allowed", "country": "aus"}, + False, + {"address": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, + ), ), ) @patch("exporter.applications.forms.parties.get_countries") From 84faff4a3b37990c83960068218239deef903cf2 Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Mon, 30 Dec 2024 18:43:41 +0000 Subject: [PATCH 2/3] changed so it cleans data --- core/helpers.py | 4 + exporter/applications/forms/parties.py | 12 ++- exporter/core/validators.py | 12 --- .../applications/forms/test_parties.py | 78 +++++++++++-------- 4 files changed, 58 insertions(+), 48 deletions(-) diff --git a/core/helpers.py b/core/helpers.py index 44c7870567..baab6212fb 100644 --- a/core/helpers.py +++ b/core/helpers.py @@ -72,3 +72,7 @@ def stream_document_response(api_response): ]: response.headers[header_to_copy] = api_response.headers[header_to_copy] return response + + +def remove_non_printable_characters(str): + return "".join([c for c in str if ord(c) > 31 or ord(c) in [9, 10, 13]]) diff --git a/exporter/applications/forms/parties.py b/exporter/applications/forms/parties.py index 823ebfeed3..e7be51a22c 100644 --- a/exporter/applications/forms/parties.py +++ b/exporter/applications/forms/parties.py @@ -1,3 +1,4 @@ +from core.helpers import remove_non_printable_characters from crispy_forms_gds.choices import Choice from crispy_forms_gds.helper import FormHelper from crispy_forms_gds.layout import Layout, Submit, HTML @@ -11,7 +12,6 @@ from core.forms.widgets import Autocomplete from exporter.core.constants import CaseTypes, FileUploadFileTypes from exporter.core.services import get_countries -from exporter.core.validators import SpecialCharacterStringValidator from lite_content.lite_exporter_frontend import strings from lite_content.lite_exporter_frontend.applications import PartyForm, PartyTypeForm from lite_forms.common import country_question @@ -223,10 +223,13 @@ class PartyNameForm(BaseForm): 80, f"End user name should be 80 characters or less", ), - SpecialCharacterStringValidator(), ], ) + def clean_name(self): + name = self.cleaned_data["name"] + return remove_non_printable_characters(name) + def get_layout_fields(self): return ("name",) @@ -302,7 +305,6 @@ class PartyAddressForm(BaseForm): address = forms.CharField( widget=forms.Textarea(attrs={"rows": "10"}), error_messages={"required": "Enter an address"}, - validators=[SpecialCharacterStringValidator()], ) country = forms.ChoiceField( choices=[("", "Select a country")], error_messages={"required": "Select the country"} @@ -318,6 +320,10 @@ def __init__(self, *args, **kwargs): country_choices = [(country["id"], country["name"]) for country in countries] self.fields["country"].choices += country_choices + def clean_address(self): + address = self.cleaned_data["address"] + return remove_non_printable_characters(address) + def get_layout_fields(self): return ( "address", diff --git a/exporter/core/validators.py b/exporter/core/validators.py index 2b0345e942..f7960cad7b 100644 --- a/exporter/core/validators.py +++ b/exporter/core/validators.py @@ -1,6 +1,5 @@ from datetime import date from dateutil.relativedelta import relativedelta -import re from django.core.exceptions import ValidationError from django.utils import timezone @@ -53,14 +52,3 @@ def __init__(self, message, **kwargs): def __call__(self, value): if value > (date.today() + self.relativedelta): raise ValidationError(self.message) - - -class SpecialCharacterStringValidator: - message = "There's a invalid special charactor in this field." - regex_string = r"^[\000-\031]" - - def __call__(self, value): - if value: - match_regex = re.compile(self.regex_string) - if bool(match_regex.match(value)): - raise ValidationError(self.message) diff --git a/unit_tests/exporter/applications/forms/test_parties.py b/unit_tests/exporter/applications/forms/test_parties.py index df1548a2dd..aec1f5e8d0 100644 --- a/unit_tests/exporter/applications/forms/test_parties.py +++ b/unit_tests/exporter/applications/forms/test_parties.py @@ -84,6 +84,26 @@ def test_consignee_name_form(data, valid, errors): assert form.errors == errors +@pytest.mark.parametrize( + "data, expected", + ( + ({"name": "\x02 test1"}, " test1"), + ({"name": "\x02test2"}, "test2"), + ({"name": "this is \n test3"}, "this is \n test3"), + ({"name": "this is \t test4"}, "this is \t test4"), + ({"name": "this is \r test5"}, "this is \r test5"), + ({"name": "namé 6"}, "namé 6"), + ({"name": "namé's"}, "namé's"), + ({"name": "test 8"}, "test 8"), + ), +) +def test_consignee_name_removes_non_printable(data, expected): + form = parties.ConsigneeNameForm(data=data) + + form.is_valid() + assert form.cleaned_data["name"] == expected + + @pytest.mark.parametrize( "data, valid, errors", ( @@ -94,17 +114,6 @@ def test_consignee_name_form(data, valid, errors): False, {"name": [f"End user name should be 80 characters or less"]}, ), - ( - {"name": "\x02 control chars not allowed"}, - False, - {"name": ["There's a invalid special charactor in this field."]}, - ), - ({"name": "control \x1A is allowed"}, True, None), - ( - {"name": "\x00 control chars not allowed"}, - False, - {"name": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, - ), ), ) def test_end_user_name_form(data, valid, errors): @@ -179,17 +188,6 @@ def test_consignee_website_form(data, valid, errors): ({"address": "this\r\nis\r\ninvalid", "country": "aus"}, True, None), ({"address": "this_is_not", "country": "aus"}, True, None), ({"address": "this\w\ais\a\ainvalid", "country": "aus"}, True, None), - ( - {"address": "\x02 control chars not allowed", "country": "aus"}, - False, - {"address": ["There's a invalid special charactor in this field."]}, - ), - ({"address": "control \x1A is allowed", "country": "aus"}, True, None), - ( - {"address": "\x00 control chars not allowed", "country": "aus"}, - False, - {"address": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, - ), ), ) @patch("exporter.applications.forms.parties.get_countries") @@ -208,6 +206,31 @@ class Request: assert form.errors == errors +@pytest.mark.parametrize( + "data, expected", + ( + ({"address": "1 somewhere", "country": "aus"}, "1 somewhere"), + ({"address": "1 \x02 somewhere", "country": "aus"}, "1 somewhere"), + ({"address": "1 \x02 \n somewhere's", "country": "aus"}, "1 \n somewhere's"), + ({"address": "1 \x01somewhere", "country": "aus"}, "somewhere"), + ({"address": "1 \x03 \n somewhere", "country": "aus"}, "1 somewhere"), + ({"address": "1 \x03 \n ô somewhere", "country": "aus"}, "1 ô somewhere"), + ({"address": "1 \x02 \r somewhere's", "country": "aus"}, "1 \r somewhere's"), + ({"address": "1 \x02 \t somewhere's", "country": "aus"}, "1 \t somewhere's"), + ), +) +@patch("exporter.applications.forms.parties.get_countries") +def test_end_user_address_removes_non_printable(mock_get_countries, data, expected): + class Request: + csp_nonce = "test" + + request = Request() + mock_get_countries.return_value = [{"id": "aus", "name": "Austria"}, {"id": "fr", "name": "France"}] + form = parties.EndUserAddressForm(request=request, data=data) + + form.is_valid() + + @pytest.mark.parametrize( "data, valid, errors", ( @@ -215,17 +238,6 @@ class Request: ({"address": "", "country": ""}, False, {"address": ["Enter an address"], "country": ["Select the country"]}), ({"address": "This-is-a-valid-address", "country": "aus"}, True, None), ({"address": "this\r\nis\r\ninvalid", "country": "aus"}, True, None), - ( - {"address": "\x02 control chars not allowed", "country": "aus"}, - False, - {"address": ["There's a invalid special charactor in this field."]}, - ), - ({"address": "control \x1A is allowed", "country": "aus"}, True, None), - ( - {"address": "\x00 control chars not allowed", "country": "aus"}, - False, - {"address": ["There's a invalid special charactor in this field.", "Null characters are not allowed."]}, - ), ), ) @patch("exporter.applications.forms.parties.get_countries") From 7363d75bc4b5380b70cc2b5abb5cd3634ede6417 Mon Sep 17 00:00:00 2001 From: Gurdeep Atwal Date: Mon, 6 Jan 2025 15:13:56 +0000 Subject: [PATCH 3/3] add comment --- core/helpers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/helpers.py b/core/helpers.py index baab6212fb..a5d58a247f 100644 --- a/core/helpers.py +++ b/core/helpers.py @@ -75,4 +75,6 @@ def stream_document_response(api_response): def remove_non_printable_characters(str): + # Given a string this will remove all non_printable_characters from ascii table + # Chars 9(/h), 10 (/n), 13(/r) are maintained return "".join([c for c in str if ord(c) > 31 or ord(c) in [9, 10, 13]])