diff --git a/ietf/doc/forms.py b/ietf/doc/forms.py index 53a8d3b07..8a480dcb8 100644 --- a/ietf/doc/forms.py +++ b/ietf/doc/forms.py @@ -6,6 +6,7 @@ import datetime import debug #pyflakes:ignore from django import forms from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.core.validators import validate_email from ietf.doc.fields import SearchableDocAliasesField, SearchableDocAliasField from ietf.doc.models import RelatedDocument, DocExtResource @@ -84,8 +85,42 @@ class NotifyForm(forms.Form): ) def clean_notify(self): - addrspecs = [x.strip() for x in self.cleaned_data["notify"].split(',')] - return ', '.join(addrspecs) + # As long as the widget is a Textarea, users will separate addresses with newlines, whether that matches the instructions or not + # We have been allowing nameaddrs for a long time (there are many Documents with namaddrs in their notify field) + # python set doesn't preserve order, so in an attempt to mostly preserve the order of what was entered, we'll use + # a dict (whose keys are guaranteed to be ordered) to cull out duplicates + + nameaddrs=dict() + duplicate_addrspecs = set() + bad_nameaddrs = [] + for nameaddr in self.cleaned_data["notify"].replace("\n", ",").split(","): + stripped = nameaddr.strip() + if stripped == "": + continue + if "<" in stripped: + if stripped[-1] != ">": + bad_nameaddrs.append(nameaddr) + continue + addrspec = stripped[stripped.find("<")+1:-1] + else: + addrspec = stripped + try: + validate_email(addrspec) + except ValidationError: + bad_nameaddrs.append(nameaddr) + if addrspec in nameaddrs: + duplicate_addrspecs.add(addrspec) + continue + else: + nameaddrs[addrspec] = stripped + error_messages = [] + if len(duplicate_addrspecs) != 0: + error_messages.append(f'Duplicate addresses: {", ".join(duplicate_addrspecs)}') + if len(bad_nameaddrs) != 0: + error_messages.append(f'Invalid addresses: {", ".join(bad_nameaddrs)}') + if len(error_messages) != 0: + raise ValidationError(" and ".join(error_messages)) + return ", ".join(nameaddrs.values()) class ActionHoldersForm(forms.Form): action_holders = SearchablePersonsField(required=False) diff --git a/ietf/doc/migrations/0048_send_notices.py b/ietf/doc/migrations/0048_allow_longer_notify.py similarity index 100% rename from ietf/doc/migrations/0048_send_notices.py rename to ietf/doc/migrations/0048_allow_longer_notify.py diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 36431cb02..5141c12f6 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -41,6 +41,7 @@ from ietf.doc.factories import ( DocumentFactory, DocEventFactory, CharterFactor IndividualRfcFactory, StateDocEventFactory, BallotPositionDocEventFactory, BallotDocEventFactory, DocumentAuthorFactory, NewRevisionDocEventFactory, StatusChangeFactory, BofreqFactory) +from ietf.doc.forms import NotifyForm from ietf.doc.fields import SearchableDocumentsField from ietf.doc.utils import create_ballot_if_not_open, uppercase_std_abbreviated_name from ietf.doc.views_search import ad_dashboard_group, ad_dashboard_group_type, shorten_group_name # TODO: red flag that we're importing from views in tests. Move these to utils. @@ -2926,3 +2927,43 @@ class PdfizedTests(TestCase): for ext in ('pdf','txt','html','anythingatall'): self.should_succeed(dict(name=rfc.name,rev=f'{r:02d}',ext=ext)) self.should_404(dict(name=rfc.name,rev='02')) + +class NotifyValidationTests(TestCase): + def test_notify_validation(self): + valid_values = [ + "foo@example.com, bar@example.com", + "Foo Bar <foobar@example.com>, baz@example.com", + "foo@example.com, ,bar@example.com,", # We're ignoring extra commas + "foo@example.com\nbar@example.com", # Yes, we're quietly accepting a newline as a comma + ] + bad_nameaddr_values = [ + "@example.com", + "foo", + "foo@", + "foo bar foobar@example.com", + ] + duplicate_values = [ + "foo@bar.com, bar@baz.com, foo@bar.com", + "Foo <foo@bar.com>, foobar <foo@bar.com>", + ] + both_duplicate_and_bad_values = [ + "foo@example.com, bar@, Foo <foo@example.com>", + "Foo <@example.com>, Bar <@example.com>", + ] + for v in valid_values: + self.assertTrue(NotifyForm({"notify": v}).is_valid()) + for v in bad_nameaddr_values: + f = NotifyForm({"notify": v}) + self.assertFalse(f.is_valid()) + self.assertTrue("Invalid addresses" in f.errors["notify"][0]) + self.assertFalse("Duplicate addresses" in f.errors["notify"][0]) + for v in duplicate_values: + f = NotifyForm({"notify": v}) + self.assertFalse(f.is_valid()) + self.assertFalse("Invalid addresses" in f.errors["notify"][0]) + self.assertTrue("Duplicate addresses" in f.errors["notify"][0]) + for v in both_duplicate_and_bad_values: + f = NotifyForm({"notify": v}) + self.assertFalse(f.is_valid()) + self.assertTrue("Invalid addresses" in f.errors["notify"][0]) + self.assertTrue("Duplicate addresses" in f.errors["notify"][0])