Merge pull request go-macaroon-bakery#87 from sergiusens/jsondecodeerror

httpbakery/client: catch non json discharge responses

macaroonbakery/httpbakery/_client.py

@@ -131,7 +131,13 @@ def acquire_discharge(self, cav, payload):
         # TODO Fabrice what is the other http response possible ??
         if resp.status_code == 200:
             return bakery.Macaroon.from_dict(resp.json().get('Macaroon'))
-        cause = Error.from_dict(resp.json())
+        # A 5xx error might not return json.
+        try:
+            cause = Error.from_dict(resp.json())
+        except ValueError:
+            raise DischargeError(
+                'unexpected response: [{}] {!r}'.format(resp.status_code, resp.content)
+            )
         if cause.code != ERR_INTERACTION_REQUIRED:
             raise DischargeError(cause.message)
         if cause.info is None:

macaroonbakery/tests/test_client.py

@@ -3,6 +3,7 @@
 import base64
 import datetime
 import json
+import platform
 import threading
 
 import macaroonbakery.bakery as bakery
@@ -11,6 +12,7 @@
 import pymacaroons
 import requests
 import macaroonbakery._utils as utils
+from macaroonbakery.httpbakery._error import DischargeError
 
 from fixtures import (
     EnvironmentVariable,
@@ -516,6 +518,56 @@ def kind(self):
         finally:
             httpd.shutdown()
 
+    def test_discharge_jsondecodeerror(self):
+        class _DischargerLocator(bakery.ThirdPartyLocator):
+            def __init__(self):
+                self.key = bakery.generate_key()
+
+            def third_party_info(self, loc):
+                if loc == 'http://1.2.3.4':
+                    return bakery.ThirdPartyInfo(
+                        public_key=self.key.public_key,
+                        version=bakery.LATEST_VERSION,
+                    )
+        d = _DischargerLocator()
+        b = new_bakery('loc', d, None)
+
+        @urlmatch(path='.*/discharge')
+        def discharge(url, request):
+            return {
+                'status_code': 503,
+                'content': 'bad system',
+            }
+
+        def handler(*args):
+            GetHandler(b, 'http://1.2.3.4', None, None, None, AGES, *args)
+
+        try:
+            httpd = HTTPServer(('', 0), handler)
+            thread = threading.Thread(target=httpd.serve_forever)
+            thread.start()
+
+            client = httpbakery.Client()
+
+            with HTTMock(discharge):
+                with self.assertRaises(DischargeError) as discharge_error:
+                    requests.get(
+                        'http://' + httpd.server_address[0] + ':' + str(
+                            httpd.server_address[1]),
+                        cookies=client.cookies,
+                        auth=client.auth())
+                if platform.python_version_tuple()[0] == '2':
+                    self.assertEquals(str(discharge_error.exception),
+                                      'third party refused dischargex: unexpected response: '
+                                      "[503] 'bad system'")
+                else:
+                    self.assertEquals(str(discharge_error.exception),
+                                      'third party refused dischargex: unexpected response: '
+                                      "[503] b'bad system'")
+
+        finally:
+            httpd.shutdown()
+
     def test_extract_macaroons_from_request(self):
         def encode_macaroon(m):
             macaroons = '[' + utils.macaroon_to_json_string(m) + ']'