Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed a regular expression denial of service issue by limiting whites… #7360

Merged
merged 5 commits into from Sep 20, 2023
+16 −3
Merged

Fixed a regular expression denial of service issue by limiting whites… #7360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
031ea9a
Fixed a regular expression denial of service issue by limiting whites…
prodigysml Sep 7, 2023
b27b96c
improved a bit, but not perfect
prodigysml Sep 7, 2023
29f2a69
further improved. Should be fully patched
prodigysml Sep 7, 2023
64cd826
Guard against length itself
adriangb Sep 20, 2023
bef9776
Fix type hints
adriangb Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions pydantic/networks.py
View file
Open in desktop
Expand Up @@ -416,16 +416,21 @@ def _validate(cls, __input_value: NetworkType, _: core_schema.ValidationInfo) ->
return cls(__input_value) # type: ignore[return-value]


def _build_pretty_email_regex() -> re.Pattern:
def _build_pretty_email_regex() -> re.Pattern[str]:
name_chars = r'[\w!#$%&\'*+\-/=?^_`{|}~]'
unquoted_name_group = fr'((?:{name_chars}+\s+)*{name_chars}+)'
quoted_name_group = r'"((?:[^"]|\")+)"'
email_group = r'<\s*(.+)\s*>'
email_group = r'<\s*(.{0,254})\s*>'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has changed the intent of the regex - previously it only matched one or more characters, now it matches 0 characters. Admittedly the previous regex would match a space alone if the address was < > and this will not, but it is a change beyond the statement in the pull request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for anyone coming to this in future, in #7599 (before releasing this PR) we reverted this change so the only actual change implemented is the length check which covers the original minor performance risk.

return re.compile(rf'\s*(?:{unquoted_name_group}|{quoted_name_group})?\s*{email_group}\s*')


pretty_email_regex = _build_pretty_email_regex()

MAX_EMAIL_LENGTH = 2048
"""Maximum length for an email.
A somewhat arbitrary but very generous number compared to what is allowed by most implementations.
"""


def validate_email(value: str) -> tuple[str, str]:
"""Email address validation using [email-validator](https://pypi.org/project/email-validator/).
Expand All @@ -440,6 +445,13 @@ def validate_email(value: str) -> tuple[str, str]:
if email_validator is None:
import_email_validator()

if len(value) > MAX_EMAIL_LENGTH:
raise PydanticCustomError(
'value_error',
'value is not a valid email address: {reason}',
{'reason': f'Length must not exceed {MAX_EMAIL_LENGTH} characters'},
)

m = pretty_email_regex.fullmatch(value)
name: str | None = None
if m:
Expand Down
3 changes: 2 additions & 1 deletion tests/test_networks.py
View file
Open in desktop
Expand Up @@ -812,9 +812,10 @@ def test_address_valid(value, name, email):
('foobar <<foobar<@example.com>', None),
('foobar <>', None),
('first.last <first.last@example.com>', None),
pytest.param('foobar <' + 'a' * 4096 + '@example.com>', 'Length must not exceed 2048 characters', id='long'),
],
)
def test_address_invalid(value, reason):
def test_address_invalid(value: str, reason: Union[str, None]):
with pytest.raises(PydanticCustomError, match=f'value is not a valid email address: {reason or ""}'):
validate_email(value)

Expand Down