From 4e0799e16319e03797e915e93db8326a8b302784 Mon Sep 17 00:00:00 2001 From: Fabien Meghazi Date: Wed, 25 Jun 2014 11:47:44 +0200 Subject: [PATCH 1/3] [FIX] _handle_exception() logic amongst requests and ir_http objects Changes in website's ir_http#_handle_exception(): - exception is mandatory, can't be None anymore - we don't touch non website_enabled requests - we don't touch explicits plain responses from parent - logic flow is now easier to read (I hope so) Change in website's ir_http#_dispatch(): - In case of real 404, instead of returning self._handle_exception(), just let parent do the job (so we call super()) --- addons/website/models/ir_http.py | 85 ++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 65773837214..be1e46fe2e2 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -60,7 +60,6 @@ class ir_http(orm.AbstractModel): request.lang = request.context['lang'] = path.pop(1) path = '/'.join(path) or '/' return self.reroute(path) - return self._handle_exception(code=404) return super(ir_http, self)._dispatch() def reroute(self, path): @@ -93,51 +92,61 @@ class ir_http(orm.AbstractModel): if str(arg.id) != placeholder.value and placeholder.value != good_slug: # TODO: properly recompose the url instead of using replace() url = url.replace(placeholder.value, good_slug) - except KeyError: - return self._handle_exception(werkzeug.exceptions.NotFound()) + except KeyError, e: + return self._handle_exception(e, code=404) if url != original_url: werkzeug.exceptions.abort(werkzeug.utils.redirect(url)) - def _handle_exception(self, exception=None, code=500): - try: + def _handle_exception(self, exception, code=500): + is_website_request = getattr(request, 'website_enabled', False) and request.website + if not is_website_request: + # Don't touch non website requests exception handling return super(ir_http, self)._handle_exception(exception) - except Exception: - if getattr(request, 'website_enabled', False) and request.website: - values = dict( - exception=exception, - traceback=traceback.format_exc(exception), - ) - if exception: - code = getattr(exception, 'code', code) - if isinstance(exception, ir_qweb.QWebException): - values.update(qweb_exception=exception) - if isinstance(exception.qweb.get('cause'), openerp.exceptions.AccessError): - code = 403 - if code == 500: - logger.error("500 Internal Server Error:\n\n%s", values['traceback']) - if 'qweb_exception' in values: - view = request.registry.get("ir.ui.view") - views = view._views_get(request.cr, request.uid, exception.qweb['template'], request.context) - to_reset = [v for v in views if v.model_data_id.noupdate is True] - values['views'] = to_reset - elif code == 403: - logger.warn("403 Forbidden:\n\n%s", values['traceback']) + else: + try: + response = super(ir_http, self)._handle_exception(exception) + if isinstance(response, Exception): + exception = response + else: + # if parent excplicitely returns a plain response, then we don't touch it + return response + except Exception, e: + exception = e - values.update( - status_message=werkzeug.http.HTTP_STATUS_CODES[code], - status_code=code, - ) + values = dict( + exception=exception, + traceback=traceback.format_exc(exception), + ) + code = getattr(exception, 'code', code) - if not request.uid: - self._auth_method_public() + if isinstance(exception, ir_qweb.QWebException): + values.update(qweb_exception=exception) + if isinstance(exception.qweb.get('cause'), openerp.exceptions.AccessError): + code = 403 - try: - html = request.website._render('website.%s' % code, values) - except Exception: - html = request.website._render('website.http_error', values) - return werkzeug.wrappers.Response(html, status=code, content_type='text/html;charset=utf-8') + if code == 500: + logger.error("500 Internal Server Error:\n\n%s", values['traceback']) + if 'qweb_exception' in values: + view = request.registry.get("ir.ui.view") + views = view._views_get(request.cr, request.uid, exception.qweb['template'], request.context) + to_reset = [v for v in views if v.model_data_id.noupdate is True] + values['views'] = to_reset + elif code == 403: + logger.warn("403 Forbidden:\n\n%s", values['traceback']) - raise + values.update( + status_message=werkzeug.http.HTTP_STATUS_CODES[code], + status_code=code, + ) + + if not request.uid: + self._auth_method_public() + + try: + html = request.website._render('website.%s' % code, values) + except Exception: + html = request.website._render('website.http_error', values) + return werkzeug.wrappers.Response(html, status=code, content_type='text/html;charset=utf-8') class ModelConverter(ir.ir_http.ModelConverter): def __init__(self, url_map, model=False): From f988d7503537990b31b9e643e3d69ae7e27b1375 Mon Sep 17 00:00:00 2001 From: Fabien Meghazi Date: Wed, 25 Jun 2014 12:20:40 +0200 Subject: [PATCH 2/3] [FIX] check after openerp's AccessErrors in website's _handle_exception --- addons/website/models/ir_http.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index be1e46fe2e2..d16077a3828 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -98,7 +98,7 @@ class ir_http(orm.AbstractModel): werkzeug.exceptions.abort(werkzeug.utils.redirect(url)) def _handle_exception(self, exception, code=500): - is_website_request = getattr(request, 'website_enabled', False) and request.website + is_website_request = bool(getattr(request, 'website_enabled', False) and request.website) if not is_website_request: # Don't touch non website requests exception handling return super(ir_http, self)._handle_exception(exception) @@ -119,6 +119,9 @@ class ir_http(orm.AbstractModel): ) code = getattr(exception, 'code', code) + if isinstance(exception, openerp.exceptions.AccessError): + code = 403 + if isinstance(exception, ir_qweb.QWebException): values.update(qweb_exception=exception) if isinstance(exception.qweb.get('cause'), openerp.exceptions.AccessError): From 141e1b2615ee992b111da7d7a2070f25b5de8318 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 25 Jun 2014 17:41:24 +0200 Subject: [PATCH 3/3] [FIX] http.dispatch: call handle_exception() even w/ session missing/expired This gives JSONRequests a chance to return a proper JSON-RPC result when an HTTPException is raised downstream, instead of returning a plain HTML 404 error. --- openerp/http.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openerp/http.py b/openerp/http.py index d32d00326f7..15565d2f7fa 100644 --- a/openerp/http.py +++ b/openerp/http.py @@ -1118,7 +1118,10 @@ class Root(object): request = self.get_request(httprequest) def _dispatch_nodb(): - func, arguments = self.nodb_routing_map.bind_to_environ(request.httprequest.environ).match() + try: + func, arguments = self.nodb_routing_map.bind_to_environ(request.httprequest.environ).match() + except werkzeug.exceptions.HTTPException, e: + return request._handle_exception(e) request.set_handler(func, arguments, "none") result = request.dispatch() return result