From 3b1a4981641ce993ac443f9be78fc0e17e20b03b Mon Sep 17 00:00:00 2001 From: joekohlsdorf Date: Fri, 3 Jun 2022 10:29:38 -0400 Subject: [PATCH 1/2] 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. --- CHANGES | 2 +- redis/commands/core.py | 18 ++++++------------ tests/test_asyncio/test_commands.py | 7 +++---- tests/test_commands.py | 6 +++--- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/CHANGES b/CHANGES index b7e238ebb3..fca0d73e18 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,4 @@ - + * Fix timezone handling for datetime to unixtime conversions * Allow negative `retries` for `Retry` class to retry forever * Add `items` parameter to `hset` signature * Create codeql-analysis.yml (#1988). Thanks @chayim diff --git a/redis/commands/core.py b/redis/commands/core.py index a337d4f314..4c7f8041e1 100644 --- a/redis/commands/core.py +++ b/redis/commands/core.py @@ -2,7 +2,6 @@ import datetime import hashlib -import time import warnings from typing import ( TYPE_CHECKING, @@ -1667,7 +1666,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: @@ -1762,14 +1761,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") @@ -1988,8 +1985,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") @@ -2190,14 +2186,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") diff --git a/tests/test_asyncio/test_commands.py b/tests/test_asyncio/test_commands.py index e128ac40b8..42f2526c21 100644 --- a/tests/test_asyncio/test_commands.py +++ b/tests/test_asyncio/test_commands.py @@ -5,7 +5,6 @@ import datetime import re import sys -import time from string import ascii_letters import pytest @@ -805,7 +804,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 @@ -930,8 +929,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") diff --git a/tests/test_commands.py b/tests/test_commands.py index fcd2ed1c0c..15a05c7710 100644 --- a/tests/test_commands.py +++ b/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") From 2a448cbf0bb7d3f9de941c36878f33237cd9ae59 Mon Sep 17 00:00:00 2001 From: dvora-h <67596500+dvora-h@users.noreply.github.com> Date: Tue, 2 Aug 2022 13:23:47 +0300 Subject: [PATCH 2/2] linters --- tests/test_asyncio/test_commands.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_asyncio/test_commands.py b/tests/test_asyncio/test_commands.py index 8c81ed1239..4c25277616 100644 --- a/tests/test_asyncio/test_commands.py +++ b/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