From e3f0e3ede5849b0a6b548aaacb01eb2f126a4070 Mon Sep 17 00:00:00 2001 From: David Glick Date: Wed, 1 Feb 2017 00:08:36 +0100 Subject: [PATCH] render Unauthorized and Redirect exceptions in HTTPExceptionHandler middleware, not in WSGIPublisher --- buildout.cfg | 1 + sources.cfg | 2 +- src/Testing/ZopeTestCase/functional.py | 4 +++ .../zopedoctest/FunctionalDocTest.txt | 1 - src/ZPublisher/HTTPResponse.py | 17 +++++-------- src/ZPublisher/WSGIPublisher.py | 25 +++++++++++-------- src/ZPublisher/tests/test_WSGIPublisher.py | 25 ++++++++----------- src/ZPublisher/tests/test_pubevents.py | 1 + 8 files changed, 37 insertions(+), 39 deletions(-) diff --git a/buildout.cfg b/buildout.cfg index dcbb250fab..23ece856cd 100644 --- a/buildout.cfg +++ b/buildout.cfg @@ -19,6 +19,7 @@ parts = requirements sources-dir = develop auto-checkout = + zExceptions [test] diff --git a/sources.cfg b/sources.cfg index 84fd1b69ce..d54cc2d0a2 100644 --- a/sources.cfg +++ b/sources.cfg @@ -14,7 +14,7 @@ five.globalrequest = git ${remotes:github}/five.globalrequest pushurl=${remotes: MultiMapping = git ${remotes:github}/MultiMapping pushurl=${remotes:github_push}/MultiMapping Persistence = git ${remotes:github}/Persistence pushurl=${remotes:github_push}/Persistence RestrictedPython = git ${remotes:github}/RestrictedPython pushurl=${remotes:github_push}/RestrictedPython -zExceptions = git ${remotes:github}/zExceptions pushurl=${remotes:github_push}/zExceptions +zExceptions = git ${remotes:github}/zExceptions pushurl=${remotes:github_push}/zExceptions branch=davisagli-exception-headers zope.globalrequest = git ${remotes:github}/zope.globalrequest pushurl=${remotes:github_push}/zope.globalrequest # Optional dependencies diff --git a/src/Testing/ZopeTestCase/functional.py b/src/Testing/ZopeTestCase/functional.py index c468c583b5..264ee8b798 100644 --- a/src/Testing/ZopeTestCase/functional.py +++ b/src/Testing/ZopeTestCase/functional.py @@ -103,6 +103,10 @@ def publish(self, path, basic=None, env=None, extra=None, wsgi_headers = BytesIO() def start_response(status, headers): + response.setStatus(status.split()[0]) + for name, value in headers: + response.setHeader(name, value) + wsgi_headers.write('HTTP/1.1 %s\r\n' % status) headers = '\r\n'.join([': '.join(x) for x in headers]) wsgi_headers.write(headers) diff --git a/src/Testing/ZopeTestCase/zopedoctest/FunctionalDocTest.txt b/src/Testing/ZopeTestCase/zopedoctest/FunctionalDocTest.txt index c14fa8bf27..81ae213339 100644 --- a/src/Testing/ZopeTestCase/zopedoctest/FunctionalDocTest.txt +++ b/src/Testing/ZopeTestCase/zopedoctest/FunctionalDocTest.txt @@ -99,7 +99,6 @@ Test Unauthorized ... GET /test_folder_1_/index_html HTTP/1.1 ... """, handle_errors=True)) HTTP/1.1 401 Unauthorized - ... WWW-Authenticate: basic realm=... Test Basic Authentication diff --git a/src/ZPublisher/HTTPResponse.py b/src/ZPublisher/HTTPResponse.py index 9867871dec..b100de1dba 100644 --- a/src/ZPublisher/HTTPResponse.py +++ b/src/ZPublisher/HTTPResponse.py @@ -998,17 +998,14 @@ def badRequestError(self, name): 'and try the request again.

' % name) raise exc - def _unauthorized(self, exc=None): - # This should be handled by zExceptions - status = exc.getStatus() if exc is not None else 401 - self.setStatus(status) + def _unauthorized(self, exc): if self.realm: - self.setHeader('WWW-Authenticate', - 'basic realm="%s"' % self.realm, 1) + exc.setHeader('WWW-Authenticate', 'basic realm="%s"' % self.realm) def unauthorized(self): - exc = Unauthorized() - exc.title = 'You are not authorized to access this resource.' + message = 'You are not authorized to access this resource.' + exc = Unauthorized(message) + exc.title = message if self.debug_mode: if self._auth: exc.detail = 'Username and password are not correct.' @@ -1017,9 +1014,7 @@ def unauthorized(self): raise exc def _redirect(self, exc): - # This should be handled by zExceptions - self.setStatus(exc.getStatus()) - self.setHeader('Location', str(exc)) + exc.setHeader('Location', str(exc)) def redirect(self, location, status=302, lock=0): """Cause a redirection.""" diff --git a/src/ZPublisher/WSGIPublisher.py b/src/ZPublisher/WSGIPublisher.py index ffeda09a96..f52e1b8ef3 100644 --- a/src/ZPublisher/WSGIPublisher.py +++ b/src/ZPublisher/WSGIPublisher.py @@ -174,17 +174,20 @@ def _publish_response(request, response, module_info, _publish=publish): elif isinstance(exc, Unauthorized): response._unauthorized(exc) - view = queryMultiAdapter((exc, request), name=u'index.html') - if view is not None: - parents = request.get('PARENTS') - if parents: - view.__parent__ = parents[0] - response.setStatus(exc.__class__) - response.setBody(view()) - return response - - if isinstance(exc, (HTTPRedirection, Unauthorized)): - return response + # Render exception views unless we are called by the test browser + # with handleErrors = False + if request.environ.get('wsgi.handleErrors', True): + view = queryMultiAdapter((exc, request), name=u'index.html') + if view is not None: + parents = request.get('PARENTS') + if parents: + view.__parent__ = parents[0] + response.setStatus(exc.__class__) + if hasattr(exc, 'headers'): + for name, value in exc.headers.items(): + response.setHeader(name, value) + response.setBody(view()) + return response raise diff --git a/src/ZPublisher/tests/test_WSGIPublisher.py b/src/ZPublisher/tests/test_WSGIPublisher.py index 97784e989b..4e9c5b4423 100644 --- a/src/ZPublisher/tests/test_WSGIPublisher.py +++ b/src/ZPublisher/tests/test_WSGIPublisher.py @@ -296,32 +296,27 @@ def test_publish_can_return_new_response(self): self.assertEqual(_after1._called_with, ((), {})) self.assertEqual(_after2._called_with, ((), {})) - def test_swallows_Unauthorized(self): + def test_raises_annotated_Unauthorized(self): from zExceptions import Unauthorized environ = self._makeEnviron() start_response = DummyCallable() _publish = DummyCallable() _publish._raise = Unauthorized('TESTING') - app_iter = self._callFUT(environ, start_response, _publish) - self.assertEqual(app_iter, ('', '')) - (status, headers), kw = start_response._called_with - self.assertEqual(status, '401 Unauthorized') - self.assertTrue(('Content-Length', '0') in headers) - self.assertEqual(kw, {}) + try: + self._callFUT(environ, start_response, _publish) + except Exception as e: + self.assertEqual(e.headers['WWW-Authenticate'], 'basic realm="Zope"') - def test_swallows_Redirect(self): + def test_raises_annotated_Redirect(self): from zExceptions import Redirect environ = self._makeEnviron() start_response = DummyCallable() _publish = DummyCallable() _publish._raise = Redirect('/redirect_to') - app_iter = self._callFUT(environ, start_response, _publish) - self.assertEqual(app_iter, ('', '')) - (status, headers), kw = start_response._called_with - self.assertEqual(status, '302 Found') - self.assertTrue(('Location', '/redirect_to') in headers) - self.assertTrue(('Content-Length', '0') in headers) - self.assertEqual(kw, {}) + try: + self._callFUT(environ, start_response, _publish) + except Exception as e: + self.assertEqual(e.headers['Location'], '/redirect_to') def test_response_body_is_file(self): from io import BytesIO diff --git a/src/ZPublisher/tests/test_pubevents.py b/src/ZPublisher/tests/test_pubevents.py index d1ddaea6b3..7fb3022d04 100644 --- a/src/ZPublisher/tests/test_pubevents.py +++ b/src/ZPublisher/tests/test_pubevents.py @@ -180,6 +180,7 @@ class _Request(BaseRequest): response = WSGIResponse() _hacked_path = False args = () + environ = {} def __init__(self, *args, **kw): BaseRequest.__init__(self, *args, **kw)