From 86d04682aea31cd596731a4b1707acfdacf47510 Mon Sep 17 00:00:00 2001 From: hodbn Date: Tue, 25 Aug 2020 12:40:13 -0700 Subject: [PATCH 1/5] Assume UTC timezone if none specified in Retry-After header --- src/urllib3/util/retry.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py index d5df6af3ea..786bdac50e 100644 --- a/src/urllib3/util/retry.py +++ b/src/urllib3/util/retry.py @@ -269,6 +269,12 @@ def parse_retry_after(self, retry_after): retry_date_tuple = email.utils.parsedate_tz(retry_after) if retry_date_tuple is None: raise InvalidHeader("Invalid Retry-After header: %s" % retry_after) + if retry_date_tuple[9] is None: # Python 2 + # Assume UTC if no timezone was specified + tmp = list(retry_date_tuple) + tmp[9] = 0 + retry_date_tuple = tuple(tmp) + retry_date = email.utils.mktime_tz(retry_date_tuple) seconds = retry_date - time.time() From 3404ed90991fa9490ba44ba87fcd82258ecdaf5f Mon Sep 17 00:00:00 2001 From: hodbn Date: Tue, 25 Aug 2020 13:04:28 -0700 Subject: [PATCH 2/5] Refactor and explain Retry-After behavior with no timezone --- src/urllib3/util/retry.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py index 786bdac50e..e5eda7a16d 100644 --- a/src/urllib3/util/retry.py +++ b/src/urllib3/util/retry.py @@ -271,9 +271,10 @@ def parse_retry_after(self, retry_after): raise InvalidHeader("Invalid Retry-After header: %s" % retry_after) if retry_date_tuple[9] is None: # Python 2 # Assume UTC if no timezone was specified - tmp = list(retry_date_tuple) - tmp[9] = 0 - retry_date_tuple = tuple(tmp) + # On Python2.7, parsedate_tz returns None for a timezone offset + # instead of 0 if no timezone is given, where mktime_tz treats + # a None timezone offset as local time. + retry_date_tuple = retry_date_tuple[:9] + (0,) + retry_date_tuple[10:] retry_date = email.utils.mktime_tz(retry_date_tuple) seconds = retry_date - time.time() From dabdeec361af94c523cc0a360bcbb30b74cd8eea Mon Sep 17 00:00:00 2001 From: hodbn Date: Wed, 26 Aug 2020 20:03:42 +0300 Subject: [PATCH 3/5] Test Retry-After support with multiple local timezones --- dev-requirements.txt | 3 +++ test/conftest.py | 11 +++++++++++ test/test_retry.py | 10 ++++++++++ test/tz_fake.py | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+) create mode 100644 test/tz_fake.py diff --git a/dev-requirements.txt b/dev-requirements.txt index dac6785e07..35f1f6dbf9 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -15,3 +15,6 @@ gcp-devrel-py-tools==0.0.15 # https://github.com/GoogleCloudPlatform/python-repo-tools/issues/23 pylint<2.0;python_version<="2.7" + +pytz==2020.1 +tzlocal==2.1 diff --git a/test/conftest.py b/test/conftest.py index f4bf8370ac..77ce869e01 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -8,6 +8,8 @@ import trustme from tornado import web, ioloop +from .tz_fake import fake_timezone_ctx + from dummyserver.handlers import TestingApp from dummyserver.server import run_tornado_app from dummyserver.server import HAS_IPV6 @@ -96,3 +98,12 @@ def ipv6_san_server(tmp_path_factory): with run_server_in_thread("https", "::1", tmpdir, ca, server_cert) as cfg: yield cfg + + +@pytest.yield_fixture +def fake_timezone(request): + """ + A pytest fixture that runs the test with a fake timezone. + """ + with fake_timezone_ctx(request.param): + yield diff --git a/test/test_retry.py b/test/test_retry.py index 0aee731fc5..7df274a217 100644 --- a/test/test_retry.py +++ b/test/test_retry.py @@ -332,6 +332,16 @@ def test_respect_retry_after_header_propagated(self, respect_retry_after_header) ("Mon Jun 3 11:30:12 2019", True, 1812), ], ) + @pytest.mark.parametrize( + "fake_timezone", + [ + "UTC", + "Asia/Jerusalem", + None, + ], + indirect=True, + ) + @pytest.mark.usefixtures("fake_timezone") def test_respect_retry_after_header_sleep( self, retry_after_header, respect_retry_after_header, sleep_duration ): diff --git a/test/tz_fake.py b/test/tz_fake.py new file mode 100644 index 0000000000..fce94b68c9 --- /dev/null +++ b/test/tz_fake.py @@ -0,0 +1,37 @@ +import pytz +from contextlib import contextmanager +import time +import os +import pytest +import tzlocal + + +@contextmanager +def fake_timezone_ctx(tz): + """ + Switch to a locally-known timezone specified by `tz`. + On exit, restore the previous timezone. + If `tz` is `None`, do nothing. + """ + if tz is None: + yield + return + + if not hasattr(time, "tzset"): + pytest.skip("Timezone patching is not supported") + + # Make sure the new timezone exists, at least in pytz + if tz not in pytz.all_timezones: + raise ValueError("Invalid timezone specified: %r" % (tz,)) + + # Get the current timezone + try: + old_tz = tzlocal.get_localzone().zone + except ValueError: + pytest.skip("Cannot determine current timezone") + + os.environ["TZ"] = tz + time.tzset() + yield + os.environ["TZ"] = old_tz + time.tzset() From 8774314fcebce857c73f12d82f6554aa8202b12d Mon Sep 17 00:00:00 2001 From: hodbn Date: Wed, 26 Aug 2020 18:25:33 +0000 Subject: [PATCH 4/5] Use dateutil instead of pytz and tzlocal --- dev-requirements.txt | 3 +-- test/conftest.py | 8 ++++---- test/test_retry.py | 4 ++-- test/tz_fake.py | 37 ------------------------------------- test/tz_stub.py | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 45 deletions(-) delete mode 100644 test/tz_fake.py create mode 100644 test/tz_stub.py diff --git a/dev-requirements.txt b/dev-requirements.txt index 35f1f6dbf9..3e31cc0e3e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -16,5 +16,4 @@ gcp-devrel-py-tools==0.0.15 # https://github.com/GoogleCloudPlatform/python-repo-tools/issues/23 pylint<2.0;python_version<="2.7" -pytz==2020.1 -tzlocal==2.1 +python-dateutil==2.8.1 diff --git a/test/conftest.py b/test/conftest.py index 77ce869e01..84e6c18e33 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -8,7 +8,7 @@ import trustme from tornado import web, ioloop -from .tz_fake import fake_timezone_ctx +from .tz_stub import stub_timezone_ctx from dummyserver.handlers import TestingApp from dummyserver.server import run_tornado_app @@ -101,9 +101,9 @@ def ipv6_san_server(tmp_path_factory): @pytest.yield_fixture -def fake_timezone(request): +def stub_timezone(request): """ - A pytest fixture that runs the test with a fake timezone. + A pytest fixture that runs the test with a stub timezone. """ - with fake_timezone_ctx(request.param): + with stub_timezone_ctx(request.param): yield diff --git a/test/test_retry.py b/test/test_retry.py index 7df274a217..a29b03e2cd 100644 --- a/test/test_retry.py +++ b/test/test_retry.py @@ -333,7 +333,7 @@ def test_respect_retry_after_header_propagated(self, respect_retry_after_header) ], ) @pytest.mark.parametrize( - "fake_timezone", + "stub_timezone", [ "UTC", "Asia/Jerusalem", @@ -341,7 +341,7 @@ def test_respect_retry_after_header_propagated(self, respect_retry_after_header) ], indirect=True, ) - @pytest.mark.usefixtures("fake_timezone") + @pytest.mark.usefixtures("stub_timezone") def test_respect_retry_after_header_sleep( self, retry_after_header, respect_retry_after_header, sleep_duration ): diff --git a/test/tz_fake.py b/test/tz_fake.py deleted file mode 100644 index fce94b68c9..0000000000 --- a/test/tz_fake.py +++ /dev/null @@ -1,37 +0,0 @@ -import pytz -from contextlib import contextmanager -import time -import os -import pytest -import tzlocal - - -@contextmanager -def fake_timezone_ctx(tz): - """ - Switch to a locally-known timezone specified by `tz`. - On exit, restore the previous timezone. - If `tz` is `None`, do nothing. - """ - if tz is None: - yield - return - - if not hasattr(time, "tzset"): - pytest.skip("Timezone patching is not supported") - - # Make sure the new timezone exists, at least in pytz - if tz not in pytz.all_timezones: - raise ValueError("Invalid timezone specified: %r" % (tz,)) - - # Get the current timezone - try: - old_tz = tzlocal.get_localzone().zone - except ValueError: - pytest.skip("Cannot determine current timezone") - - os.environ["TZ"] = tz - time.tzset() - yield - os.environ["TZ"] = old_tz - time.tzset() diff --git a/test/tz_stub.py b/test/tz_stub.py new file mode 100644 index 0000000000..5599f78699 --- /dev/null +++ b/test/tz_stub.py @@ -0,0 +1,40 @@ +import pytz +from contextlib import contextmanager +import time +import datetime +import os +import pytest +from dateutil import tz + + +@contextmanager +def stub_timezone_ctx(tzname): + """ + Switch to a locally-known timezone specified by `tzname`. + On exit, restore the previous timezone. + If `tzname` is `None`, do nothing. + """ + if tzname is None: + yield + return + + # Only supported on Unix + if not hasattr(time, "tzset"): + pytest.skip("Timezone patching is not supported") + + # Make sure the new timezone exists, at least in dateutil + new_tz = tz.gettz(tzname) + if new_tz is None: + raise ValueError("Invalid timezone specified: %r" % (tzname,)) + + # Get the current timezone + local_tz = tz.tzlocal() + if local_tz is None: + raise EnvironmentError("Cannot determine current timezone") + old_tzname = datetime.datetime.now(local_tz).tzname() + + os.environ["TZ"] = tzname + time.tzset() + yield + os.environ["TZ"] = old_tzname + time.tzset() From 7efccfb3e1be66881d64e306b0e38e7127951f08 Mon Sep 17 00:00:00 2001 From: hodbn Date: Wed, 26 Aug 2020 18:31:27 +0000 Subject: [PATCH 5/5] Remove unnecessary import --- test/tz_stub.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/tz_stub.py b/test/tz_stub.py index 5599f78699..5b1a8c7e18 100644 --- a/test/tz_stub.py +++ b/test/tz_stub.py @@ -1,4 +1,3 @@ -import pytz from contextlib import contextmanager import time import datetime