Handle JSON decode error in handle_exceptions()

pki.handle_exceptions() raises a JSON decode exception when the body of
the HTTPException is not a valid JSON string. The JSON exception hides
the true error message.

The patch also fixes a bug in PKIException.from_json(). The code and
ClassName attribute are now correctly set. Finally we have our first
unit test.

https://fedorahosted.org/pki/ticket/1488
https://fedorahosted.org/freeipa/ticket/5129

4 files changed, 97 insertions, 11 deletions

diff --git a/.gitignore b/.gitignore
index d2ecc2f89..172610bb1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,5 @@ tests/dogtag/dev_java_tests/bin/
 .tox
 dist
 MANIFEST
+*.pyc
+__pycache__
diff --git a/base/common/python/pki/__init__.py b/base/common/python/pki/__init__.py
index c383cdb4d..39a0db717 100644
--- a/base/common/python/pki/__init__.py
+++ b/base/common/python/pki/__init__.py
@@ -24,6 +24,7 @@ This module contains top-level classes and functions used by the Dogtag project.
 from functools import wraps
 import os
 import re
+import sys
 import requests
 
 
@@ -205,10 +206,12 @@ class PKIException(Exception, ResourceMessage):
         :type json_value: str
         :return: pki.PKIException
         """
-        ret = cls(json_value['Message'], json_value['Code'],
-                  json_value['ClassName'])
+        ret = cls(
+            message=json_value['Message'],
+            code=json_value['Code'],
+            class_name=json_value['ClassName']
+        )
         for attr in json_value['Attributes']['Attribute']:
-            print str(attr)
             ret.add_attribute(attr["name"], attr["value"])
         return ret
 
@@ -293,15 +296,25 @@ def handle_exceptions():
             """ Decorator to catch and re-throw PKIExceptions."""
             try:
                 return fn_call(inst, *args, **kwargs)
-            except requests.exceptions.HTTPError as exc:
-                clazz = exc.response.json()['ClassName']
-                if clazz in EXCEPTION_MAPPINGS:
-                    exception_class = EXCEPTION_MAPPINGS[clazz]
-                    pki_exception = exception_class.from_json(
-                        exc.response.json())
-                    raise pki_exception
+            except requests.exceptions.HTTPError:
+                # store exception information. json may raise another
+                # exception. We want to re-raise the HTTPError.
+                exc_type, exc_val, exc_tb = sys.exc_info()
+                try:
+                    json = exc_val.response.json()
+                except ValueError:
+                    # json raises ValueError. simplejson raises
+                    # JSONDecodeError, which is a subclass of ValueError.
+                    # re-raise original exception
+                    raise exc_type, exc_val, exc_tb
                 else:
-                    raise exc
+                    # clear reference cycle
+                    exc_type = exc_val = exc_tb = None
+                    clazz = json.get('ClassName')
+                    if clazz and clazz in EXCEPTION_MAPPINGS:
+                        exception_class = EXCEPTION_MAPPINGS[clazz]
+                        pki_exception = exception_class.from_json(json)
+                        raise pki_exception
 
         return handler
 
diff --git a/tests/python/test_pki.py b/tests/python/test_pki.py
new file mode 100644
index 000000000..d30a1cd84
--- /dev/null
+++ b/tests/python/test_pki.py
@@ -0,0 +1,65 @@
+# Authors:
+#     Christian Heimes <cheimes@redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright (C) 2015 Red Hat, Inc.
+# All rights reserved.
+#
+
+import unittest
+
+import pki
+import requests
+import json
+
+
+class TestHTTPError(requests.exceptions.HTTPError):
+    def __init__(self, body):
+        super(TestHTTPError, self).__init__()
+        self.response = requests.Response()
+        self.response._content = body
+
+class PKITests(unittest.TestCase):
+    def test_handle_exceptions(self):
+
+        @pki.handle_exceptions()
+        def raiser(body):
+            raise TestHTTPError(body)
+
+        body = json.dumps({
+            'Message': 'message',
+            'Code': 42,
+            'ClassName': 'com.netscape.certsrv.base.BadRequestException',
+            'Attributes': {
+                'Attribute': [],
+            },
+        })
+
+        with self.assertRaises(pki.BadRequestException) as e:
+            raiser(body)
+
+        self.assertEqual(e.exception.message, 'message')
+        self.assertEqual(e.exception.code, 42)
+        self.assertEqual(
+            e.exception.ClassName,
+            'com.netscape.certsrv.base.BadRequestException'
+        )
+
+        with self.assertRaises(TestHTTPError) as e:
+            raiser('no json body')
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tox.ini b/tox.ini
index 8b0c619c8..54ff3b873 100644
--- a/tox.ini
+++ b/tox.ini
@@ -25,6 +25,8 @@ envlist = py27,lint,docs
 # force installation of sphinx and lint in virtual env, otherwise
 # the command pick up the `pki` package from the system's site packages.
 install_command = pip install {opts} --force-reinstall --upgrade {packages}
+deps =
+    pytest
 
 [testenv:py27]
 sitepackages = True
@@ -34,6 +36,7 @@ commands =
     python2.7 {envbindir}/pki-server --help
     python2.7 {envbindir}/pki-server-upgrade --help
     python2.7 {envbindir}/pki-upgrade --help
+    py.test --capture=no --strict {posargs}
 
 [testenv:lint]
 basepython = python2.7
@@ -71,6 +74,9 @@ deps =
 commands =
     sphinx-build -v -W -b html -d {envtmpdir}/doctrees . {envtmpdir}/html
 
+[pytest]
+python_files = tests/python/*.py
+
 [flake8]
 exclude = .tox,*.egg,dist,build,conf.py,tests/*
 include = *.py,pki-upgrade,pkidestroy,pki-server,pki-server-upgrade,pkispawn,pki