gh-126845: Some edge cases in email.utils.parsedate_to_datetime seem to differ from RFC2822 spec #134438
Conversation
| @@ -149,7 +149,7 @@ def _parsedate_tz(data): | |||
| # calls for a two-digit yy, but RFC 2822 (which obsoletes RFC 822) | |||
| # mandates a 4-digit yy. For more information, see the documentation for | |||
| # the time module. | |||
| if yy < 100: | |||
| if len(str(yy)) >= 2 and yy < 100: | |||
There was a problem hiding this comment.
yy is now a number, so if yy < 100 (and it's positive), len(str(yy)) >= 2 is never true.
You need to keep the original year string for this check.
| @@ -3227,7 +3227,7 @@ def test_parsedate_y2k(self): | |||
|
|
|||
| """ | |||
| self.assertEqual(utils.parsedate_tz('25 Feb 03 13:47:26 -0800'), | |||
| utils.parsedate_tz('25 Feb 2003 13:47:26 -0800')) | |||
| utils.parsedate_tz('25 Feb 3 13:47:26 -0800')) | |||
There was a problem hiding this comment.
This makes the test ineffective.
| import unittest | ||
| from email.utils import parsedate_to_datetime | ||
|
|
||
| class ParsedateToDatetimeTest(unittest.TestCase): |
There was a problem hiding this comment.
parsedate_to_datetime tests belong in test_utils in DateTimeTests. (Those tests could maybe use some refactoring, but let's ignore that ;)
| def test(self): | ||
| expectations = { | ||
| "Sat, 15 Aug 0001 23:12:09 +0500": "0001", | ||
| "Thu, 1 Sep 1 23:12:09 +0800": "0001", |
There was a problem hiding this comment.
Logically when doing a Postel "be generous in what you accept" recovery here, a single digit year should be treated as if it were a two digit year, which means the result here should 2001, which is what the current code produces.
| expectations = { | ||
| "Sat, 15 Aug 0001 23:12:09 +0500": "0001", | ||
| "Thu, 1 Sep 1 23:12:09 +0800": "0001", | ||
| "Thu, 7 Oct 123 23:12:09 +0500": "0123", |
There was a problem hiding this comment.
And this result should be 2023, per the rfc text you quoted (three digit years get 1900 added). I have no idea why that settled on that logic, it makes no sense to me, but that's what the RFC says.
| "Thu, 1 Sep 1 23:12:09 +0800": "0001", | ||
| "Thu, 7 Oct 123 23:12:09 +0500": "0123", | ||
| } | ||
| for input_string, output_string in expectations.items(): |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Continuation of PR #134350 which was mistakenly closed due to deleted local fork.
Regarding no.1 from #126845
Also added tests for different digit years and updated a test in test_email.py to correctly assert 1 digit years (03 is evaluated to 3)
email.utils.parsedate_to_datetimeseem to differ from RFC2822 spec #126845