gh-126845: Some edge cases in email.utils.parsedate_to_datetime seem to differ from RFC2822 spec#134438
gh-126845: Some edge cases in email.utils.parsedate_to_datetime seem to differ from RFC2822 spec#134438gostak-dd wants to merge 10 commits intopython:mainfrom
Conversation
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
This parses 0001 correctly, but breaks parsing of real two-digit years.
| # 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.
There was a problem hiding this comment.
I was able to remove this check following what @bitdancer said below
| """ | ||
| 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 |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @bitdancer: please review the changes made to this pull request. |
|
This PR is stale because it has been open for 30 days with no activity. |
3 participants
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