From c50847cabfd6c8506ec87649e13202af5d6af3e2 Mon Sep 17 00:00:00 2001 From: Christopher Grebs Date: Fri, 26 Jul 2019 16:28:44 +0200 Subject: [PATCH 1/3] Handle 'None' return value of wait() properly during throttling. Fixes #6836 --- rest_framework/views.py | 12 +++++++++++- tests/test_throttling.py | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 832f172330..5f52adaa51 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -356,7 +356,17 @@ def check_throttles(self, request): throttle_durations.append(throttle.wait()) if throttle_durations: - self.throttled(request, max(throttle_durations)) + # Filter out `None` values which may happen in case of config / rate + # changes, see #1438 + durations = [ + duration for duration in throttle_durations + if duration is not None + ] + + if durations: + self.throttled(request, max(durations)) + else: + self.throttled(request, None) def determine_version(self, request, *args, **kwargs): """ diff --git a/tests/test_throttling.py b/tests/test_throttling.py index 3c172e2636..2cc60a1a7a 100644 --- a/tests/test_throttling.py +++ b/tests/test_throttling.py @@ -159,6 +159,26 @@ def test_request_throttling_multiple_throttles(self): assert response.status_code == 429 assert int(response['retry-after']) == 58 + def test_handle_negative_throttle_value(self): + self.set_throttle_timer(MockView_DoubleThrottling, 0) + request = self.factory.get('/') + for dummy in range(24): + response = MockView_DoubleThrottling.as_view()(request) + assert response.status_code == 429 + assert int(response['retry-after']) == 60 + + previous_rate = User3SecRateThrottle.rate + User3SecRateThrottle.rate = '1/sec' + + for dummy in range(24): + response = MockView_DoubleThrottling.as_view()(request) + + assert response.status_code == 429 + assert int(response['retry-after']) == 60 + + # reset + User3SecRateThrottle.rate = previous_rate + def ensure_response_header_contains_proper_throttle_field(self, view, expected_headers): """ Ensure the response returns an Retry-After field with status and next attributes From 957b39fa8d6ff0f9866d0f697c4ff93a343613b7 Mon Sep 17 00:00:00 2001 From: Christopher Grebs Date: Tue, 30 Jul 2019 09:40:32 +0200 Subject: [PATCH 2/3] Wrap test in try/finally, cleanup duration determination --- rest_framework/views.py | 6 ++---- tests/test_throttling.py | 17 +++++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index 5f52adaa51..da85cccec8 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -363,10 +363,8 @@ def check_throttles(self, request): if duration is not None ] - if durations: - self.throttled(request, max(durations)) - else: - self.throttled(request, None) + duration = max(durations) if durations else None + self.throttled(request, duration) def determine_version(self, request, *args, **kwargs): """ diff --git a/tests/test_throttling.py b/tests/test_throttling.py index 2cc60a1a7a..eb34a6c913 100644 --- a/tests/test_throttling.py +++ b/tests/test_throttling.py @@ -168,16 +168,17 @@ def test_handle_negative_throttle_value(self): assert int(response['retry-after']) == 60 previous_rate = User3SecRateThrottle.rate - User3SecRateThrottle.rate = '1/sec' + try: + User3SecRateThrottle.rate = '1/sec' - for dummy in range(24): - response = MockView_DoubleThrottling.as_view()(request) - - assert response.status_code == 429 - assert int(response['retry-after']) == 60 + for dummy in range(24): + response = MockView_DoubleThrottling.as_view()(request) - # reset - User3SecRateThrottle.rate = previous_rate + assert response.status_code == 429 + assert int(response['retry-after']) == 60 + finally: + # reset + User3SecRateThrottle.rate = previous_rate def ensure_response_header_contains_proper_throttle_field(self, view, expected_headers): """ From 8e6ee98ee921ea2f7a3d94dc99e54a9617483831 Mon Sep 17 00:00:00 2001 From: Christopher Grebs Date: Wed, 7 Aug 2019 10:48:15 +0200 Subject: [PATCH 3/3] Use default argument of max() and change test name --- rest_framework/views.py | 2 +- tests/test_throttling.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rest_framework/views.py b/rest_framework/views.py index da85cccec8..bec10560a9 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -363,7 +363,7 @@ def check_throttles(self, request): if duration is not None ] - duration = max(durations) if durations else None + duration = max(durations, default=None) self.throttled(request, duration) def determine_version(self, request, *args, **kwargs): diff --git a/tests/test_throttling.py b/tests/test_throttling.py index eb34a6c913..d5a61232d9 100644 --- a/tests/test_throttling.py +++ b/tests/test_throttling.py @@ -159,7 +159,7 @@ def test_request_throttling_multiple_throttles(self): assert response.status_code == 429 assert int(response['retry-after']) == 58 - def test_handle_negative_throttle_value(self): + def test_throttle_rate_change_negative(self): self.set_throttle_timer(MockView_DoubleThrottling, 0) request = self.factory.get('/') for dummy in range(24):