Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#28209 closed Bug (fixed)

Date-based generic views can fail with ValueError: year is out of range

Reported by: Tim Graham Owned by: Adit Biswas
Component: Generic views Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Test to reproduce:

  • tests/generic_views/test_dates.py

    diff --git a/tests/generic_views/test_dates.py b/tests/generic_views/test_dates.
    index bf462b4..76d4b2b 100644
    a b class DateDetailViewTests(TestDataMixin, TestCase): 
    661661        self.assertEqual(res.context['book'], b)
    662662        self.assertTemplateUsed(res, 'generic_views/book_detail.html')
    663663
     664    def test_year_out_of_range(self):
     665        self.client.get('/dates/books/9999/')
     666
    664667    def test_invalid_url(self):
    665668        with self.assertRaises(AttributeError):
    666669            self.client.get("/dates/books/2008/oct/01/nopk/")

Traceback:

  File "/home/tim/code/django/django/views/generic/dates.py", line 58, in _get_next_year
    return date.replace(year=date.year + 1, month=1, day=1)
ValueError: year is out of range

I think returning a 404 response is the correct thing to do. Other mixins besides YearMixin may be affected.

Change History (10)

comment:1 Changed 4 months ago by Adit Biswas

Has patch: set

added a PR for this PR

Last edited 4 months ago by Adit Biswas (previous) (diff)

comment:3 Changed 4 months ago by Adit Biswas

updated MonthMixin aswell which will throw the same error when crossing year 9999

comment:4 Changed 4 months ago by Adit Biswas

Owner: changed from nobody to Adit Biswas
Status: newassigned

comment:5 Changed 4 months ago by Adit Biswas

Sorry, I'm not sure if I've done enough. Waiting for some discussion around this before I continue making changes.
basically datetime will throw value errors whenever the year crosses 9999.
Wherever a datetime is manipulated to the date to cross year 10000 will be affected.

YearMixin._get_next_year(self, date)
MonthMixin._get_next_month(self, date)
DayMixin._get_next_day(self, date)
WeekMixin._get_next_week(self, date)
DateMixin._make_date_lookup_arg(self, value)

While 404 would be an appropriate response for dates crossing the Year 9999
shouldnt the dates between 01-01-9999 and 31-12-9999 be handled appropriately
if I throw 404 for each mixin the behaviour will be slightly odd. since
/dates/books/9999/ will throw a 404 but /dates/books/9999/01 will not

In case of DateMixin._make_date_lookup_arg method, the datetime object does not actually throw an error, but it behaves in an unexpected way and cycles to a different date altogether
eg. /dates/books/10000/ causes the date argument to become the 2008 but the view will not return a 500.

Last edited 4 months ago by Adit Biswas (previous) (diff)

comment:6 Changed 4 months ago by Adit Biswas

Patch needs improvement: set

comment:7 Changed 4 months ago by Tim Graham

I think preventing a crash is of main importance at this point. Django will probably be obsolete before handling dates with year 9999 is a practical problem.

comment:8 Changed 4 months ago by Adit Biswas

added a PR for this PR

comment:9 Changed 4 months ago by Adit Biswas

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:10 Changed 4 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In c2eea61d:

Fixed #28209 -- Made date-based generic views return a 404 rather than crash when given an out of range date.

comment:11 Changed 4 months ago by Tim Graham <timograham@…>

In 4e67599:

[1.11.x] Fixed #28209 -- Made date-based generic views return a 404 rather than crash when given an out of range date.

Backport of c2eea61dff44b16caab04928d582b856c2b8b615 from master

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