From 553e0473062e99efdbf2ed5178303022e7955dd4 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 15:15:50 +0100 Subject: [PATCH 1/6] Refactor the jsonfield doctest to a simpler unit test. --- news/5.tests.1 | 2 ++ plone/schema/jsonfield.py | 9 +++++---- plone/schema/tests/test_doctests.py | 8 -------- plone/schema/tests/test_jsonfield.py | 16 ++++++++++++++++ 4 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 news/5.tests.1 delete mode 100644 plone/schema/tests/test_doctests.py create mode 100644 plone/schema/tests/test_jsonfield.py diff --git a/news/5.tests.1 b/news/5.tests.1 new file mode 100644 index 0000000..85a364d --- /dev/null +++ b/news/5.tests.1 @@ -0,0 +1,2 @@ +Refactor the ``jsonfield`` doctest to a simpler unit test. +[maurits] \ No newline at end of file diff --git a/plone/schema/jsonfield.py b/plone/schema/jsonfield.py index c488b1f..83a2e7b 100644 --- a/plone/schema/jsonfield.py +++ b/plone/schema/jsonfield.py @@ -54,14 +54,15 @@ def fromUnicode(self, value): Value can be a valid JSON object: - >>> JSONField().fromUnicode('{"items": []}') - {'items': []} + JSONField().fromUnicode('{"items": []}') or it can be a Python dict stored as string: - >>> JSONField().fromUnicode("{'items': []}") - {'items': []} + JSONField().fromUnicode("{'items': []}") + In both cases the result is: + + {"items": []} """ try: v = json.loads(value) diff --git a/plone/schema/tests/test_doctests.py b/plone/schema/tests/test_doctests.py deleted file mode 100644 index fe4d706..0000000 --- a/plone/schema/tests/test_doctests.py +++ /dev/null @@ -1,8 +0,0 @@ -"""Tests""" - -import doctest -import unittest - - -def test_suite(): - return unittest.TestSuite((doctest.DocTestSuite("plone.schema.jsonfield"),)) diff --git a/plone/schema/tests/test_jsonfield.py b/plone/schema/tests/test_jsonfield.py new file mode 100644 index 0000000..9721924 --- /dev/null +++ b/plone/schema/tests/test_jsonfield.py @@ -0,0 +1,16 @@ +import unittest + + +class TestJsonField(unittest.TestCase): + + def test_fromUnicode(self): + from plone.schema.jsonfield import JSONField + + # Value can be a valid JSON object: + self.assertDictEqual( + JSONField().fromUnicode('{"items": []}'), + {"items": []}, + ) + + # or it can be a Python dict stored as string: + self.assertDictEqual(JSONField().fromUnicode("{'items': []}"), {"items": []}) From 78ce5be3a5679c4a3f6c4ee409b74260e9a99e49 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 15:25:53 +0100 Subject: [PATCH 2/6] Add basic tests for the email field. --- news/5.tests.2 | 2 ++ plone/schema/tests/test_email.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 news/5.tests.2 create mode 100644 plone/schema/tests/test_email.py diff --git a/news/5.tests.2 b/news/5.tests.2 new file mode 100644 index 0000000..e7f681e --- /dev/null +++ b/news/5.tests.2 @@ -0,0 +1,2 @@ +Add basic tests for the email field. +[maurits] \ No newline at end of file diff --git a/plone/schema/tests/test_email.py b/plone/schema/tests/test_email.py new file mode 100644 index 0000000..1d4f5d8 --- /dev/null +++ b/plone/schema/tests/test_email.py @@ -0,0 +1,32 @@ +import unittest + + +class TestEmail(unittest.TestCase): + + def test_fromUnicode(self): + from plone.schema.email import Email + from plone.schema.email import InvalidEmail + + # Value must be string + self.assertEqual( + Email().fromUnicode("arthur@example.org"), + "arthur@example.org", + ) + with self.assertRaises(InvalidEmail): + Email().fromUnicode(b"arthur@example.org") + + # We strip spaces. + self.assertEqual( + Email().fromUnicode(" arthur@example.org "), + "arthur@example.org", + ) + + # We do some validation + with self.assertRaises(InvalidEmail): + Email().fromUnicode("arthur") + with self.assertRaises(InvalidEmail): + Email().fromUnicode("arthur@") + with self.assertRaises(InvalidEmail): + Email().fromUnicode("arthur@one") + with self.assertRaises(InvalidEmail): + Email().fromUnicode("@one.two") From eb8db6578a458bffbc859b04cd1a309da6dd426a Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 16:17:03 +0100 Subject: [PATCH 3/6] Fix email validation. * allow apostrophes * allow accented characters * allow ampersand in the user part * do not allows spaces. * accept TLDs with more than 4 characters --- news/30.bugfix | 7 +++ plone/schema/email.py | 87 +++++++++++++++++++++++++++++--- plone/schema/tests/test_email.py | 58 ++++++++++++++++++++- 3 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 news/30.bugfix diff --git a/news/30.bugfix b/news/30.bugfix new file mode 100644 index 0000000..f269604 --- /dev/null +++ b/news/30.bugfix @@ -0,0 +1,7 @@ +Fix email validation: +* allow apostrophes +* allow accented characters +* allow ampersand in the user part +* do not allows spaces. +* accept TLDs with more than 4 characters +[maurits] diff --git a/plone/schema/email.py b/plone/schema/email.py index 962f863..adf7ca1 100644 --- a/plone/schema/email.py +++ b/plone/schema/email.py @@ -5,15 +5,9 @@ from zope.schema.interfaces import INativeStringLine from zope.schema.interfaces import ValidationError -import re - _ = MessageFactory("plone") -# Taken from http://www.regular-expressions.info/email.html -_isemail = r"[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}" -_isemail = re.compile(_isemail).match - class IEmail(INativeStringLine): """A field containing an email address""" @@ -23,6 +17,79 @@ class InvalidEmail(ValidationError): __doc__ = _("""The specified email is not valid.""") +def _isemail(value): + r"""Is this a valid email? + + https://www.regular-expressions.info/email.html has some hints on how to + check for a valid email with regular expressions. It also gives reasons + for why you may *not* want to use them. A too simple regex will work for + most cases, but may give false negatives: it will treat a rare but valid + email address as invalid. A complex regex may still allow some invalid + email addresses, and is hard to debug in case of errors. + + Originally we had this regex, unchanged between 2013 and 2024: + + import re + _isemail = r"[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}" + _isemail = re.compile(_isemail).match + + Problems: + + 1. It does not allow apostrophes. + Example: o'hara@example.org + See: https://github.com/plone/plone.schema/issues/6 + + 2. It does not allow accented characters. + Example: jens@österreich.example.com + See: https://github.com/plone/plone.schema/issues/9 + + 3. It does not allow ampersand in the user part. + Example: q&a@.example.com + See: https://github.com/plone/plone.schema/issues/15 + + 4. It allows spaces. + Example: "test@aha.ok hello" + See: https://github.com/plone/plone.schema/issues/30 + + 5. It correctly accepts TLDs with more than 4 characters, + but this seems by accident, so it could get lost in an update. + Example: me@you.online + See: https://github.com/plone/plone.schema/issues/30 + """ + # We only accept a string. + if not isinstance(value, str): + return False + + # It is up to the caller to strip spaces, newlines, etc. + if value != value.strip(): + return False + if len(value.split()) != 1: + return False + + # only one @ sign + if value.count("@") != 1: + return False + + # at least one dot in the domain + user, domain = value.split("@") + if "." not in domain: + return False + + # minimum lengths for user and domain + if not user: + return False + if len(domain) < 3: + return False + + # Tthe maximum length of an email address that can be handled by SMTP + # is 254 characters. + if len(value) > 254: + return False + + # We have found no problems. + return True + + @implementer(IEmail, IFromUnicode) class Email(NativeStringLine): """Email schema field""" @@ -34,6 +101,14 @@ def _validate(self, value): raise InvalidEmail(value) + def fromBytes(self, value): + # Originally, fromBytes was not defined. + # Upstream NativeStringLine had it, but without the 'strip' + # that we added in fromUnicode. + value = value.strip().decode("utf-8") + self.validate(value) + return value + def fromUnicode(self, value): v = str(value.strip()) self.validate(v) diff --git a/plone/schema/tests/test_email.py b/plone/schema/tests/test_email.py index 1d4f5d8..7f1ce0c 100644 --- a/plone/schema/tests/test_email.py +++ b/plone/schema/tests/test_email.py @@ -12,8 +12,6 @@ def test_fromUnicode(self): Email().fromUnicode("arthur@example.org"), "arthur@example.org", ) - with self.assertRaises(InvalidEmail): - Email().fromUnicode(b"arthur@example.org") # We strip spaces. self.assertEqual( @@ -30,3 +28,59 @@ def test_fromUnicode(self): Email().fromUnicode("arthur@one") with self.assertRaises(InvalidEmail): Email().fromUnicode("@one.two") + + def test_fromBytes(self): + from plone.schema.email import Email + from plone.schema.email import InvalidEmail + + # Value must be bytes + self.assertEqual( + Email().fromBytes(b"arthur@example.org"), + "arthur@example.org", + ) + + # We strip spaces. + self.assertEqual( + Email().fromBytes(b" arthur@example.org "), + "arthur@example.org", + ) + + # We do some validation + with self.assertRaises(InvalidEmail): + Email().fromBytes(b"arthur@one") + + def test_validation(self): + # Let's test the email validation directly, without the field. + from plone.schema.email import _isemail + + # Some good: + self.assertTrue(_isemail("arthur@example.org")) + + # Some bad: + self.assertFalse(_isemail("")) + self.assertFalse(_isemail(" ")) + self.assertFalse(_isemail(" arthur@example.org")) + self.assertFalse(_isemail("arthur@example.org\n")) + self.assertFalse(_isemail("arthur\t@example.org")) + self.assertFalse(_isemail("arthur@one")) + self.assertFalse(_isemail("arthur@example@org")) + self.assertFalse(_isemail("x" * 254 + "@too.long")) + + # Explicitly test some examples from the docstring, + # reported in the issue tracker. + + # 1. allow apostrophes + self.assertTrue(_isemail("o'hara@example.org")) + + # 2. allow accented characters + self.assertTrue(_isemail("jens@österreich.example.com")) + + # 3. allow ampersand in the user part + self.assertTrue(_isemail("q&a@.example.com")) + + # 4. do not allows spaces. + self.assertFalse(_isemail("test@aha.ok hello")) + + # 5. accept TLDs with more than 4 characters + self.assertTrue(_isemail("me@you.online")) + self.assertTrue(_isemail("me@example.museum")) From 08189019098b358a28afab09ef42e18b10f69e24 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 16:53:54 +0100 Subject: [PATCH 4/6] Update classifiers. --- setup.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.py b/setup.py index 2b4d068..c52b664 100644 --- a/setup.py +++ b/setup.py @@ -22,12 +22,15 @@ "Framework :: Zope :: 5", "Framework :: Plone", "Framework :: Plone :: 6.0", + "Framework :: Plone :: 6.1", "Framework :: Plone :: Core", "Programming Language :: Python", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "Topic :: Software Development :: Libraries :: Python Modules", "License :: OSI Approved :: BSD License", ], From 32c1086a581dcc892e5c9cc20b5701d99139513e Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 17:18:52 +0100 Subject: [PATCH 5/6] fix typo in comment --- plone/schema/email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plone/schema/email.py b/plone/schema/email.py index adf7ca1..fb2f0b7 100644 --- a/plone/schema/email.py +++ b/plone/schema/email.py @@ -81,7 +81,7 @@ def _isemail(value): if len(domain) < 3: return False - # Tthe maximum length of an email address that can be handled by SMTP + # The maximum length of an email address that can be handled by SMTP # is 254 characters. if len(value) > 254: return False From 9627aadfc51ea38687fd568ea79622ce9edd7dd5 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Tue, 18 Feb 2025 19:26:47 +0100 Subject: [PATCH 6/6] Better check of domain part. Thanks for the suggestion, @ale-rt. --- plone/schema/email.py | 9 ++++----- plone/schema/tests/test_email.py | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plone/schema/email.py b/plone/schema/email.py index fb2f0b7..0233ae0 100644 --- a/plone/schema/email.py +++ b/plone/schema/email.py @@ -70,16 +70,15 @@ def _isemail(value): if value.count("@") != 1: return False - # at least one dot in the domain + # At least one dot in the domain. And when split on dot, + # each part must not be empty. user, domain = value.split("@") - if "." not in domain: + if not all(domain.partition(".")): return False - # minimum lengths for user and domain + # user part must not be empty if not user: return False - if len(domain) < 3: - return False # The maximum length of an email address that can be handled by SMTP # is 254 characters. diff --git a/plone/schema/tests/test_email.py b/plone/schema/tests/test_email.py index 7f1ce0c..3f500fc 100644 --- a/plone/schema/tests/test_email.py +++ b/plone/schema/tests/test_email.py @@ -64,6 +64,7 @@ def test_validation(self): self.assertFalse(_isemail("arthur\t@example.org")) self.assertFalse(_isemail("arthur@one")) self.assertFalse(_isemail("arthur@example@org")) + self.assertFalse(_isemail("me@.me")) self.assertFalse(_isemail("x" * 254 + "@too.long")) # Explicitly test some examples from the docstring, @@ -76,7 +77,7 @@ def test_validation(self): self.assertTrue(_isemail("jens@österreich.example.com")) # 3. allow ampersand in the user part - self.assertTrue(_isemail("q&a@.example.com")) + self.assertTrue(_isemail("q&a@example.com")) # 4. do not allows spaces. self.assertFalse(_isemail("test@aha.ok hello"))