From ab9b62f54b6c27c992f32242d43727f69de99a97 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 7 Nov 2019 11:25:18 -0500 Subject: [PATCH] Update validators and filter to remove strings that contain only whitespace The validator ListItemRequired() was only checking for None and an empty string, not for strings that were multiple whitespace characters. This fixes this issue by checking each item with regex to make sure it contains non whitespace characters The filter remove_empty_string() also was not checking for strings that were multiple whitespace characters. This was also fixed by using regex tomake sure that the string contains non whitespace characters, and also clips any trailing whitespace. --- .secrets.baseline | 11 +---------- atst/forms/application.py | 15 +++++++++++---- atst/forms/forms.py | 7 ++++++- atst/forms/portfolio.py | 2 -- atst/forms/validators.py | 6 ++++-- tests/forms/test_base_form.py | 22 +++++++++++++++++++++- tests/forms/test_validators.py | 17 +++++++++++++++++ 7 files changed, 60 insertions(+), 20 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 9e94b782..1060e869 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2019-11-01T18:50:54Z", + "generated_at": "2019-11-06T21:35:23Z", "plugins_used": [ { "base64_limit": 4.5, @@ -170,15 +170,6 @@ "type": "Private Key" } ], - "tests/forms/test_validators.py": [ - { - "hashed_secret": "260408f687da9094705a841acda9b029563053ee", - "is_secret": false, - "is_verified": false, - "line_number": 30, - "type": "Hex High Entropy String" - } - ], "tests/routes/task_orders/test_new.py": [ { "hashed_secret": "e4f14805dfd1e6af030359090c535e149e6b4207", diff --git a/atst/forms/application.py b/atst/forms/application.py index 16db3827..ea71d1e3 100644 --- a/atst/forms/application.py +++ b/atst/forms/application.py @@ -7,24 +7,31 @@ from atst.utils.localization import translate class EditEnvironmentForm(BaseForm): name = StringField( - label=translate("forms.environments.name_label"), validators=[Required()] + label=translate("forms.environments.name_label"), + validators=[Required()], + filters=[BaseForm.remove_empty_string], ) class NameAndDescriptionForm(BaseForm): name = StringField( - label=translate("forms.application.name_label"), validators=[Required()] + label=translate("forms.application.name_label"), + validators=[Required()], + filters=[BaseForm.remove_empty_string], ) description = TextAreaField( label=translate("forms.application.description_label"), validators=[Optional()], - filters=[lambda x: x or None], + filters=[BaseForm.remove_empty_string], ) class EnvironmentsForm(BaseForm): environment_names = FieldList( - StringField(label=translate("forms.application.environment_names_label")), + StringField( + label=translate("forms.application.environment_names_label"), + filters=[BaseForm.remove_empty_string], + ), validators=[ ListItemRequired( message=translate( diff --git a/atst/forms/forms.py b/atst/forms/forms.py index 4287c81c..36bb7994 100644 --- a/atst/forms/forms.py +++ b/atst/forms/forms.py @@ -1,5 +1,6 @@ from flask_wtf import FlaskForm from flask import current_app, request as http_request +import re from atst.utils.flash import formatted_flash as flash @@ -38,4 +39,8 @@ class BaseForm(FlaskForm): @classmethod def remove_empty_string(cls, value): - return value or None + # only return strings that contain non whitespace characters + if value and re.search(r"\S", value): + return value.strip() + else: + return None diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 6e807876..d9991471 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -78,7 +78,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.complexity.description"), choices=APPLICATION_COMPLEXITY, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) @@ -94,7 +93,6 @@ class PortfolioCreationForm(BaseForm): description=translate("forms.task_order.dev_team.description"), choices=DEV_TEAM, default=None, - filters=[BaseForm.remove_empty_string], widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), ) diff --git a/atst/forms/validators.py b/atst/forms/validators.py index cac21e3e..a16bf4f5 100644 --- a/atst/forms/validators.py +++ b/atst/forms/validators.py @@ -65,10 +65,12 @@ def Name(message=translate("forms.validators.name_message")): def ListItemRequired( message=translate("forms.validators.list_item_required_message"), - empty_values=("", None), + empty_values=[None], ): def _list_item_required(form, field): - non_empty_values = [v for v in field.data if v not in empty_values] + non_empty_values = [ + v for v in field.data if (v not in empty_values and re.search(r"\S", v)) + ] if len(non_empty_values) == 0: raise ValidationError(message) diff --git a/tests/forms/test_base_form.py b/tests/forms/test_base_form.py index fe6456e7..5587649b 100644 --- a/tests/forms/test_base_form.py +++ b/tests/forms/test_base_form.py @@ -1,5 +1,5 @@ import pytest -from wtforms.fields import RadioField +from wtforms.fields import RadioField, FieldList, StringField from werkzeug.datastructures import ImmutableMultiDict from atst.forms.forms import BaseForm @@ -16,6 +16,12 @@ class FormWithChoices(BaseForm): ) +class FormWithList(BaseForm): + list = FieldList( + StringField("a very fancy list", filters=[BaseForm.remove_empty_string]) + ) + + class TestBaseForm: class Foo: person = {"force_side": None} @@ -34,3 +40,17 @@ class TestBaseForm: form_data_3 = ImmutableMultiDict({"force_side": "dark"}) form_3 = FormWithChoices(form_data_3, obj=self.obj) assert form_3.data["force_side"] is "dark" + + @pytest.mark.parametrize( + "form_data", + [["testing", "", "QA"], ["testing", " ", "QA"], ["testing", None, "QA"]], + ) + def test_blank_list_items_removed(self, form_data): + form = FormWithList(list=form_data) + assert form.validate(flash_invalid=False) + assert not form.data == ["testing", "QA"] + + def test_remove_empty_string_clips_whitespace(self): + form = FormWithList(list=[" QA", " testing "]) + assert form.validate(flash_invalid=False) + assert form.list.data == ["QA", "testing"] diff --git a/tests/forms/test_validators.py b/tests/forms/test_validators.py index f19d06cb..bac06f47 100644 --- a/tests/forms/test_validators.py +++ b/tests/forms/test_validators.py @@ -85,3 +85,20 @@ class TestFileLength: dummy_field.data = "random string" assert validator(dummy_form, dummy_field) + + +class TestListItemRequired: + @pytest.mark.parametrize("valid", [[" a", ""], ["a ", ""], ["a", ""]]) + def test_ListItemRequired(self, valid, dummy_form, dummy_field): + validator = ListItemRequired() + dummy_field.data = valid + validator(dummy_form, dummy_field) + + @pytest.mark.parametrize("invalid", [[""], [" "], [None], []]) + def test_ListItemRequired_rejects_blank_names( + self, invalid, dummy_form, dummy_field + ): + validator = ListItemRequired() + dummy_field.data = invalid + with pytest.raises(ValidationError): + validator(dummy_form, dummy_field)