From e648965fb6d8c46d8145df3c188a0947390b8435 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 18 Sep 2020 14:49:19 -0400 Subject: [PATCH] Add helpers to implement the desired exception behavior and use them --- src/allmydata/web/common.py | 100 ++++++++++++++++++++++++--------- src/allmydata/web/directory.py | 13 ++++- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 03db8463b..ac748c237 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -204,33 +204,52 @@ def should_create_intermediate_directories(req): t not in ("delete", "rename", "rename-form", "check")) def humanize_failure(f): - # return text, responsecode - if f.check(EmptyPathnameComponentError): + """ + Create an human-oriented description of a failure along with some HTTP + metadata. + + :param Failure f: The failure to describe. + + :return (bytes, int): A tuple of some prose and an HTTP code describing + the failure. + """ + return humanize_exception(f.value) + + +def humanize_exception(exc): + """ + Like ``humanize_failure`` but for an exception. + + :param Exception exc: The exception to describe. + + :return: See ``humanize_failure``. + """ + if isinstance(exc, EmptyPathnameComponentError): return ("The webapi does not allow empty pathname components, " "i.e. a double slash", http.BAD_REQUEST) - if f.check(ExistingChildError): + if isinstance(exc, ExistingChildError): return ("There was already a child by that name, and you asked me " "to not replace it.", http.CONFLICT) - if f.check(NoSuchChildError): - quoted_name = quote_output(f.value.args[0], encoding="utf-8", quotemarks=False) + if isinstance(exc, NoSuchChildError): + quoted_name = quote_output(exc.args[0], encoding="utf-8", quotemarks=False) return ("No such child: %s" % quoted_name, http.NOT_FOUND) - if f.check(NotEnoughSharesError): + if isinstance(exc, NotEnoughSharesError): t = ("NotEnoughSharesError: This indicates that some " "servers were unavailable, or that shares have been " "lost to server departure, hard drive failure, or disk " "corruption. You should perform a filecheck on " "this object to learn more.\n\nThe full error message is:\n" - "%s") % str(f.value) + "%s") % str(exc) return (t, http.GONE) - if f.check(NoSharesError): + if isinstance(exc, NoSharesError): t = ("NoSharesError: no shares could be found. " "Zero shares usually indicates a corrupt URI, or that " "no servers were connected, but it might also indicate " "severe corruption. You should perform a filecheck on " "this object to learn more.\n\nThe full error message is:\n" - "%s") % str(f.value) + "%s") % str(exc) return (t, http.GONE) - if f.check(UnrecoverableFileError): + if isinstance(exc, UnrecoverableFileError): t = ("UnrecoverableFileError: the directory (or mutable file) could " "not be retrieved, because there were insufficient good shares. " "This might indicate that no servers were connected, " @@ -239,9 +258,9 @@ def humanize_failure(f): "failure, or disk corruption. You should perform a filecheck on " "this object to learn more.") return (t, http.GONE) - if f.check(MustNotBeUnknownRWError): - quoted_name = quote_output(f.value.args[1], encoding="utf-8") - immutable = f.value.args[2] + if isinstance(exc, MustNotBeUnknownRWError): + quoted_name = quote_output(exc.args[1], encoding="utf-8") + immutable = exc.args[2] if immutable: t = ("MustNotBeUnknownRWError: an operation to add a child named " "%s to a directory was given an unknown cap in a write slot.\n" @@ -261,29 +280,30 @@ def humanize_failure(f): "writecap in the write slot if desired, would also work in this " "case.") % quoted_name return (t, http.BAD_REQUEST) - if f.check(MustBeDeepImmutableError): - quoted_name = quote_output(f.value.args[1], encoding="utf-8") + if isinstance(exc, MustBeDeepImmutableError): + quoted_name = quote_output(exc.args[1], encoding="utf-8") t = ("MustBeDeepImmutableError: a cap passed to this operation for " "the child named %s, needed to be immutable but was not. Either " "the cap is being added to an immutable directory, or it was " "originally retrieved from an immutable directory as an unknown " "cap.") % quoted_name return (t, http.BAD_REQUEST) - if f.check(MustBeReadonlyError): - quoted_name = quote_output(f.value.args[1], encoding="utf-8") + if isinstance(exc, MustBeReadonlyError): + quoted_name = quote_output(exc.args[1], encoding="utf-8") t = ("MustBeReadonlyError: a cap passed to this operation for " "the child named '%s', needed to be read-only but was not. " "The cap is being passed in a read slot (ro_uri), or was retrieved " "from a read slot as an unknown cap.") % quoted_name return (t, http.BAD_REQUEST) - if f.check(blacklist.FileProhibited): - t = "Access Prohibited: %s" % quote_output(f.value.reason, encoding="utf-8", quotemarks=False) + if isinstance(exc, blacklist.FileProhibited): + t = "Access Prohibited: %s" % quote_output(exc.reason, encoding="utf-8", quotemarks=False) return (t, http.FORBIDDEN) - if f.check(WebError): - return (f.value.text, f.value.code) - if f.check(FileTooLargeError): - return (f.getTraceback(), http.REQUEST_ENTITY_TOO_LARGE) - return (str(f), None) + if isinstance(exc, WebError): + return (exc.text, exc.code) + if isinstance(exc, FileTooLargeError): + return ("FileTooLargeError: %s" % (exc,), http.REQUEST_ENTITY_TOO_LARGE) + return (str(exc), None) + class MyExceptionHandler(appserver.DefaultExceptionHandler, object): def simple(self, ctx, text, code=http.BAD_REQUEST): @@ -481,8 +501,38 @@ class TokenOnlyWebApi(resource.Resource, object): req.setResponseCode(e.code) return json.dumps({"error": e.text}) except Exception as e: - message, code = humanize_failure(Failure()) + message, code = humanize_exception(e) req.setResponseCode(500 if code is None else code) return json.dumps({"error": message}) else: raise WebError("'%s' invalid type for 't' arg" % (t,), http.BAD_REQUEST) + + +def exception_to_child(f): + """ + Decorate ``getChild`` method with exception handling behavior to render an + error page reflecting the exception. + """ + @wraps(f) + def g(self, name, req): + try: + return m(self, name, req) + except Exception as e: + description, status = humanize_exception(e) + return ErrorPage(status, "Error", description) + return g + + +def render_exception(m): + """ + Decorate a ``render_*`` method with exception handling behavior to render + an error page reflecting the exception. + """ + @wraps(m) + def g(self, request): + try: + return m(self, request) + except Exception as e: + description, status = humanize_exception(e) + return ErrorPage(status, "Error", description).render(request) + return g diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index cabcd023e..3ad935bab 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -2,6 +2,7 @@ import json import urllib from datetime import timedelta +from functools import wraps from zope.interface import implementer from twisted.internet import defer @@ -48,6 +49,7 @@ from allmydata.web.common import ( parse_replace_arg, should_create_intermediate_directories, humanize_failure, + humanize_exception, convert_children_json, get_format, get_mutable_type, @@ -55,6 +57,8 @@ from allmydata.web.common import ( render_time, MultiFormatResource, SlotsSequenceElement, + exception_to_child, + render_exception, ) from allmydata.web.filenode import ReplaceMeMixin, \ FileNodeHandler, PlaceHolderNodeHandler @@ -94,6 +98,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): self.name = name self._operations = client.get_web_service().get_operations() + @exception_to_child def getChild(self, name, req): """ Dynamically create a child for the given request and name @@ -113,9 +118,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # we will follow suit. for segment in req.prepath: if not segment: - raise EmptyPathnameComponentError( - u"The webapi does not allow empty pathname components", - ) + raise EmptyPathnameComponentError() d = self.node.get(name) d.addBoth(self._got_child, req, name) @@ -210,6 +213,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): d.addCallback(lambda res: self.node.get_uri()) return d + @render_exception def render_GET(self, req): # This is where all of the directory-related ?t=* code goes. t = get_arg(req, "t", "").strip() @@ -248,6 +252,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): raise WebError("GET directory: bad t=%s" % t) + @render_exception def render_PUT(self, req): t = get_arg(req, "t", "").strip() replace = parse_replace_arg(get_arg(req, "replace", "true")) @@ -267,6 +272,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): raise WebError("PUT to a directory") + @render_exception def render_POST(self, req): t = get_arg(req, "t", "").strip() @@ -1458,6 +1464,7 @@ class UnknownNodeHandler(Resource, object): self.parentnode = parentnode self.name = name + @render_exception def render_GET(self, req): t = get_arg(req, "t", "").strip() if t == "info":