Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#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 Ad Timmering)

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)

parse-http-date-year.patch (2.1 KB) - added by Mads Jensen 4 years ago.

Download all attachments as: .zip

Change History (29)

Changed 4 years ago by Mads Jensen

Attachment: parse-http-date-year.patch added

comment:1 Changed 4 years ago by Claude Paroz

Component: UncategorizedUtilities
Triage Stage: UnreviewedAccepted

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.

comment:2 Changed 4 years ago by Alexander Vyushkov

Owner: changed from nobody to Alexander Vyushkov
Status: newassigned

comment:3 Changed 4 years ago by Alexander Vyushkov

Has patch: set

Created a pull request: Created a pull request: https://github.com/django/django/pull/9214

comment:4 Changed 4 years ago by Levi Payne

Triage Stage: AcceptedReady for checkin

comment:5 Changed 4 years ago by Levi Payne

Triage Stage: Ready for checkinAccepted

Still some suggested edits on the PR.

comment:6 Changed 4 years ago by Tim Graham

Patch needs improvement: set

comment:7 Changed 4 years ago by Alexander Vyushkov

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 4 years ago by Tim Graham

Patch needs improvement: set

comment:9 Changed 4 years ago by Alexander Vyushkov

Patch needs improvement: unset

sent new pull request

comment:10 Changed 4 years ago by Carlton Gibson

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 3 years ago by Tameesh Biswas

As this issue hasn't received any updates in the last 8 months, may I work on this ticket?

Last edited 3 years ago by Tameesh Biswas (previous) (diff)

comment:12 Changed 3 years ago by Alexander Vyushkov

Owner: Alexander Vyushkov deleted
Status: assignednew

Go for it, I don't think I will have time to finish it.

comment:13 Changed 3 years ago by Tameesh Biswas

Owner: set to Tameesh Biswas
Status: newassigned

Thanks, I'll pick up from where you left off in the PR and make the recommended changes on a new PR.

Last edited 3 years ago by Tameesh Biswas (previous) (diff)

comment:14 Changed 3 years ago by Vishvajit Pathak

Tameesh Biswas

Are you working on this ?

comment:15 Changed 3 years ago by Tameesh Biswas

Yes, I am.

comment:16 Changed 3 years ago by Tameesh Biswas

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 3 years ago by Simon Charette

Tameesh, I left a comment on the PR regarding the use of non-UTC today.

comment:18 Changed 3 years ago by David Jovanović

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:19 Changed 3 years ago by David Jovanović

comment:20 Changed 3 years ago by David Jovanović

Patch needs improvement: unset

comment:21 Changed 3 years ago by Tim Bell

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 3 years ago by Tim Bell

Patch needs improvement: set

comment:23 Changed 2 years ago by Ad Timmering

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:)

PR => https://github.com/django/django/pull/11848

  • Year is now checked in relation to current year, rolling over to the past if more than 50 years in the future
  • Test now uses a patched version of datetime.datetime to pin to a specific year and have static test cases, addressing feedback from charettes@ on PR 10749 in Dec 2018.
Last edited 2 years ago by Ad Timmering (previous) (diff)

comment:24 Changed 2 years ago by Ad Timmering

Patch needs improvement: unset

comment:25 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 7cbd25a:

Refs #28690 -- Added more tests for parse_http_date().

comment:26 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 7b5f8ac:

Fixed #28690 -- Fixed handling of two-digit years in parse_http_date().

Due to RFC7231 ayear that appears to be more than 50 years in the
future are interpreted as representing the past.

comment:27 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In f38655e:

[3.0.x] Refs #28690 -- Added more tests for parse_http_date().

Backport of 7cbd25a06e820cbd1a0bfbc339fb7d9a737c54fa from master

comment:28 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 556d0c0:

[3.0.x] Fixed #28690 -- Fixed handling of two-digit years in parse_http_date().

Due to RFC7231 ayear that appears to be more than 50 years in the
future are interpreted as representing the past.

Backport of 7b5f8acb9e6395a1660dd7bfeb365866ca8ef47c from master

Note: See TracTickets for help on using tickets.
Back to Top