Fix timezone handling for datetime to unixtime conversions (#2213)

* Fix timezone handling for datetime to unixtime conversions

datetime objects are supported to set expire, these can have timezones.
mktime was used to convert these to unixtime. mktime in Python however is not timezone aware, it expects the input to be UTC and redis-py did not convert the datetime timestamps to UTC before calling mktime.

This can lead to:
1) Setting incorrect expire times because the input datetime object has a timezone but is passed to mktime without converting to UTC first.
2) When the datetime timestamp is within DST, mktime fails with "OverflowError: mktime argument out of range" because UTC doesn't have DST. This depends on libc versions.

* linters

Co-authored-by: dvora-h <[email protected]>

Author: joekohlsdorf

CHANGES

@@ -1,3 +1,4 @@
+    * Fix timezone handling for datetime to unixtime conversions
     * Fix start_id type for XAUTOCLAIM
     * Remove verbose logging from cluster.py
     * Add retry mechanism to async version of Connection

redis/commands/core.py

@@ -2,7 +2,6 @@
 
 import datetime
 import hashlib
-import time
 import warnings
 from typing import (
     TYPE_CHECKING,
@@ -1674,7 +1673,7 @@ def expireat(
         For more information see https://redis.io/commands/expireat
         """
         if isinstance(when, datetime.datetime):
-            when = int(time.mktime(when.timetuple()))
+            when = int(when.timestamp())
 
         exp_option = list()
         if nx:
@@ -1769,14 +1768,12 @@ def getex(
         if exat is not None:
             pieces.append("EXAT")
             if isinstance(exat, datetime.datetime):
-                s = int(exat.microsecond / 1000000)
-                exat = int(time.mktime(exat.timetuple())) + s
+                exat = int(exat.timestamp())
             pieces.append(exat)
         if pxat is not None:
             pieces.append("PXAT")
             if isinstance(pxat, datetime.datetime):
-                ms = int(pxat.microsecond / 1000)
-                pxat = int(time.mktime(pxat.timetuple())) * 1000 + ms
+                pxat = int(pxat.timestamp() * 1000)
             pieces.append(pxat)
         if persist:
             pieces.append("PERSIST")
@@ -1995,8 +1992,7 @@ def pexpireat(
         For more information see https://redis.io/commands/pexpireat
         """
         if isinstance(when, datetime.datetime):
-            ms = int(when.microsecond / 1000)
-            when = int(time.mktime(when.timetuple())) * 1000 + ms
+            when = int(when.timestamp() * 1000)
         exp_option = list()
         if nx:
             exp_option.append("NX")
@@ -2197,14 +2193,12 @@ def set(
         if exat is not None:
             pieces.append("EXAT")
             if isinstance(exat, datetime.datetime):
-                s = int(exat.microsecond / 1000000)
-                exat = int(time.mktime(exat.timetuple())) + s
+                exat = int(exat.timestamp())
             pieces.append(exat)
         if pxat is not None:
             pieces.append("PXAT")
             if isinstance(pxat, datetime.datetime):
-                ms = int(pxat.microsecond / 1000)
-                pxat = int(time.mktime(pxat.timetuple())) * 1000 + ms
+                pxat = int(pxat.timestamp() * 1000)
             pieces.append(pxat)
         if keepttl:
             pieces.append("KEEPTTL")

tests/test_asyncio/test_commands.py

@@ -4,7 +4,6 @@
 import binascii
 import datetime
 import re
-import time
 from string import ascii_letters
 
 import pytest
@@ -750,7 +749,7 @@ async def test_expireat_no_key(self, r: redis.Redis):
     async def test_expireat_unixtime(self, r: redis.Redis):
         expire_at = await redis_server_time(r) + datetime.timedelta(minutes=1)
         await r.set("a", "foo")
-        expire_at_seconds = int(time.mktime(expire_at.timetuple()))
+        expire_at_seconds = int(expire_at.timestamp())
         assert await r.expireat("a", expire_at_seconds)
         assert 0 < await r.ttl("a") <= 61
 
@@ -875,8 +874,8 @@ async def test_pexpireat_no_key(self, r: redis.Redis):
     async def test_pexpireat_unixtime(self, r: redis.Redis):
         expire_at = await redis_server_time(r) + datetime.timedelta(minutes=1)
         await r.set("a", "foo")
-        expire_at_seconds = int(time.mktime(expire_at.timetuple())) * 1000
-        assert await r.pexpireat("a", expire_at_seconds)
+        expire_at_milliseconds = int(expire_at.timestamp() * 1000)
+        assert await r.pexpireat("a", expire_at_milliseconds)
         assert 0 < await r.pttl("a") <= 61000
 
     @skip_if_server_version_lt("2.6.0")

tests/test_commands.py

@@ -1185,7 +1185,7 @@ def test_expireat_no_key(self, r):
     def test_expireat_unixtime(self, r):
         expire_at = redis_server_time(r) + datetime.timedelta(minutes=1)
         r["a"] = "foo"
-        expire_at_seconds = int(time.mktime(expire_at.timetuple()))
+        expire_at_seconds = int(expire_at.timestamp())
         assert r.expireat("a", expire_at_seconds) is True
         assert 0 < r.ttl("a") <= 61
 
@@ -1428,8 +1428,8 @@ def test_pexpireat_no_key(self, r):
     def test_pexpireat_unixtime(self, r):
         expire_at = redis_server_time(r) + datetime.timedelta(minutes=1)
         r["a"] = "foo"
-        expire_at_seconds = int(time.mktime(expire_at.timetuple())) * 1000
-        assert r.pexpireat("a", expire_at_seconds) is True
+        expire_at_milliseconds = int(expire_at.timestamp() * 1000)
+        assert r.pexpireat("a", expire_at_milliseconds) is True
         assert 0 < r.pttl("a") <= 61000
 
     @skip_if_server_version_lt("7.0.0")