Skip to content

bpo-30681: Support invalid date format or value in email Date header #10783

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.

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Doc/library/email.utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ of the new API.
.. function:: parsedate_to_datetime(date)

The inverse of :func:`format_datetime`. Performs the same function as
:func:`parsedate`, but on success returns a :mod:`~datetime.datetime`. If
:func:`parsedate`, but on success returns a :mod:`~datetime.datetime`;
otherwise ``None`` may be returned if parsing fails, or a ``ValueError``
raised if *date* contains an invalid value such as an hour greater than
23 or a timezone offset not between -24 and 24 hours. If
the input date has a timezone of ``-0000``, the ``datetime`` will be a naive
``datetime``, and if the date is conforming to the RFCs it will represent a
time in UTC but with no indication of the actual source timezone of the
Expand Down
3 changes: 3 additions & 0 deletions Lib/email/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,6 @@ class NonASCIILocalPartDefect(HeaderDefect):
"""local_part contains non-ASCII characters"""
# This defect only occurs during unicode parsing, not when
# parsing messages decoded from binary.

class InvalidDateDefect(HeaderDefect):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also please add this to documentation under email.errors.rst ?

"""Header has unparseable or invalid date"""
9 changes: 8 additions & 1 deletion Lib/email/headerregistry.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,14 @@ def parse(cls, value, kwds):
kwds['parse_tree'] = parser.TokenList()
return
if isinstance(value, str):
value = utils.parsedate_to_datetime(value)
kwds['decoded'] = value
try:
value = utils.parsedate_to_datetime(value)
except (ValueError, TypeError):
kwds['defects'].append(errors.InvalidDateDefect('Invalid date value or format'))
kwds['datetime'] = None
kwds['parse_tree'] = parser.TokenList()
return
kwds['datetime'] = value
kwds['decoded'] = utils.format_datetime(kwds['datetime'])
kwds['parse_tree'] = cls.value_parser(kwds['decoded'])
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_email/test_headerregistry.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,22 @@ def test_no_value_is_defect(self):
self.assertEqual(len(h.defects), 1)
self.assertIsInstance(h.defects[0], errors.HeaderMissingRequiredValue)

def test_invalid_date_format(self):
s = 'Not a date header'
h = self.make_header('date', s)
self.assertEqual(h, s)
self.assertIsNone(h.datetime)
self.assertEqual(len(h.defects), 1)
self.assertIsInstance(h.defects[0], errors.InvalidDateDefect)

def test_invalid_date_value(self):
s = 'Tue, 06 Jun 2017 27:39:33 +0600'
h = self.make_header('date', s)
self.assertEqual(h, s)
self.assertIsNone(h.datetime)
self.assertEqual(len(h.defects), 1)
self.assertIsInstance(h.defects[0], errors.InvalidDateDefect)

def test_datetime_read_only(self):
h = self.make_header('date', self.datestring)
with self.assertRaises(AttributeError):
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_email/test_inversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ def msg_as_input(self, msg):
foo
"""),),

'header_with_invalid_date': (dedent(b"""\
Date: Tue, 06 Jun 2017 27:39:33 +0600
From: [email protected]
Subject: timezones

How do they work even?
"""),),

}

payload_params = {
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_email/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ def test_parsedate_to_datetime_naive(self):
utils.parsedate_to_datetime(self.datestring + ' -0000'),
self.naive_dt)

def test_parsedate_to_datetime_with_invalid_raises_typeerror(self):
with self.assertRaises(TypeError):
Copy link
Member

@pganssle pganssle Jun 11, 2019

Choose a reason for hiding this comment

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

I think this is fine as is, but I think it would be improved if you used a loop and subtests:

invalid_dates = ['', '0', 'A Complete Waste of Time']
for dtstr in invalid_dates:
    with self.subTest(dtstr=dtstr):
        with self.assertRaises(TypeError):
            utils.parsedate_to_datetime(dtstr)

Obviously that's pretty nested (though you can probably cut out one layer of nesting by using self.assertRaises(TypeError, utils.parsedate_to_datetime, dtstr) rather than the context manager - I still prefer the context manager, myself), so I suppose it's a matter of taste whether DRY and cleaner separation of test cases matters more than keeping code flat.

Same comment also applies to the test for ValueError below.

utils.parsedate_to_datetime('')
with self.assertRaises(TypeError):
utils.parsedate_to_datetime('0')
with self.assertRaises(TypeError):
utils.parsedate_to_datetime('A Complete Waste of Time')

def test_parsedate_to_datetime_with_invalid_raises_valueerror(self):
with self.assertRaises(ValueError):
utils.parsedate_to_datetime('Tue, 06 Jun 2017 27:39:33 +0600')
with self.assertRaises(ValueError):
utils.parsedate_to_datetime('Tue, 06 Jun 2017 07:39:33 +2600')
with self.assertRaises(ValueError):
utils.parsedate_to_datetime('Tue, 06 Jun 2017 27:39:33')


class LocaltimeTests(unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Handle exceptions caused by unparseable date headers when using email
"default" policy. Patch by Tim Bell.