Opened 3 months ago

Closed 3 months ago

#35683 closed Bug (wontfix)

django.utils.timezone.make_naive can underflow for timezones close to datetime.min

Reported by: Liam DeVoe Owned by: Shubham Singh Sugara
Component: Database layer (models, ORM) Version: 5.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Liam DeVoe)

I'm a contributor over at Hypothesis. We found that saving (or retrieving?) models with aware DateTimeField values close to datetime.min can error if:

  • timezone support is enabled (USE_TZ=True)
  • you are using the sqlite database (presumably also any other db backend without tz aware support, if they exist?)

Explicitly:

from datetime import datetime, timezone
from zoneinfo import ZoneInfo
from django.utils.timezone import make_naive

dt = datetime.min.replace(tzinfo=ZoneInfo(key='Africa/Addis_Ababa'))
make_naive(dt, timezone.utc) # OverflowError

where make_naive is called from here. I would guess the overflow case for datetime.max is symmetric and similar as well.

I hope you'll forgive the lack of a django reproducer - I live in testing land and don't have a local django environment handy :). Hopefully the cause is clear and reproducible with vanilla django models.

Change History (8)

comment:1 by Liam DeVoe, 3 months ago

Description: modified (diff)

comment:2 by Sarah Boyce, 3 months ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted

Replicated, thank you!

Should be an issue in MySQL and Oracle as well - refs #23820
Rough testcase:

  • tests/backends/sqlite/test_operations.py

    a b  
    11import unittest
     2from datetime import datetime
    23
    34from django.core.management.color import no_style
    45from django.db import connection
    5 from django.test import TestCase
     6from django.test import TestCase, override_settings
     7from django.utils import timezone
    68
    79from ..models import Person, Tag
    810
    class SQLiteOperationsTests(TestCase):  
    8688            "zzz'",
    8789            statements[-1],
    8890        )
     91
     92    @override_settings(USE_TZ=True, TIME_ZONE="UTC")
     93    def test_adapt_datetimefield_value_close_to_datetime_min(self):
     94        africa_nairobi_tz = timezone.get_fixed_timezone(180)
     95        value = datetime.min.replace(tzinfo=africa_nairobi_tz)
     96
     97        connection.ops.adapt_datetimefield_value(value)  # OverflowError

comment:3 by Shubham Singh Sugara, 3 months ago

Owner: set to Shubham Singh Sugara
Status: newassigned

will try to work on it but will have to check first
Bug is causing issue on other db as well if so look for fix

comment:4 by Tim Graham, 3 months ago

What behavior do you expect? If the timezone conversion causes the datetime to be too small or too big, what can be done?

in reply to:  4 comment:5 by Shubham Singh Sugara, 3 months ago

Replying to Tim Graham:

What behavior do you expect? If the timezone conversion causes the datetime to be too small or too big, what can be done?

For now i can think of this solution
Max case should be also included if datetime.min is giving error than there is a possibility datetime.max also giving the error due to corner cases

We can Update make_naive Function:

Modify the make_naive function in Django to handle boundary cases(max and min). For instance, if an OverflowError is detected, the function could return the closest safe datetime value or handle the error gracefully in another way.

for now i think this can work across all db as changing other parts of modules will be db specific
but if any other suggestion open to discussion

comment:6 by Tim Graham, 3 months ago

The use case for storing datetime.min and max is unclear, but it seems doubtful that we should silently change the value the user provided. I would close this ticket as "needs info" or "wontfix" unless the reporter can provide more explanation.

comment:7 by Liam DeVoe, 3 months ago

I half expected this ticket to be wontfix when I opened it :). Here's one sane alternative I can think of: if you're converting to naive solely for serializing to the db, and when you deserialize from the db you make it tz aware again, then the only time it's out of bounds for the serialization/storage portion. But you control the serialization format, so you could extend the limits by 24 hours in each direction to accommodate those overflows, relying on the invariant that they will be made immediately tz aware and thus in bounds before reaching the user.

I'm not familiar with django internals, so this may not be acceptable or the assumed invariants may not be correct - that's up to you guys. I agree that silently capping is not a good idea. Possibly this should just be wontifx'd but documented; it does seem surprising to a user that I cannot store an in-bounds tz aware datetime, because there is no reason to expect that django will convert it to a naive datetime.

Last edited 3 months ago by Liam DeVoe (previous) (diff)

comment:8 by Sarah Boyce, 3 months ago

Resolution: wontfix
Status: assignedclosed

Thank you both for the discussion!
Given I believe this was found and validated via testing scenarios (rather than "in real life"), and much of this code has been this way for years, going to update to wontfix.
If anyone finds that this causes them an issue with their application, they're welcome to add this discussion.

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