diff --git a/CHANGES.txt b/CHANGES.txt index da59c3e6f8..96d8d82369 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,13 @@ unreleased ========== +- Pyramid HTTPExceptions will now take into account the best match for the + clients Accept header, and depending on what is requested will return + text/html, application/json or text/plain. The default for */* is still + text/html, but if application/json is explicitly mentioned it will now + receive a valid JSON response. See: + https://github.com/Pylons/pyramid/pull/2489 + - (Deprecation) Support for Python 3.3 will be removed in Pyramid 1.8. https://github.com/Pylons/pyramid/issues/2477 diff --git a/pyramid/httpexceptions.py b/pyramid/httpexceptions.py index 8bf9a0a729..e76f43c8a2 100644 --- a/pyramid/httpexceptions.py +++ b/pyramid/httpexceptions.py @@ -123,12 +123,14 @@ field. Reflecting this, these subclasses have one additional keyword argument: ``location``, which indicates the location to which to redirect. """ +import json from string import Template from zope.interface import implementer from webob import html_escape as _html_escape +from webob.acceptparse import MIMEAccept from pyramid.compat import ( class_types, @@ -214,7 +216,7 @@ class HTTPException(Response, Exception): empty_body = False def __init__(self, detail=None, headers=None, comment=None, - body_template=None, **kw): + body_template=None, json_formatter=None, **kw): status = '%s %s' % (self.code, self.title) Response.__init__(self, status=status, **kw) Exception.__init__(self, detail) @@ -225,6 +227,8 @@ def __init__(self, detail=None, headers=None, comment=None, if body_template is not None: self.body_template = body_template self.body_template_obj = Template(body_template) + if json_formatter is not None: + self._json_formatter = json_formatter if self.empty_body: del self.content_type @@ -233,18 +237,48 @@ def __init__(self, detail=None, headers=None, comment=None, def __str__(self): return self.detail or self.explanation + def _json_formatter(self, status, body, title, environ): + return {'message': body, + 'code': status, + 'title': self.title} + def prepare(self, environ): if not self.body and not self.empty_body: html_comment = '' comment = self.comment or '' - accept = environ.get('HTTP_ACCEPT', '') - if accept and 'html' in accept or '*/*' in accept: + accept_value = environ.get('HTTP_ACCEPT', '') + accept = MIMEAccept(accept_value) + # Attempt to match text/html or application/json, if those don't + # match, we will fall through to defaulting to text/plain + match = accept.best_match(['text/html', 'application/json']) + + if match == 'text/html': self.content_type = 'text/html' escape = _html_escape page_template = self.html_template_obj br = '
' if comment: html_comment = '' % escape(comment) + elif match == 'application/json': + self.content_type = 'application/json' + self.charset = None + escape = _no_escape + br = '\n' + if comment: + html_comment = escape(comment) + + class JsonPageTemplate(object): + def __init__(self, excobj): + self.excobj = excobj + + def substitute(self, status, body): + jsonbody = self.excobj._json_formatter( + status=status, + body=body, title=self.excobj.title, + environ=environ) + return json.dumps(jsonbody) + + page_template = JsonPageTemplate(self) else: self.content_type = 'text/plain' escape = _no_escape @@ -253,11 +287,11 @@ def prepare(self, environ): if comment: html_comment = escape(comment) args = { - 'br':br, + 'br': br, 'explanation': escape(self.explanation), 'detail': escape(self.detail or ''), 'comment': escape(comment), - 'html_comment':html_comment, + 'html_comment': html_comment, } body_tmpl = self.body_template_obj if HTTPException.body_template_obj is not body_tmpl: @@ -274,7 +308,7 @@ def prepare(self, environ): body = body_tmpl.substitute(args) page = page_template.substitute(status=self.status, body=body) if isinstance(page, text_type): - page = page.encode(self.charset) + page = page.encode(self.charset if self.charset else 'UTF-8') self.app_iter = [page] self.body = page @@ -1001,8 +1035,8 @@ class HTTPInternalServerError(HTTPServerError): code = 500 title = 'Internal Server Error' explanation = ( - 'The server has either erred or is incapable of performing ' - 'the requested operation.') + 'The server has either erred or is incapable of performing ' + 'the requested operation.') class HTTPNotImplemented(HTTPServerError): """ diff --git a/pyramid/tests/test_httpexceptions.py b/pyramid/tests/test_httpexceptions.py index b94ef30e44..6c6e16d551 100644 --- a/pyramid/tests/test_httpexceptions.py +++ b/pyramid/tests/test_httpexceptions.py @@ -28,9 +28,9 @@ def test_status_201(self): self.assertTrue(isinstance(self._callFUT(201), HTTPCreated)) def test_extra_kw(self): - resp = self._callFUT(404, headers=[('abc', 'def')]) + resp = self._callFUT(404, headers=[('abc', 'def')]) self.assertEqual(resp.headers['abc'], 'def') - + class Test_default_exceptionresponse_view(unittest.TestCase): def _callFUT(self, context, request): from pyramid.httpexceptions import default_exceptionresponse_view @@ -129,7 +129,7 @@ def test_ctor_extends_headers(self): def test_ctor_sets_body_template_obj(self): exc = self._makeOne(body_template='${foo}') self.assertEqual( - exc.body_template_obj.substitute({'foo':'foo'}), 'foo') + exc.body_template_obj.substitute({'foo': 'foo'}), 'foo') def test_ctor_with_empty_body(self): cls = self._getTargetSubclass(empty_body=True) @@ -160,7 +160,7 @@ def test_ctor_with_body_sets_default_app_iter_html(self): self.assertTrue(b'200 OK' in body) self.assertTrue(b'explanation' in body) self.assertTrue(b'detail' in body) - + def test_ctor_with_body_sets_default_app_iter_text(self): cls = self._getTargetSubclass() exc = cls('detail') @@ -173,7 +173,7 @@ def test__str__detail(self): exc = self._makeOne() exc.detail = 'abc' self.assertEqual(str(exc), 'abc') - + def test__str__explanation(self): exc = self._makeOne() exc.explanation = 'def' @@ -212,6 +212,9 @@ def test__default_app_iter_no_comment_plain(self): environ = _makeEnviron() start_response = DummyStartResponse() body = list(exc(environ, start_response))[0] + for header in start_response.headerlist: + if header[0] == 'Content-Type': + self.assertEqual(header[1], 'text/plain; charset=UTF-8') self.assertEqual(body, b'200 OK\n\nexplanation\n\n\n\n\n') def test__default_app_iter_with_comment_plain(self): @@ -220,26 +223,78 @@ def test__default_app_iter_with_comment_plain(self): environ = _makeEnviron() start_response = DummyStartResponse() body = list(exc(environ, start_response))[0] + for header in start_response.headerlist: + if header[0] == 'Content-Type': + self.assertEqual(header[1], 'text/plain; charset=UTF-8') self.assertEqual(body, b'200 OK\n\nexplanation\n\n\n\ncomment\n') - + def test__default_app_iter_no_comment_html(self): cls = self._getTargetSubclass() exc = cls() environ = _makeEnviron() start_response = DummyStartResponse() body = list(exc(environ, start_response))[0] + for header in start_response.headerlist: + if header[0] == 'Content-Type': + self.assertEqual(header[1], 'text/plain; charset=UTF-8') self.assertFalse(b'' in body) - def test__default_app_iter_with_comment_html2(self): + def test__default_app_iter_with_comment_html(self): cls = self._getTargetSubclass() exc = cls(comment='comment & comment') environ = _makeEnviron() @@ -248,6 +303,38 @@ def test__default_app_iter_with_comment_html2(self): body = list(exc(environ, start_response))[0] self.assertTrue(b'' in body) + def test__default_app_iter_with_comment_json(self): + cls = self._getTargetSubclass() + exc = cls(comment='comment & comment') + environ = _makeEnviron() + environ['HTTP_ACCEPT'] = 'application/json' + start_response = DummyStartResponse() + body = list(exc(environ, start_response))[0] + import json + retval = json.loads(body.decode('UTF-8')) + self.assertEqual(retval['code'], '200 OK') + self.assertEqual(retval['title'], 'OK') + + def test__default_app_iter_with_custom_json(self): + def json_formatter(status, body, title, environ): + return {'message': body, + 'code': status, + 'title': title, + 'custom': environ['CUSTOM_VARIABLE'] + } + cls = self._getTargetSubclass() + exc = cls(comment='comment', json_formatter=json_formatter) + environ = _makeEnviron() + environ['HTTP_ACCEPT'] = 'application/json' + environ['CUSTOM_VARIABLE'] = 'custom!' + start_response = DummyStartResponse() + body = list(exc(environ, start_response))[0] + import json + retval = json.loads(body.decode('UTF-8')) + self.assertEqual(retval['code'], '200 OK') + self.assertEqual(retval['title'], 'OK') + self.assertEqual(retval['custom'], 'custom!') + def test_custom_body_template(self): cls = self._getTargetSubclass() exc = cls(body_template='${REQUEST_METHOD}') @@ -261,7 +348,8 @@ def test_custom_body_template_with_custom_variable_doesnt_choke(self): exc = cls(body_template='${REQUEST_METHOD}') environ = _makeEnviron() class Choke(object): - def __str__(self): raise ValueError + def __str__(self): # pragma nocover + raise ValueError environ['gardentheory.user'] = Choke() start_response = DummyStartResponse() body = list(exc(environ, start_response))[0] @@ -293,7 +381,7 @@ def _doit(self, content_type): self.assertTrue(bytes_(exc.status) in result) L.append(result) self.assertEqual(len(L), len(status_map)) - + def test_it_plain(self): self._doit('text/plain') @@ -367,12 +455,11 @@ class DummyStartResponse(object): def __call__(self, status, headerlist): self.status = status self.headerlist = headerlist - + def _makeEnviron(**kw): - environ = {'REQUEST_METHOD':'GET', - 'wsgi.url_scheme':'http', - 'SERVER_NAME':'localhost', - 'SERVER_PORT':'80'} + environ = {'REQUEST_METHOD': 'GET', + 'wsgi.url_scheme': 'http', + 'SERVER_NAME': 'localhost', + 'SERVER_PORT': '80'} environ.update(kw) return environ -