From 2e2ad4bd757f8b2d90e2ee72234d81dfc6857b7e Mon Sep 17 00:00:00 2001 From: Seth Michael Larson Date: Tue, 4 Jun 2019 12:05:05 -0500 Subject: Allow Python 3.8-dev to fail (#1633) --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index c803f488..f2835b58 100644 --- a/.travis.yml +++ b/.travis.yml @@ -105,6 +105,7 @@ matrix: - ./_travis/deploy.sh allow_failures: + - python: 3.8-dev - python: pypy3.5-6.0 - python: pypy2.7-6.0 -- cgit v1.2.1 From 21671d8158966f1d1e194bd9cd1802cfe80ce16e Mon Sep 17 00:00:00 2001 From: Robert David Grant Date: Tue, 4 Jun 2019 14:31:00 -0400 Subject: Clarify the units for timeouts within documentation (#1630) --- docs/user-guide.rst | 4 ++-- src/urllib3/util/timeout.py | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/user-guide.rst b/docs/user-guide.rst index 36081e09..bc77d547 100644 --- a/docs/user-guide.rst +++ b/docs/user-guide.rst @@ -302,8 +302,8 @@ to the standard-library :mod:`ssl` module. You may experience Using timeouts -------------- -Timeouts allow you to control how long requests are allowed to run before -being aborted. In simple cases, you can specify a timeout as a ``float`` +Timeouts allow you to control how long (in seconds) requests are allowed to run +before being aborted. In simple cases, you can specify a timeout as a ``float`` to :meth:`~poolmanager.PoolManager.request`:: >>> http.request( diff --git a/src/urllib3/util/timeout.py b/src/urllib3/util/timeout.py index 6623bbad..c1dc1e97 100644 --- a/src/urllib3/util/timeout.py +++ b/src/urllib3/util/timeout.py @@ -46,19 +46,20 @@ class Timeout(object): :type total: integer, float, or None :param connect: - The maximum amount of time to wait for a connection attempt to a server - to succeed. Omitting the parameter will default the connect timeout to - the system default, probably `the global default timeout in socket.py + The maximum amount of time (in seconds) to wait for a connection + attempt to a server to succeed. Omitting the parameter will default the + connect timeout to the system default, probably `the global default + timeout in socket.py `_. None will set an infinite timeout for connection attempts. :type connect: integer, float, or None :param read: - The maximum amount of time to wait between consecutive - read operations for a response from the server. Omitting - the parameter will default the read timeout to the system - default, probably `the global default timeout in socket.py + The maximum amount of time (in seconds) to wait between consecutive + read operations for a response from the server. Omitting the parameter + will default the read timeout to the system default, probably `the + global default timeout in socket.py `_. None will set an infinite timeout. @@ -195,7 +196,7 @@ class Timeout(object): def get_connect_duration(self): """ Gets the time elapsed since the call to :meth:`start_connect`. - :return: Elapsed time. + :return: Elapsed time in seconds. :rtype: float :raises urllib3.exceptions.TimeoutStateError: if you attempt to get duration for a timer that hasn't been started. -- cgit v1.2.1 From f6d5b4b8da14630acc86633bbfd0757603f07edd Mon Sep 17 00:00:00 2001 From: Ratan Kulshreshtha Date: Wed, 5 Jun 2019 00:01:58 +0530 Subject: Ignore flake8 errors that are introduced deliberately by black (#1631) --- setup.cfg | 1 + src/urllib3/connection.py | 6 +++--- src/urllib3/contrib/pyopenssl.py | 4 ++-- src/urllib3/contrib/securetransport.py | 4 +--- src/urllib3/util/url.py | 4 ++-- test/test_response.py | 8 ++++---- 6 files changed, 13 insertions(+), 14 deletions(-) diff --git a/setup.cfg b/setup.cfg index dbd367b8..16d64d20 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,4 +1,5 @@ [flake8] +ignore = E501, E203, W503, W504 exclude=./docs/conf.py,./src/urllib3/packages/* max-line-length=99 diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py index f0396f25..27bc1867 100644 --- a/src/urllib3/connection.py +++ b/src/urllib3/connection.py @@ -412,9 +412,9 @@ class VerifiedHTTPSConnection(HTTPSConnection): if not cert.get("subjectAltName", ()): warnings.warn( ( - "Certificate for {0} has no `subjectAltName`, falling back to check for a " # noqa - "`commonName` for now. This feature is being removed by major browsers and " # noqa - "deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 " # noqa + "Certificate for {0} has no `subjectAltName`, falling back to check for a " + "`commonName` for now. This feature is being removed by major browsers and " + "deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 " "for details.)".format(hostname) ), SubjectAltNameWarning, diff --git a/src/urllib3/contrib/pyopenssl.py b/src/urllib3/contrib/pyopenssl.py index 58ea971e..3051ef3a 100644 --- a/src/urllib3/contrib/pyopenssl.py +++ b/src/urllib3/contrib/pyopenssl.py @@ -187,7 +187,7 @@ def _dnsname_to_stdlib(name): try: for prefix in [u"*.", u"."]: if name.startswith(prefix): - name = name[len(prefix) :] # noqa + name = name[len(prefix) :] return prefix.encode("ascii") + idna.encode(name) return idna.encode(name) except idna.core.IDNAError: @@ -349,7 +349,7 @@ class WrappedSocket(object): total_sent = 0 while total_sent < len(data): sent = self._send_until_done( - data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE] # noqa + data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE] ) total_sent += sent diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py index 7b975c0d..24e6b5c4 100644 --- a/src/urllib3/contrib/securetransport.py +++ b/src/urllib3/contrib/securetransport.py @@ -616,9 +616,7 @@ class WrappedSocket(object): def sendall(self, data): total_sent = 0 while total_sent < len(data): - sent = self.send( - data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE] # noqa - ) + sent = self.send(data[total_sent : total_sent + SSL_WRITE_BLOCKSIZE]) total_sent += sent def shutdown(self): diff --git a/src/urllib3/util/url.py b/src/urllib3/util/url.py index 2ba45360..f225cd8b 100644 --- a/src/urllib3/util/url.py +++ b/src/urllib3/util/url.py @@ -149,7 +149,7 @@ def split_first(s, delims): if min_idx is None or min_idx < 0: return s, "", None - return s[:min_idx], s[min_idx + 1 :], min_delim # noqa + return s[:min_idx], s[min_idx + 1 :], min_delim def _encode_invalid_chars(component, allowed_chars, encoding="utf-8"): @@ -173,7 +173,7 @@ def _encode_invalid_chars(component, allowed_chars, encoding="utf-8"): for i in range(0, len(uri_bytes)): # Will return a single character bytestring on both Python 2 & 3 - byte = uri_bytes[i : i + 1] # noqa + byte = uri_bytes[i : i + 1] byte_ord = ord(byte) if (is_percent_encoded and byte == b"%") or ( byte_ord < 128 and byte.decode() in allowed_chars diff --git a/test/test_response.py b/test/test_response.py index 18b3e373..23a63be1 100644 --- a/test/test_response.py +++ b/test/test_response.py @@ -440,7 +440,7 @@ class TestResponse(object): def __init__(self, payload, payload_part_size): self.payloads = [ - payload[i * payload_part_size : (i + 1) * payload_part_size] # noqa + payload[i * payload_part_size : (i + 1) * payload_part_size] for i in range(NUMBER_OF_READS + 1) ] @@ -633,7 +633,7 @@ class TestResponse(object): data = compress.compress(b"foobar") data += compress.flush() for i in range(0, len(data), 2): - yield data[i : i + 2] # noqa + yield data[i : i + 2] fp = MockChunkedEncodingResponse(list(stream())) r = httplib.HTTPResponse(MockSock) @@ -801,7 +801,7 @@ class TestResponse(object): data = compress.compress(b"foo\nbar") data += compress.flush() for i in range(0, len(data), 2): - yield data[i : i + 2] # noqa + yield data[i : i + 2] fp = MockChunkedEncodingResponse(list(stream())) r = httplib.HTTPResponse(MockSock) @@ -862,7 +862,7 @@ class MockChunkedEncodingResponse(object): return b"" else: chunk_part = self.cur_chunk[: i + 2] - self.cur_chunk = self.cur_chunk[i + 2 :] # noqa + self.cur_chunk = self.cur_chunk[i + 2 :] return chunk_part elif amt <= -1: chunk_part = self.cur_chunk -- cgit v1.2.1 From 728d9244665ef5b03103cb74d7b409ebe4f23b43 Mon Sep 17 00:00:00 2001 From: James Meickle Date: Tue, 4 Jun 2019 14:32:54 -0400 Subject: Improve implementation of respect_retry_after_header (#1607) --- CHANGES.rst | 9 ++++++ CONTRIBUTORS.txt | 3 ++ src/urllib3/util/retry.py | 3 +- test/test_retry.py | 70 +++++++++++++++++++++++++++++++++++++++++++++++ test/test_util.py | 15 ---------- 5 files changed, 84 insertions(+), 16 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index a7724471..55a9c0c9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,15 @@ Changes ======= +dev (master) +------------ + +* Propagate Retry-After header settings to subsequent retries. (Pull #1607) + +* Fix edge case where Retry-After header was still respected even when + explicitly opted out of. (Pull #1607) + + 1.25.3 (2019-05-23) ------------------- diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 3da291e5..6f0362c3 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -275,5 +275,8 @@ In chronological order: * Katsuhiko YOSHIDA * Remove Authorization header regardless of case when redirecting to cross-site +* James Meickle + * Improve handling of Retry-After header + * [Your name or handle] <[email or website]> * [Brief summary of your changes] diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py index c26f48c9..5a049fe6 100644 --- a/src/urllib3/util/retry.py +++ b/src/urllib3/util/retry.py @@ -210,6 +210,7 @@ class Retry(object): raise_on_status=self.raise_on_status, history=self.history, remove_headers_on_redirect=self.remove_headers_on_redirect, + respect_retry_after_header=self.respect_retry_after_header, ) params.update(kw) return type(self)(**params) @@ -294,7 +295,7 @@ class Retry(object): this method will return immediately. """ - if response: + if self.respect_retry_after_header and response: slept = self.sleep_for_retry(response) if slept: return diff --git a/test/test_retry.py b/test/test_retry.py index 7b228d75..c36476f8 100644 --- a/test/test_retry.py +++ b/test/test_retry.py @@ -1,10 +1,15 @@ +import datetime +import mock import pytest +import time from urllib3.response import HTTPResponse +from urllib3.packages import six from urllib3.packages.six.moves import xrange from urllib3.util.retry import Retry, RequestHistory from urllib3.exceptions import ( ConnectTimeoutError, + InvalidHeader, MaxRetryError, ReadTimeoutError, ResponseError, @@ -271,3 +276,68 @@ class TestRetry(object): retry = Retry(remove_headers_on_redirect=["X-API-Secret"]) assert list(retry.remove_headers_on_redirect) == ["x-api-secret"] + + @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")]) # \xb2 = ^2 + def test_parse_retry_after_invalid(self, value): + retry = Retry() + with pytest.raises(InvalidHeader): + retry.parse_retry_after(value) + + @pytest.mark.parametrize( + "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)] + ) + def test_parse_retry_after(self, value, expected): + retry = Retry() + assert retry.parse_retry_after(value) == expected + + @pytest.mark.parametrize("respect_retry_after_header", [True, False]) + def test_respect_retry_after_header_propagated(self, respect_retry_after_header): + + retry = Retry(respect_retry_after_header=respect_retry_after_header) + new_retry = retry.new() + assert new_retry.respect_retry_after_header == respect_retry_after_header + + @pytest.mark.parametrize( + "retry_after_header,respect_retry_after_header,sleep_duration", + [ + ("3600", True, 3600), + ("3600", False, None), + # Will sleep due to header is 1 hour in future + ("Mon, 3 Jun 2019 12:00:00 UTC", True, 3600), + # Won't sleep due to not respecting header + ("Mon, 3 Jun 2019 12:00:00 UTC", False, None), + # Won't sleep due to current time reached + ("Mon, 3 Jun 2019 11:00:00 UTC", True, None), + # Won't sleep due to current time reached + not respecting header + ("Mon, 3 Jun 2019 11:00:00 UTC", False, None), + ], + ) + def test_respect_retry_after_header_sleep( + self, retry_after_header, respect_retry_after_header, sleep_duration + ): + retry = Retry(respect_retry_after_header=respect_retry_after_header) + + # Date header syntax can specify an absolute date; compare this to the + # time in the parametrized inputs above. + current_time = mock.MagicMock( + return_value=time.mktime( + datetime.datetime(year=2019, month=6, day=3, hour=11).timetuple() + ) + ) + + with mock.patch("time.sleep") as sleep_mock, mock.patch( + "time.time", current_time + ): + # for the default behavior, it must be in RETRY_AFTER_STATUS_CODES + response = HTTPResponse( + status=503, headers={"Retry-After": retry_after_header} + ) + + retry.sleep(response) + + # The expected behavior is that we'll only sleep if respecting + # this header (since we won't have any backoff sleep attempts) + if respect_retry_after_header and sleep_duration is not None: + sleep_mock.assert_called_with(sleep_duration) + else: + sleep_mock.assert_not_called() diff --git a/test/test_util.py b/test/test_util.py index b2906f22..37c09ab2 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -13,7 +13,6 @@ import pytest from urllib3 import add_stderr_logger, disable_warnings from urllib3.util.request import make_headers, rewind_body, _FAILEDTELL from urllib3.util.response import assert_header_parsing -from urllib3.util.retry import Retry from urllib3.util.timeout import Timeout from urllib3.util.url import get_host, parse_url, split_first, Url from urllib3.util.ssl_ import ( @@ -27,7 +26,6 @@ from urllib3.exceptions import ( TimeoutStateError, InsecureRequestWarning, SNIMissingWarning, - InvalidHeader, UnrewindableBodyError, ) from urllib3.util.connection import allowed_gai_family, _has_ipv6 @@ -782,19 +780,6 @@ class TestUtil(object): with patch("urllib3.util.connection.HAS_IPV6", False): assert allowed_gai_family() == socket.AF_INET - @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")]) # \xb2 = ^2 - def test_parse_retry_after_invalid(self, value): - retry = Retry() - with pytest.raises(InvalidHeader): - retry.parse_retry_after(value) - - @pytest.mark.parametrize( - "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)] - ) - def test_parse_retry_after(self, value, expected): - retry = Retry() - assert retry.parse_retry_after(value) == expected - @pytest.mark.parametrize("headers", [b"foo", None, object]) def test_assert_header_parsing_throws_typeerror_with_non_headers(self, headers): with pytest.raises(TypeError): -- cgit v1.2.1