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 16 commits into
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.
|
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. |
|
Hi @bitdancer could you please review? |
bitdancer
left a comment
bitdancer
left a comment
There was a problem hiding this comment.
Sorry about the long wait for a re-review. I'll do my best to review new changes more promptly.
|
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 |
gostak-dd
requested review from
a team,
ezio-melotti,
hugovk,
itamaro,
sethmlarson and
webknjaz
as code owners
gostak-dd
force-pushed
the
bugfix/parsedate_to_datetime
branch
from
a975f26 to
9a40eed
Compare
|
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. |
|
Well, it looks like I screwed things up by trying to use github's web UI to resolve the merge conflict. Can you straighten it out? The comments we added were correct, the conflict was with a change that added 'RFC 5233' to the same comment we changed, but I incorporated that change my suggestion that you applied. |
| # The year is between 1969 and 1999 (inclusive). | ||
| if yy > 68: | ||
| yy += 1900 | ||
| # The year is between 2000 and 2068 (inclusive). |
There was a problem hiding this comment.
Why removed this comment? It was accurate.
| @@ -0,0 +1,7 @@ | |||
| Fixed the :mod:`email` module parsing of three digit dates to | |||
There was a problem hiding this comment.
The only change is removing a comment and adding a test. I do not think this NEWS entry is still relevant.
There was a problem hiding this comment.
No, the PR got messed up by my using the web to try to resolve the merge conflict, It's current state does not represent its intent, and all should be clear once it gets straightened out.
| "Mon, 1 Jan 69 00:00:00 +0000": "1969", | ||
| "Mon, 1 Jan 99 00:00:00 +0000": "1999", | ||
| # 3-digit year boundary | ||
| "Mon, 1 Jan 999 00:00:00 +0000": "2899", |
There was a problem hiding this comment.
Why the result is not 999?
There was a problem hiding this comment.
Because the RFC says it should be interpreted by adding 1900, which is what the PR is going to fix.
|
Well, I'm not enough of a git expert to sort this out in the existing branch, so I built a new PR, #150864, by cherry picking commits from this one. Please verify that I got it right, and I'll merge that one. I'm going to avoid that web conflict resolution like the plague in the future. |
|
Closing this in favor of #150864, since I broke this one :( |
4 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