Opened 14 months ago

Last modified 3 days ago

#28690 assigned Bug

django.utils.http.parse_http_date two digit year check is incorrect

Reported by: Mads Jensen Owned by: Tameesh Biswas
Component: Utilities Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

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.

The < 70 is incorrect, and should have been < 50. I inserted a diff that applies.

Attachments (1)

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

Download all attachments as: .zip

Change History (17)

Changed 14 months ago by Mads Jensen

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

comment:1 Changed 14 months 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 14 months ago by Alexander Vyushkov

Owner: changed from nobody to Alexander Vyushkov
Status: newassigned

comment:3 Changed 14 months 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 14 months ago by Levi Payne

Triage Stage: AcceptedReady for checkin

comment:5 Changed 14 months ago by Levi Payne

Triage Stage: Ready for checkinAccepted

Still some suggested edits on the PR.

comment:6 Changed 14 months ago by Tim Graham

Patch needs improvement: set

comment:7 Changed 11 months 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 11 months ago by Tim Graham

Patch needs improvement: set

comment:9 Changed 11 months ago by Alexander Vyushkov

Patch needs improvement: unset

sent new pull request

comment:10 Changed 10 months 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 2 months 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 2 months ago by Tameesh Biswas (previous) (diff)

comment:12 Changed 2 months 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 2 months 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 2 months ago by Tameesh Biswas (previous) (diff)

comment:14 Changed 6 weeks ago by Vishvajit Pathak

Tameesh Biswas

Are you working on this ?

comment:15 Changed 6 weeks ago by Tameesh Biswas

Yes, I am.

comment:16 Changed 3 days 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?

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