#28690 closed Bug (fixed)
django.utils.http.parse_http_date two digit year check is incorrect
Reported by: | Mads Jensen | Owned by: | Ad Timmering |
---|---|---|---|
Component: | Utilities | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Ad Timmering | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
RFC 850 does not mention this, but in RFC 7231 (and there's something similar in RFC 2822), there's the following quote:
Recipients of a timestamp value in rfc850-date format, which uses a
two-digit year, MUST interpret a timestamp that appears to be more
than 50 years in the future as representing the most recent year in
the past that had the same last two digits.
Current logic is hard coded to consider 0-69 to be in 2000-2069, and 70-99 to be 1970-1999, instead of comparing versus the current year.
Attachments (1)
Change History (29)
Changed 6 years ago by
Attachment: | parse-http-date-year.patch added |
---|
comment:1 Changed 6 years ago by
Component: | Uncategorized → Utilities |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 Changed 6 years ago by
Owner: | changed from nobody to Alexander Vyushkov |
---|---|
Status: | new → assigned |
comment:3 Changed 6 years ago by
Has patch: | set |
---|
Created a pull request: Created a pull request: https://github.com/django/django/pull/9214
comment:4 Changed 6 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 Changed 6 years ago by
Triage Stage: | Ready for checkin → Accepted |
---|
Still some suggested edits on the PR.
comment:6 Changed 6 years ago by
Patch needs improvement: | set |
---|
comment:7 Changed 6 years ago by
Patch needs improvement: | unset |
---|
I added regression test that fails with old code (test_parsing_rfc850_year_69), updated commit message to hopefully follow the guidelines, and added additional comments about the change. Squashed commits as well.
Could you review the pull request again?
comment:8 Changed 6 years ago by
Patch needs improvement: | set |
---|
comment:10 Changed 6 years ago by
Patch needs improvement: | set |
---|
This is awaiting for changes from Tim's feedback on PR.
(Please uncheck "Patch needs improvement" again when that's done. 🙂)
comment:11 Changed 5 years ago by
As this issue hasn't received any updates in the last 8 months, may I work on this ticket?
comment:12 Changed 5 years ago by
Owner: | Alexander Vyushkov deleted |
---|---|
Status: | assigned → new |
Go for it, I don't think I will have time to finish it.
comment:13 Changed 5 years ago by
Owner: | set to Tameesh Biswas |
---|---|
Status: | new → assigned |
Thanks, I'll pick up from where you left off in the PR and make the recommended changes on a new PR.
comment:16 Changed 5 years ago by
I've just picked up from the previous PR and opened a new PR here: https://github.com/django/django/pull/10749
It adds regression tests in the first commit that pass without applying the fix and adds the fix with another test-case that only passes with the fix applied.
Could you please review the changes?
comment:17 Changed 5 years ago by
Tameesh, I left a comment on the PR regarding the use of non-UTC today.
comment:18 Changed 5 years ago by
Owner: | changed from Tameesh Biswas to David Jovanović |
---|
As an issue haven't received an update for 4 months, I'm taking it over (djangocon europe 2019 sprint day 1).
comment:20 Changed 5 years ago by
Patch needs improvement: | unset |
---|
comment:21 Changed 5 years ago by
I think an earlier comment by Simon Charette (about using a fixed year in the tests) still applies to the new PR; I've added it.
comment:22 Changed 5 years ago by
Patch needs improvement: | set |
---|
comment:23 Changed 4 years ago by
Cc: | Ad Timmering added |
---|---|
Description: | modified (diff) |
Owner: | changed from David Jovanović to Ad Timmering |
Taking the liberty to reassign due to inactivity (6 months) and adding a pull request with revised code and addressing feedback on prior PRs. Please add give your comments for any concerns:)
comment:24 Changed 4 years ago by
Patch needs improvement: | unset |
---|
Accepted, however I don't think your patch is correct. The check should be relative to the current year, if I read the RFC quote correctly.