From 2bf3127e2e950e40e40046331a569a503e74d8a1 Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Sat, 17 Jun 2017 10:45:39 +1000 Subject: [PATCH 1/6] bpo-30681: Change error handling to return None in case of invalid date --- Lib/email/utils.py | 18 +++++++++++++----- Lib/test/test_email/test_utils.py | 10 ++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Lib/email/utils.py b/Lib/email/utils.py index a759d23308d302..b694d2aa91156d 100644 --- a/Lib/email/utils.py +++ b/Lib/email/utils.py @@ -207,11 +207,19 @@ def make_msgid(idstring=None, domain=None): def parsedate_to_datetime(data): - *dtuple, tz = _parsedate_tz(data) - if tz is None: - return datetime.datetime(*dtuple[:6]) - return datetime.datetime(*dtuple[:6], - tzinfo=datetime.timezone(datetime.timedelta(seconds=tz))) + try: + *dtuple, tz = _parsedate_tz(data) + except TypeError: + # _parsedate_tz(data) returned None due to failure to parse + return None + try: + if tz is None: + return datetime.datetime(*dtuple[:6]) + return datetime.datetime(*dtuple[:6], + tzinfo=datetime.timezone(datetime.timedelta(seconds=tz))) + except ValueError: + # Date parsed ok, but one or more component values are invalid + return None def parseaddr(addr): diff --git a/Lib/test/test_email/test_utils.py b/Lib/test/test_email/test_utils.py index 6dcb3bbe7aab8c..1047090db67fda 100644 --- a/Lib/test/test_email/test_utils.py +++ b/Lib/test/test_email/test_utils.py @@ -48,6 +48,16 @@ def test_parsedate_to_datetime_naive(self): utils.parsedate_to_datetime(self.datestring + ' -0000'), self.naive_dt) + def test_parsedate_to_datetime_invalid_string(self): + self.assertIsNone( + utils.parsedate_to_datetime('') + ) + + def test_parsedate_to_datetime_invalid_date(self): + self.assertIsNone( + utils.parsedate_to_datetime('Tue, 06 Jun 2017 27:39:33 +0600') + ) + class LocaltimeTests(unittest.TestCase): From e0b616abe303d3e0d2800e22e6d2cbeb34cf557f Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Sat, 17 Jun 2017 11:29:11 +1000 Subject: [PATCH 2/6] bpo-30681: Handle invalid date in Date header --- Lib/email/headerregistry.py | 6 ++++++ Lib/test/test_email/test_headerregistry.py | 16 ++++++++++++++++ Lib/test/test_email/test_inversion.py | 8 ++++++++ 3 files changed, 30 insertions(+) diff --git a/Lib/email/headerregistry.py b/Lib/email/headerregistry.py index 81fee146dcc357..38f301db20f07e 100644 --- a/Lib/email/headerregistry.py +++ b/Lib/email/headerregistry.py @@ -300,7 +300,13 @@ def parse(cls, value, kwds): kwds['parse_tree'] = parser.TokenList() return if isinstance(value, str): + kwds['decoded'] = value value = utils.parsedate_to_datetime(value) + if value is None: + kwds['defects'].append(errors.InvalidHeaderDefect('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']) diff --git a/Lib/test/test_email/test_headerregistry.py b/Lib/test/test_email/test_headerregistry.py index af836dc9726622..664bce73ea68b1 100644 --- a/Lib/test/test_email/test_headerregistry.py +++ b/Lib/test/test_email/test_headerregistry.py @@ -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.InvalidHeaderDefect) + + 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.InvalidHeaderDefect) + def test_datetime_read_only(self): h = self.make_header('date', self.datestring) with self.assertRaises(AttributeError): diff --git a/Lib/test/test_email/test_inversion.py b/Lib/test/test_email/test_inversion.py index 8e8d67641b8943..7bd7f2a72067ad 100644 --- a/Lib/test/test_email/test_inversion.py +++ b/Lib/test/test_email/test_inversion.py @@ -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: abc@xyz.com + Subject: timezones + + How do they work even? + """),), + } payload_params = { From c59e0bf1a10ffef706d8229f05a8f0ffd9d53fbb Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Sat, 17 Jun 2017 17:39:00 +1000 Subject: [PATCH 3/6] bpo-30681: Add InvalidDateDefect --- Lib/email/errors.py | 3 +++ Lib/email/headerregistry.py | 2 +- Lib/test/test_email/test_headerregistry.py | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/email/errors.py b/Lib/email/errors.py index 791239fa6a54cc..a40e64ce280be7 100644 --- a/Lib/email/errors.py +++ b/Lib/email/errors.py @@ -105,3 +105,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): + """Header has unparseable or invalid date""" diff --git a/Lib/email/headerregistry.py b/Lib/email/headerregistry.py index 38f301db20f07e..d0aa4a3375bbbd 100644 --- a/Lib/email/headerregistry.py +++ b/Lib/email/headerregistry.py @@ -303,7 +303,7 @@ def parse(cls, value, kwds): kwds['decoded'] = value value = utils.parsedate_to_datetime(value) if value is None: - kwds['defects'].append(errors.InvalidHeaderDefect('Invalid date value or format')) + kwds['defects'].append(errors.InvalidDateDefect('Invalid date value or format')) kwds['datetime'] = None kwds['parse_tree'] = parser.TokenList() return diff --git a/Lib/test/test_email/test_headerregistry.py b/Lib/test/test_email/test_headerregistry.py index 664bce73ea68b1..975a734ba5bedb 100644 --- a/Lib/test/test_email/test_headerregistry.py +++ b/Lib/test/test_email/test_headerregistry.py @@ -209,7 +209,7 @@ def test_invalid_date_format(self): self.assertEqual(h, s) self.assertIsNone(h.datetime) self.assertEqual(len(h.defects), 1) - self.assertIsInstance(h.defects[0], errors.InvalidHeaderDefect) + self.assertIsInstance(h.defects[0], errors.InvalidDateDefect) def test_invalid_date_value(self): s = 'Tue, 06 Jun 2017 27:39:33 +0600' @@ -217,7 +217,7 @@ def test_invalid_date_value(self): self.assertEqual(h, s) self.assertIsNone(h.datetime) self.assertEqual(len(h.defects), 1) - self.assertIsInstance(h.defects[0], errors.InvalidHeaderDefect) + self.assertIsInstance(h.defects[0], errors.InvalidDateDefect) def test_datetime_read_only(self): h = self.make_header('date', self.datestring) From 5c6cfafe4e60f61f9f1801b492a14cd2c4c45bb1 Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Sat, 17 Jun 2017 10:45:39 +1000 Subject: [PATCH 4/6] Revert "bpo-30681: Change error handling to return None in case of invalid date" This reverts commit 2bf3127e2e950e40e40046331a569a503e74d8a1. --- Lib/email/utils.py | 18 +++++------------- Lib/test/test_email/test_utils.py | 10 ---------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/Lib/email/utils.py b/Lib/email/utils.py index b694d2aa91156d..a759d23308d302 100644 --- a/Lib/email/utils.py +++ b/Lib/email/utils.py @@ -207,19 +207,11 @@ def make_msgid(idstring=None, domain=None): def parsedate_to_datetime(data): - try: - *dtuple, tz = _parsedate_tz(data) - except TypeError: - # _parsedate_tz(data) returned None due to failure to parse - return None - try: - if tz is None: - return datetime.datetime(*dtuple[:6]) - return datetime.datetime(*dtuple[:6], - tzinfo=datetime.timezone(datetime.timedelta(seconds=tz))) - except ValueError: - # Date parsed ok, but one or more component values are invalid - return None + *dtuple, tz = _parsedate_tz(data) + if tz is None: + return datetime.datetime(*dtuple[:6]) + return datetime.datetime(*dtuple[:6], + tzinfo=datetime.timezone(datetime.timedelta(seconds=tz))) def parseaddr(addr): diff --git a/Lib/test/test_email/test_utils.py b/Lib/test/test_email/test_utils.py index 1047090db67fda..6dcb3bbe7aab8c 100644 --- a/Lib/test/test_email/test_utils.py +++ b/Lib/test/test_email/test_utils.py @@ -48,16 +48,6 @@ def test_parsedate_to_datetime_naive(self): utils.parsedate_to_datetime(self.datestring + ' -0000'), self.naive_dt) - def test_parsedate_to_datetime_invalid_string(self): - self.assertIsNone( - utils.parsedate_to_datetime('') - ) - - def test_parsedate_to_datetime_invalid_date(self): - self.assertIsNone( - utils.parsedate_to_datetime('Tue, 06 Jun 2017 27:39:33 +0600') - ) - class LocaltimeTests(unittest.TestCase): From 8c8ac38a04fb810d5fbabf988feb590793adebbc Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Thu, 29 Nov 2018 09:59:44 +1100 Subject: [PATCH 5/6] bpo-30681: Handle invalid date in Date header --- Lib/email/headerregistry.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/email/headerregistry.py b/Lib/email/headerregistry.py index d0aa4a3375bbbd..47364560b91ae6 100644 --- a/Lib/email/headerregistry.py +++ b/Lib/email/headerregistry.py @@ -301,8 +301,9 @@ def parse(cls, value, kwds): return if isinstance(value, str): kwds['decoded'] = value - value = utils.parsedate_to_datetime(value) - if value is None: + 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() From 96f82236397be9d0d3f62ef9b8c51e2d41feafe9 Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Thu, 29 Nov 2018 10:01:29 +1100 Subject: [PATCH 6/6] bpo-30681: Document failure modes of parsedate_to_datetime and add tests --- Doc/library/email.util.rst | 5 ++++- Lib/test/test_email/test_utils.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Doc/library/email.util.rst b/Doc/library/email.util.rst index 63fae2ab84e213..63e2ca1bb0e017 100644 --- a/Doc/library/email.util.rst +++ b/Doc/library/email.util.rst @@ -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 diff --git a/Lib/test/test_email/test_utils.py b/Lib/test/test_email/test_utils.py index 6dcb3bbe7aab8c..60f175a05d5437 100644 --- a/Lib/test/test_email/test_utils.py +++ b/Lib/test/test_email/test_utils.py @@ -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): + 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):