From f9fabb7df7f1ca189fd560767db8d1319f0d4f81 Mon Sep 17 00:00:00 2001 From: Frederic BLANC Date: Wed, 25 Jan 2017 13:38:00 -0800 Subject: [PATCH 1/4] fixes #2244 (cherry picked from commit b6fecca7ba17c7cf8dbfa5206c5e2469c0ccd1f6) --- acme/acme/client.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index b5db57235..fb8e092fa 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -660,8 +660,16 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes def post(self, url, obj, content_type=JOSE_CONTENT_TYPE, **kwargs): """POST object wrapped in `.JWS` and check response.""" - data = self._wrap_in_jws(obj, self._get_nonce(url)) - kwargs.setdefault('headers', {'Content-Type': content_type}) - response = self._send_request('POST', url, data=data, **kwargs) - self._add_nonce(response) - return self._check_response(response, content_type=content_type) + MAX_ATTEMPTS = 3 + for attempt in range(MAX_ATTEMPTS+1): + try: + data = self._wrap_in_jws(obj, self._get_nonce(url)) + kwargs.setdefault('headers', {'Content-Type': content_type}) + response = self._send_request('POST', url, data=data, **kwargs) + self._add_nonce(response) + return self._check_response(response, content_type=content_type) + except messages.Error as e: + if attempt < MAX_ATTEMPTS and e.typ == 'urn:ietf:params:acme:error:badNonce': + logger.debug('Got badNonce error (%i times)', attempt+1) + else: + raise From 342ef923eec035b474b233af99122f5e9312973a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 Jan 2017 14:10:19 -0800 Subject: [PATCH 2/4] fix stylistic nits with POST retry (cherry picked from commit a5da551965bf78cd5394c5cafb10e1893bbd132b) --- acme/acme/client.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index fb8e092fa..cdfa43ee0 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -658,18 +658,27 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes self._add_nonce(self.head(url)) return self._nonces.pop() - def post(self, url, obj, content_type=JOSE_CONTENT_TYPE, **kwargs): - """POST object wrapped in `.JWS` and check response.""" - MAX_ATTEMPTS = 3 - for attempt in range(MAX_ATTEMPTS+1): + def post(self, *args, **kwargs): + """POST object wrapped in `.JWS` and check response. + + If the server responded with a badNonce error, the request will + be retried once. + + """ + should_retry = True + while True: try: - data = self._wrap_in_jws(obj, self._get_nonce(url)) - kwargs.setdefault('headers', {'Content-Type': content_type}) - response = self._send_request('POST', url, data=data, **kwargs) - self._add_nonce(response) - return self._check_response(response, content_type=content_type) - except messages.Error as e: - if attempt < MAX_ATTEMPTS and e.typ == 'urn:ietf:params:acme:error:badNonce': - logger.debug('Got badNonce error (%i times)', attempt+1) + return self._post_once(*args, **kwargs) + except messages.Error as error: + if should_retry and error.code == 'badNonce': + logger.debug('Retrying request after error:\n%s', error) + should_retry = False else: raise + + def _post_once(self, url, obj, content_type=JOSE_CONTENT_TYPE, **kwargs): + data = self._wrap_in_jws(obj, self._get_nonce(url)) + kwargs.setdefault('headers', {'Content-Type': content_type}) + response = self._send_request('POST', url, data=data, **kwargs) + self._add_nonce(response) + return self._check_response(response, content_type=content_type) From 4e23315999979901b17ff19f37bd81baeccf7cea Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 Jan 2017 15:08:01 -0800 Subject: [PATCH 3/4] add test_post_failed_retry (cherry picked from commit 46d9809fa1a99caf2027e181dc39c70ebc5944a0) --- acme/acme/client_test.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index e0403ef28..2de67519c 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -616,7 +616,9 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.wrapped_obj = mock.MagicMock() self.content_type = mock.sentinel.content_type - self.all_nonces = [jose.b64encode(b'Nonce'), jose.b64encode(b'Nonce2')] + self.all_nonces = [ + jose.b64encode(b'Nonce'), + jose.b64encode(b'Nonce2'), jose.b64encode(b'Nonce3')] self.available_nonces = self.all_nonces[:] def send_request(*args, **kwargs): @@ -664,7 +666,7 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.net._wrap_in_jws.assert_called_once_with( self.obj, jose.b64decode(self.all_nonces.pop())) - assert not self.available_nonces + self.available_nonces = [] self.assertRaises(errors.MissingNonce, self.net.post, 'uri', self.obj, content_type=self.content_type) self.net._wrap_in_jws.assert_called_with( @@ -680,6 +682,15 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.assertRaises(errors.BadNonce, self.net.post, 'uri', self.obj, content_type=self.content_type) + def test_post_failed_retry(self): + check_response = mock.MagicMock() + check_response.side_effect = messages.Error.with_code('badNonce') + + # pylint: disable=protected-access + self.net._check_response = check_response + self.assertRaises(messages.Error, self.net.post, 'uri', + self.obj, content_type=self.content_type) + def test_head_get_post_error_passthrough(self): self.send_request.side_effect = requests.exceptions.RequestException for method in self.net.head, self.net.get: From 9d7382d3d6d46be177885a1809042be73471b75a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 Jan 2017 15:10:24 -0800 Subject: [PATCH 4/4] add test_post_successful_retry (cherry picked from commit c650c9a7099d2d918d3a7dcb3fde4ef07e2829f4) --- acme/acme/client_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 2de67519c..5672f4200 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -691,6 +691,16 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.assertRaises(messages.Error, self.net.post, 'uri', self.obj, content_type=self.content_type) + def test_post_successful_retry(self): + check_response = mock.MagicMock() + check_response.side_effect = [messages.Error.with_code('badNonce'), + self.checked_response] + + # pylint: disable=protected-access + self.net._check_response = check_response + self.assertEqual(self.checked_response, self.net.post( + 'uri', self.obj, content_type=self.content_type)) + def test_head_get_post_error_passthrough(self): self.send_request.side_effect = requests.exceptions.RequestException for method in self.net.head, self.net.get: