From 18e1c290a7d571d3d8c2d3c253c46d8ed68d68bc Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 2 Oct 2020 10:28:55 -0400 Subject: [PATCH 01/15] Reorganize code so allmydata.web.check_results can import without Nevow being installed. --- src/allmydata/web/check_results.py | 4 +- src/allmydata/web/common.py | 183 +---------------------- src/allmydata/web/common_py3.py | 226 ++++++++++++++++++++++++++++- src/allmydata/web/operations.py | 38 +---- 4 files changed, 234 insertions(+), 217 deletions(-) diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index 7c4723333..fd8891d3c 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -14,7 +14,7 @@ from twisted.web.template import ( renderElement, tags, ) -from allmydata.web.common import ( +from allmydata.web.common_py3 import ( exception_to_child, get_arg, get_root, @@ -22,8 +22,8 @@ from allmydata.web.common import ( WebError, MultiFormatResource, SlotsSequenceElement, + ReloadMixin, ) -from allmydata.web.operations import ReloadMixin from allmydata.interfaces import ( ICheckAndRepairResults, ICheckResults, diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 102e67adc..999256ad3 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -7,39 +7,27 @@ from twisted.web import ( http, resource, server, - template, ) from twisted.python import log from nevow import appserver from nevow.inevow import IRequest -from allmydata import blacklist from allmydata.interfaces import ( - EmptyPathnameComponentError, - ExistingChildError, - FileTooLargeError, - MustBeDeepImmutableError, - MustBeReadonlyError, - MustNotBeUnknownRWError, - NoSharesError, - NoSuchChildError, - NotEnoughSharesError, MDMF_VERSION, SDMF_VERSION, ) -from allmydata.mutable.common import UnrecoverableFileError from allmydata.util.hashutil import timing_safe_compare from allmydata.util.time_format import ( format_delta, format_time, ) from allmydata.util.encodingutil import ( - quote_output, to_bytes, ) # Originally part of this module, so still part of its API: from .common_py3 import ( # noqa: F401 - get_arg, abbreviate_time, MultiFormatResource, WebError, + get_arg, abbreviate_time, MultiFormatResource, WebError, humanize_exception, + SlotsSequenceElement, render_exception, get_root, exception_to_child ) @@ -117,13 +105,6 @@ def parse_offset_arg(offset): return offset -def get_root(ctx_or_req): - req = IRequest(ctx_or_req) - depth = len(req.prepath) + len(req.postpath) - link = "/".join([".."] * depth) - return link - - def convert_children_json(nodemaker, children_json): """I convert the JSON output of GET?t=json into the dict-of-nodes input to both dirnode.create_subdirectory() and @@ -217,94 +198,6 @@ def should_create_intermediate_directories(req): return bool(req.method in ("PUT", "POST") and t not in ("delete", "rename", "rename-form", "check")) -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 isinstance(exc, ExistingChildError): - return ("There was already a child by that name, and you asked me " - "to not replace it.", http.CONFLICT) - 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 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(exc) - return (t, http.GONE) - 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(exc) - return (t, http.GONE) - 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, " - "insufficient servers were connected, the URI was corrupt, or " - "that shares have been lost due to server departure, hard drive " - "failure, or disk corruption. You should perform a filecheck on " - "this object to learn more.") - return (t, http.GONE) - 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" - "If the cap is actually an immutable readcap, then using a " - "webapi server that supports a later version of Tahoe may help.\n\n" - "If you are using the webapi directly, then specifying an immutable " - "readcap in the read slot (ro_uri) of the JSON PROPDICT, and " - "omitting the write slot (rw_uri), would also work in this " - "case.") % quoted_name - else: - t = ("MustNotBeUnknownRWError: an operation to add a child named " - "%s to a directory was given an unknown cap in a write slot.\n" - "Using a webapi server that supports a later version of Tahoe " - "may help.\n\n" - "If you are using the webapi directly, specifying a readcap in " - "the read slot (ro_uri) of the JSON PROPDICT, as well as a " - "writecap in the write slot if desired, would also work in this " - "case.") % quoted_name - return (t, http.BAD_REQUEST) - 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 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 isinstance(exc, blacklist.FileProhibited): - t = "Access Prohibited: %s" % quote_output(exc.reason, encoding="utf-8", quotemarks=False) - return (t, http.FORBIDDEN) - 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) - def humanize_failure(f): """ @@ -368,49 +261,6 @@ class NeedOperationHandleError(WebError): pass -class SlotsSequenceElement(template.Element): - """ - ``SlotsSequenceElement` is a minimal port of nevow's sequence renderer for - twisted.web.template. - - Tags passed in to be templated will have two renderers available: ``item`` - and ``tag``. - """ - - def __init__(self, tag, seq): - self.loader = template.TagLoader(tag) - self.seq = seq - - @template.renderer - def header(self, request, tag): - return tag - - @template.renderer - def item(self, request, tag): - """ - A template renderer for each sequence item. - - ``tag`` will be cloned for each item in the sequence provided, and its - slots filled from the sequence item. Each item must be dict-like enough - for ``tag.fillSlots(**item)``. Each cloned tag will be siblings with no - separator beween them. - """ - for item in self.seq: - yield tag.clone(deep=False).fillSlots(**item) - - @template.renderer - def empty(self, request, tag): - """ - A template renderer for empty sequences. - - This renderer will either return ``tag`` unmodified if the provided - sequence has no items, or return the empty string if there are any - items. - """ - if len(self.seq) > 0: - return u'' - else: - return tag class TokenOnlyWebApi(resource.Resource, object): @@ -465,32 +315,3 @@ class TokenOnlyWebApi(resource.Resource, object): 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 f(self, name, req) - except Exception as e: - description, status = humanize_exception(e) - return resource.ErrorPage(status, "Error", description) - return g - - -def render_exception(f): - """ - Decorate a ``render_*`` method with exception handling behavior to render - an error page reflecting the exception. - """ - @wraps(f) - def g(self, request): - try: - return f(self, request) - except Exception as e: - description, status = humanize_exception(e) - return resource.ErrorPage(status, "Error", description).render(request) - return g diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index 73130cbab..bce1494a6 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -6,15 +6,32 @@ Can eventually be merged back into allmydata.web.common. from future.utils import PY2 +from functools import wraps + if PY2: from nevow.inevow import IRequest as INevowRequest else: INevowRequest = None -from twisted.web import resource, http +from twisted.web import resource, http, template +from twisted.web.template import tags as T from twisted.web.iweb import IRequest +from allmydata import blacklist +from allmydata.interfaces import ( + EmptyPathnameComponentError, + ExistingChildError, + FileTooLargeError, + MustBeDeepImmutableError, + MustBeReadonlyError, + MustNotBeUnknownRWError, + NoSharesError, + NoSuchChildError, + NotEnoughSharesError, +) +from allmydata.mutable.common import UnrecoverableFileError from allmydata.util import abbreviate +from allmydata.util.encodingutil import quote_output class WebError(Exception): @@ -120,3 +137,210 @@ def abbreviate_time(data): if s >= 0.001: return "%.1fms" % (1000*s) return "%.0fus" % (1000000*s) + + +def render_exception(f): + """ + Decorate a ``render_*`` method with exception handling behavior to render + an error page reflecting the exception. + """ + @wraps(f) + def g(self, request): + try: + return f(self, request) + except Exception as e: + description, status = humanize_exception(e) + return resource.ErrorPage(status, "Error", description).render(request) + return g + + +class SlotsSequenceElement(template.Element): + """ + ``SlotsSequenceElement` is a minimal port of nevow's sequence renderer for + twisted.web.template. + + Tags passed in to be templated will have two renderers available: ``item`` + and ``tag``. + """ + + def __init__(self, tag, seq): + self.loader = template.TagLoader(tag) + self.seq = seq + + @template.renderer + def header(self, request, tag): + return tag + + @template.renderer + def item(self, request, tag): + """ + A template renderer for each sequence item. + + ``tag`` will be cloned for each item in the sequence provided, and its + slots filled from the sequence item. Each item must be dict-like enough + for ``tag.fillSlots(**item)``. Each cloned tag will be siblings with no + separator beween them. + """ + for item in self.seq: + yield tag.clone(deep=False).fillSlots(**item) + + @template.renderer + def empty(self, request, tag): + """ + A template renderer for empty sequences. + + This renderer will either return ``tag`` unmodified if the provided + sequence has no items, or return the empty string if there are any + items. + """ + if len(self.seq) > 0: + return u'' + else: + return tag + + +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 isinstance(exc, ExistingChildError): + return ("There was already a child by that name, and you asked me " + "to not replace it.", http.CONFLICT) + 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 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(exc) + return (t, http.GONE) + 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(exc) + return (t, http.GONE) + 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, " + "insufficient servers were connected, the URI was corrupt, or " + "that shares have been lost due to server departure, hard drive " + "failure, or disk corruption. You should perform a filecheck on " + "this object to learn more.") + return (t, http.GONE) + 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" + "If the cap is actually an immutable readcap, then using a " + "webapi server that supports a later version of Tahoe may help.\n\n" + "If you are using the webapi directly, then specifying an immutable " + "readcap in the read slot (ro_uri) of the JSON PROPDICT, and " + "omitting the write slot (rw_uri), would also work in this " + "case.") % quoted_name + else: + t = ("MustNotBeUnknownRWError: an operation to add a child named " + "%s to a directory was given an unknown cap in a write slot.\n" + "Using a webapi server that supports a later version of Tahoe " + "may help.\n\n" + "If you are using the webapi directly, specifying a readcap in " + "the read slot (ro_uri) of the JSON PROPDICT, as well as a " + "writecap in the write slot if desired, would also work in this " + "case.") % quoted_name + return (t, http.BAD_REQUEST) + 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 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 isinstance(exc, blacklist.FileProhibited): + t = "Access Prohibited: %s" % quote_output(exc.reason, encoding="utf-8", quotemarks=False) + return (t, http.FORBIDDEN) + 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) + + +def get_root(ctx_or_req): + if PY2: + req = INevowRequest(ctx_or_req) + else: + req = IRequest(ctx_or_req) + depth = len(req.prepath) + len(req.postpath) + link = "/".join([".."] * depth) + return link + + +class ReloadMixin(object): + REFRESH_TIME = 1*60 + + @template.renderer + def refresh(self, req, tag): + if self.monitor.is_finished(): + return "" + # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) + # but I can't tell if he's joking or not + tag.attributes["http-equiv"] = "refresh" + tag.attributes["content"] = str(self.REFRESH_TIME) + return tag + + @template.renderer + def reload(self, req, tag): + if self.monitor.is_finished(): + return "" + # url.gethere would break a proxy, so the correct thing to do is + # req.path[-1] + queryargs + ophandle = req.prepath[-1] + reload_target = ophandle + "?output=html" + cancel_target = ophandle + "?t=cancel" + cancel_button = T.form(T.input(type="submit", value="Cancel"), + action=cancel_target, + method="POST", + enctype="multipart/form-data",) + + return (T.h2("Operation still running: ", + T.a("Reload", href=reload_target), + ), + cancel_button,) + + +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 f(self, name, req) + except Exception as e: + description, status = humanize_exception(e) + return resource.ErrorPage(status, "Error", description) + return g diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 21c2ec7ef..6c1fb190f 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -20,6 +20,11 @@ from allmydata.web.common import ( exception_to_child, ) +# Originally part of this module, so still part of its API: +from .common_py3 import ( # noqa: F401 + ReloadMixin, +) + MINUTE = 60 HOUR = 60*MINUTE DAY = 24*HOUR @@ -142,36 +147,3 @@ class OphandleTable(resource.Resource, service.Service): self.timers[ophandle].cancel() self.timers.pop(ophandle, None) self.handles.pop(ophandle, None) - - -class ReloadMixin(object): - REFRESH_TIME = 1*MINUTE - - @renderer - def refresh(self, req, tag): - if self.monitor.is_finished(): - return "" - # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) - # but I can't tell if he's joking or not - tag.attributes["http-equiv"] = "refresh" - tag.attributes["content"] = str(self.REFRESH_TIME) - return tag - - @renderer - def reload(self, req, tag): - if self.monitor.is_finished(): - return "" - # url.gethere would break a proxy, so the correct thing to do is - # req.path[-1] + queryargs - ophandle = req.prepath[-1] - reload_target = ophandle + "?output=html" - cancel_target = ophandle + "?t=cancel" - cancel_button = T.form(T.input(type="submit", value="Cancel"), - action=cancel_target, - method="POST", - enctype="multipart/form-data",) - - return (T.h2("Operation still running: ", - T.a("Reload", href=reload_target), - ), - cancel_button,) From ade4a5a543c18ad49dce3e327c7c8d03aaafda11 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 2 Oct 2020 10:42:05 -0400 Subject: [PATCH 02/15] As far as I can tell, none of the IServer implementations actually implement comparison methods. --- src/allmydata/immutable/filenode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py index 670989c3a..3db3a2432 100644 --- a/src/allmydata/immutable/filenode.py +++ b/src/allmydata/immutable/filenode.py @@ -139,7 +139,7 @@ class CiphertextFileNode(object): for server in servers: sm.add(shnum, server) servers_responding.add(server) - servers_responding = sorted(servers_responding) + servers_responding = sorted(servers_responding, key=lambda o: id(o)) good_hosts = len(reduce(set.union, sm.values(), set())) is_healthy = bool(len(sm) >= verifycap.total_shares) From 1088e5368de705e710e6b99fe96c4e03268a17dc Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 2 Oct 2020 10:42:44 -0400 Subject: [PATCH 03/15] A little progress on passing tests. --- src/allmydata/immutable/checker.py | 2 +- src/allmydata/test/test_checker.py | 47 ++++++++++++++++++------------ 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py index ce533b969..2bed90e1c 100644 --- a/src/allmydata/immutable/checker.py +++ b/src/allmydata/immutable/checker.py @@ -616,7 +616,7 @@ class Checker(log.PrefixingLogMixin): d.addCallback(_got_ueb) def _discard_result(r): - assert isinstance(r, str), r + assert isinstance(r, bytes), r # to free up the RAM return None diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 882356aeb..3ae1cc822 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -1,3 +1,5 @@ +from future.utils import PY2 +from past.builtins import unicode import json import os.path, shutil @@ -7,7 +9,14 @@ from bs4 import BeautifulSoup from twisted.trial import unittest from twisted.internet import defer -from nevow.inevow import IRequest +# We need to use `nevow.inevow.IRequest` for now for compatibility +# with the code in web/common.py. Once nevow bits are gone from +# web/common.py, we can use `twisted.web.iweb.IRequest` here. +if PY2: + from nevow.inevow import IRequest +else: + from twisted.web.iweb import IRequest + from zope.interface import implementer from twisted.web.server import Request from twisted.web.test.requesthelper import DummyChannel @@ -102,7 +111,7 @@ class FakeCheckResults(object): def get_corrupt_shares(self): # returns a list of (IServer, storage_index, sharenum) - return [(FakeServer(), "", 0)] + return [(FakeServer(), b"", 0)] @implementer(ICheckAndRepairResults) @@ -141,13 +150,13 @@ class WebResultsRendering(unittest.TestCase): sb = StorageFarmBroker(True, None, EMPTY_CLIENT_CONFIG) # s.get_name() (the "short description") will be "v0-00000000". # s.get_longname() will include the -long suffix. - servers = [("v0-00000000-long", "\x00"*20, "peer-0"), - ("v0-ffffffff-long", "\xff"*20, "peer-f"), - ("v0-11111111-long", "\x11"*20, "peer-11")] + servers = [(b"v0-00000000-long", b"\x00"*20, "peer-0"), + (b"v0-ffffffff-long", b"\xff"*20, "peer-f"), + (b"v0-11111111-long", b"\x11"*20, "peer-11")] for (key_s, binary_tubid, nickname) in servers: server_id = key_s tubid_b32 = base32.b2a(binary_tubid) - furl = "pb://%s@nowhere/fake" % tubid_b32 + furl = b"pb://%s@nowhere/fake" % tubid_b32 ann = { "version": 0, "service-name": "storage", "anonymous-storage-FURL": furl, @@ -174,11 +183,11 @@ class WebResultsRendering(unittest.TestCase): lcr = web_check_results.LiteralCheckResultsRendererElement() html = self.render_element(lcr) - self.failUnlessIn("Literal files are always healthy", html) + self.failUnlessIn(b"Literal files are always healthy", html) html = self.render_element(lcr, args={"return_to": ["FOOURL"]}) - self.failUnlessIn("Literal files are always healthy", html) - self.failUnlessIn('Return to file.', html) + self.failUnlessIn(b"Literal files are always healthy", html) + self.failUnlessIn(b'Return to file.', html) c = self.create_fake_client() lcr = web_check_results.LiteralCheckResultsRenderer(c) @@ -196,7 +205,7 @@ class WebResultsRendering(unittest.TestCase): serverid_f = "\xff"*20 server_1 = sb.get_stub_server(serverid_1) server_f = sb.get_stub_server(serverid_f) - u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234) + u = uri.CHKFileURI(b"\x00"*16, b"\x00"*32, 3, 10, 1234) data = { "count_happiness": 8, "count_shares_needed": 3, "count_shares_expected": 9, @@ -301,9 +310,9 @@ class WebResultsRendering(unittest.TestCase): def test_check_and_repair(self): c = self.create_fake_client() sb = c.storage_broker - serverid_1 = "\x00"*20 - serverid_f = "\xff"*20 - u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234) + serverid_1 = b"\x00"*20 + serverid_f = b"\xff"*20 + u = uri.CHKFileURI(b"\x00"*16, b"\x00"*32, 3, 10, 1234) data = { "count_happiness": 5, "count_shares_needed": 3, @@ -419,7 +428,7 @@ class WebResultsRendering(unittest.TestCase): def test_deep_check_renderer(self): - status = check_results.DeepCheckResults("fake-root-si") + status = check_results.DeepCheckResults(b"fake-root-si") status.add_check( FakeCheckResults("", False, False), (u"fake", u"unhealthy", u"unrecoverable") @@ -512,7 +521,7 @@ class WebResultsRendering(unittest.TestCase): ) def test_deep_check_and_repair_renderer(self): - status = check_results.DeepCheckAndRepairResults("") + status = check_results.DeepCheckAndRepairResults(b"") status.add_check_and_repair( FakeCheckAndRepairResults("attempted/success", True, True), @@ -676,8 +685,8 @@ class BalancingAct(GridTestMixin, unittest.TestCase): c0.encoding_params['n'] = 4 c0.encoding_params['k'] = 3 - DATA = "data" * 100 - d = c0.upload(Data(DATA, convergence="")) + DATA = b"data" * 100 + d = c0.upload(Data(DATA, convergence=b"")) def _stash_immutable(ur): self.imm = c0.create_node_from_uri(ur.get_uri()) self.uri = self.imm.get_uri() @@ -834,8 +843,8 @@ class TooParallel(GridTestMixin, unittest.TestCase): "max_segment_size": 5, } self.uris = {} - DATA = "data" * 100 # 400/5 = 80 blocks - return self.c0.upload(Data(DATA, convergence="")) + DATA = b"data" * 100 # 400/5 = 80 blocks + return self.c0.upload(Data(DATA, convergence=b"")) d.addCallback(_start) def _do_check(ur): n = self.c0.create_node_from_uri(ur.get_uri()) From f8f8329d70b4ec0bf5b9c671a79a92760ba94ba0 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 2 Oct 2020 10:48:54 -0400 Subject: [PATCH 04/15] More passing tests on Python 3. --- src/allmydata/immutable/filenode.py | 2 +- src/allmydata/mutable/publish.py | 48 ++++++++++++++--------------- src/allmydata/test/test_checker.py | 6 ++-- src/allmydata/web/common_py3.py | 6 ++-- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py index 3db3a2432..6a90b8c21 100644 --- a/src/allmydata/immutable/filenode.py +++ b/src/allmydata/immutable/filenode.py @@ -1,4 +1,4 @@ - +from functools import reduce import binascii from time import time as now diff --git a/src/allmydata/mutable/publish.py b/src/allmydata/mutable/publish.py index 12ad3d992..d4b6f4dea 100644 --- a/src/allmydata/mutable/publish.py +++ b/src/allmydata/mutable/publish.py @@ -1,5 +1,5 @@ import os, time -from six.moves import cStringIO as StringIO +from io import BytesIO from itertools import count from zope.interface import implementer from twisted.internet import defer @@ -46,7 +46,7 @@ class PublishStatus(object): self.size = None self.status = "Not started" self.progress = 0.0 - self.counter = self.statusid_counter.next() + self.counter = next(self.statusid_counter) self.started = time.time() def add_per_server_time(self, server, elapsed): @@ -305,7 +305,7 @@ class Publish(object): # Our update process fetched these for us. We need to update # them in place as publishing happens. self.blockhashes = {} # (shnum, [blochashes]) - for (i, bht) in blockhashes.iteritems(): + for (i, bht) in blockhashes.items(): # We need to extract the leaves from our old hash tree. old_segcount = mathutil.div_ceil(version[4], version[3]) @@ -313,7 +313,7 @@ class Publish(object): bht = dict(enumerate(bht)) h.set_hashes(bht) leaves = h[h.get_leaf_index(0):] - for j in xrange(self.num_segments - len(leaves)): + for j in range(self.num_segments - len(leaves)): leaves.append(None) assert len(leaves) >= self.num_segments @@ -509,10 +509,10 @@ class Publish(object): # This will eventually hold the block hash chain for each share # that we publish. We define it this way so that empty publishes # will still have something to write to the remote slot. - self.blockhashes = dict([(i, []) for i in xrange(self.total_shares)]) - for i in xrange(self.total_shares): + self.blockhashes = dict([(i, []) for i in range(self.total_shares)]) + for i in range(self.total_shares): blocks = self.blockhashes[i] - for j in xrange(self.num_segments): + for j in range(self.num_segments): blocks.append(None) self.sharehash_leaves = None # eventually [sharehashes] self.sharehashes = {} # shnum -> [sharehash leaves necessary to @@ -526,7 +526,7 @@ class Publish(object): return self.done_deferred def _get_some_writer(self): - return list(self.writers.values()[0])[0] + return list(list(self.writers.values())[0])[0] def _update_status(self): self._status.set_status("Sending Shares: %d placed out of %d, " @@ -684,7 +684,7 @@ class Publish(object): salt = os.urandom(16) assert self._version == SDMF_VERSION - for shnum, writers in self.writers.iteritems(): + for shnum, writers in self.writers.items(): for writer in writers: writer.put_salt(salt) @@ -704,7 +704,7 @@ class Publish(object): self.log("Pushing segment %d of %d" % (segnum + 1, self.num_segments)) data = self.data.read(segsize) # XXX: This is dumb. Why return a list? - data = "".join(data) + data = b"".join(data) assert len(data) == segsize, len(data) @@ -732,7 +732,7 @@ class Publish(object): for i in range(len(crypttext_pieces)): offset = i * piece_size piece = crypttext[offset:offset+piece_size] - piece = piece + "\x00"*(piece_size - len(piece)) # padding + piece = piece + b"\x00"*(piece_size - len(piece)) # padding crypttext_pieces[i] = piece assert len(piece) == piece_size d = fec.encode(crypttext_pieces) @@ -751,7 +751,7 @@ class Publish(object): results, salt = encoded_and_salt shares, shareids = results self._status.set_status("Pushing segment") - for i in xrange(len(shares)): + for i in range(len(shares)): sharedata = shares[i] shareid = shareids[i] if self._version == MDMF_VERSION: @@ -786,7 +786,7 @@ class Publish(object): def push_encprivkey(self): encprivkey = self._encprivkey self._status.set_status("Pushing encrypted private key") - for shnum, writers in self.writers.iteritems(): + for shnum, writers in self.writers.items(): for writer in writers: writer.put_encprivkey(encprivkey) @@ -794,7 +794,7 @@ class Publish(object): def push_blockhashes(self): self.sharehash_leaves = [None] * len(self.blockhashes) self._status.set_status("Building and pushing block hash tree") - for shnum, blockhashes in self.blockhashes.iteritems(): + for shnum, blockhashes in self.blockhashes.items(): t = hashtree.HashTree(blockhashes) self.blockhashes[shnum] = list(t) # set the leaf for future use. @@ -808,7 +808,7 @@ class Publish(object): def push_sharehashes(self): self._status.set_status("Building and pushing share hash chain") share_hash_tree = hashtree.HashTree(self.sharehash_leaves) - for shnum in xrange(len(self.sharehash_leaves)): + for shnum in range(len(self.sharehash_leaves)): needed_indices = share_hash_tree.needed_hashes(shnum) self.sharehashes[shnum] = dict( [ (i, share_hash_tree[i]) for i in needed_indices] ) @@ -824,7 +824,7 @@ class Publish(object): # - Get the checkstring of the resulting layout; sign that. # - Push the signature self._status.set_status("Pushing root hashes and signature") - for shnum in xrange(self.total_shares): + for shnum in range(self.total_shares): writers = self.writers[shnum] for writer in writers: writer.put_root_hash(self.root_hash) @@ -852,7 +852,7 @@ class Publish(object): signable = self._get_some_writer().get_signable() self.signature = rsa.sign_data(self._privkey, signable) - for (shnum, writers) in self.writers.iteritems(): + for (shnum, writers) in self.writers.items(): for writer in writers: writer.put_signature(self.signature) self._status.timings['sign'] = time.time() - started @@ -867,7 +867,7 @@ class Publish(object): ds = [] verification_key = rsa.der_string_from_verifying_key(self._pubkey) - for (shnum, writers) in self.writers.copy().iteritems(): + for (shnum, writers) in self.writers.copy().items(): for writer in writers: writer.put_verification_key(verification_key) self.num_outstanding += 1 @@ -1003,7 +1003,7 @@ class Publish(object): # TODO: Precompute this. shares = [] - for shnum, writers in self.writers.iteritems(): + for shnum, writers in self.writers.items(): shares.extend([x.shnum for x in writers if x.server == server]) known_shnums = set(shares) surprise_shares -= known_shnums @@ -1198,7 +1198,7 @@ class Publish(object): class MutableFileHandle(object): """ I am a mutable uploadable built around a filehandle-like object, - usually either a StringIO instance or a handle to an actual file. + usually either a BytesIO instance or a handle to an actual file. """ def __init__(self, filehandle): @@ -1268,14 +1268,14 @@ class MutableFileHandle(object): class MutableData(MutableFileHandle): """ I am a mutable uploadable built around a string, which I then cast - into a StringIO and treat as a filehandle. + into a BytesIO and treat as a filehandle. """ def __init__(self, s): # Take a string and return a file-like uploadable. - assert isinstance(s, str) + assert isinstance(s, bytes) - MutableFileHandle.__init__(self, StringIO(s)) + MutableFileHandle.__init__(self, BytesIO(s)) @implementer(IMutableUploadable) @@ -1361,7 +1361,7 @@ class TransformingUploadable(object): self.log("reading %d bytes of new data" % length) new_data = self._newdata.read(length) - new_data = "".join(new_data) + new_data = b"".join(new_data) self._read_marker += len(old_start_data + new_data + old_end_data) diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 3ae1cc822..0a10ea5d4 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -751,13 +751,13 @@ class AddLease(GridTestMixin, unittest.TestCase): c0 = self.g.clients[0] c0.encoding_params['happy'] = 1 self.uris = {} - DATA = "data" * 100 - d = c0.upload(Data(DATA, convergence="")) + DATA = b"data" * 100 + d = c0.upload(Data(DATA, convergence=b"")) def _stash_immutable(ur): self.imm = c0.create_node_from_uri(ur.get_uri()) d.addCallback(_stash_immutable) d.addCallback(lambda ign: - c0.create_mutable_file(MutableData("contents"))) + c0.create_mutable_file(MutableData(b"contents"))) def _stash_mutable(node): self.mut = node d.addCallback(_stash_mutable) diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index bce1494a6..a94401b45 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -314,12 +314,12 @@ class ReloadMixin(object): @template.renderer def reload(self, req, tag): if self.monitor.is_finished(): - return "" + return b"" # url.gethere would break a proxy, so the correct thing to do is # req.path[-1] + queryargs ophandle = req.prepath[-1] - reload_target = ophandle + "?output=html" - cancel_target = ophandle + "?t=cancel" + reload_target = ophandle + b"?output=html" + cancel_target = ophandle + b"?t=cancel" cancel_button = T.form(T.input(type="submit", value="Cancel"), action=cancel_target, method="POST", From 963f9ba94bf29e95d43efbedd99db7d549534295 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 2 Oct 2020 11:01:24 -0400 Subject: [PATCH 05/15] Closer to passing tests. --- src/allmydata/test/test_checker.py | 20 ++++++++++---------- src/allmydata/web/check_results.py | 5 +++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 0a10ea5d4..0d5877abe 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -201,8 +201,8 @@ class WebResultsRendering(unittest.TestCase): def test_check(self): c = self.create_fake_client() sb = c.storage_broker - serverid_1 = "\x00"*20 - serverid_f = "\xff"*20 + serverid_1 = b"\x00"*20 + serverid_f = b"\xff"*20 server_1 = sb.get_stub_server(serverid_1) server_f = sb.get_stub_server(serverid_f) u = uri.CHKFileURI(b"\x00"*16, b"\x00"*32, 3, 10, 1234) @@ -269,7 +269,7 @@ class WebResultsRendering(unittest.TestCase): self.failUnlessIn("Not Recoverable! : rather dead", s) html = self.render_element(w, args={"return_to": ["FOOURL"]}) - self.failUnlessIn('Return to file/directory.', + self.failUnlessIn(b'Return to file/directory.', html) w = web_check_results.CheckResultsRenderer(c, cr) @@ -430,19 +430,19 @@ class WebResultsRendering(unittest.TestCase): def test_deep_check_renderer(self): status = check_results.DeepCheckResults(b"fake-root-si") status.add_check( - FakeCheckResults("", False, False), + FakeCheckResults(b"", False, False), (u"fake", u"unhealthy", u"unrecoverable") ) status.add_check( - FakeCheckResults("", True, True), + FakeCheckResults(b"", True, True), (u"fake", u"healthy", u"recoverable") ) status.add_check( - FakeCheckResults("", True, False), + FakeCheckResults(b"", True, False), (u"fake", u"healthy", u"unrecoverable") ) status.add_check( - FakeCheckResults("", False, True), + FakeCheckResults(b"", False, True), (u"fake", u"unhealthy", u"recoverable") ) @@ -524,15 +524,15 @@ class WebResultsRendering(unittest.TestCase): status = check_results.DeepCheckAndRepairResults(b"") status.add_check_and_repair( - FakeCheckAndRepairResults("attempted/success", True, True), + FakeCheckAndRepairResults(b"attempted/success", True, True), (u"attempted", u"success") ) status.add_check_and_repair( - FakeCheckAndRepairResults("attempted/failure", True, False), + FakeCheckAndRepairResults(b"attempted/failure", True, False), (u"attempted", u"failure") ) status.add_check_and_repair( - FakeCheckAndRepairResults("unattempted/failure", False, False), + FakeCheckAndRepairResults(b"unattempted/failure", False, False), (u"unattempted", u"failure") ) diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index fd8891d3c..de2762df1 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -1,3 +1,4 @@ +from future.builtins import str import time import json @@ -200,7 +201,7 @@ class ResultsBase(object): return tags.ul(r) def _html(self, s): - if isinstance(s, (str, unicode)): + if isinstance(s, (bytes, str)): return html.escape(s) assert isinstance(s, (list, tuple)) return [html.escape(w) for w in s] @@ -522,7 +523,7 @@ class DeepCheckResultsRendererElement(Element, ResultsBase, ReloadMixin): summary = cr.get_summary() if summary: summary_text = ": " + summary - summary_text += " [SI: %s]" % cr.get_storage_index_string() + summary_text += " [SI: %s]" % cr.get_storage_index_string().decode("ascii") problems.append({ # Not sure self._join_pathstring(path) is the # right thing to use here. From 96231fab5f292ec9c58b22fd9026b43fde68adc7 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 5 Oct 2020 11:01:11 -0400 Subject: [PATCH 06/15] Support bytes in JSON output. --- src/allmydata/test/test_util.py | 19 +++++++++++ src/allmydata/util/_python3.py | 1 + src/allmydata/util/jsonbytes.py | 51 ++++++++++++++++++++++++++++++ src/allmydata/web/check_results.py | 2 +- 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 src/allmydata/util/jsonbytes.py diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py index f1f2b1c66..14fc020b9 100644 --- a/src/allmydata/test/test_util.py +++ b/src/allmydata/test/test_util.py @@ -13,16 +13,19 @@ if PY2: import six import os, time, sys import yaml +import json from twisted.trial import unittest from allmydata.util import idlib, mathutil from allmydata.util import fileutil +from allmydata.util import jsonbytes from allmydata.util import pollmixin from allmydata.util import yamlutil from allmydata.util.fileutil import EncryptedTemporaryFile from allmydata.test.common_util import ReallyEqualMixin + if six.PY3: long = int @@ -469,3 +472,19 @@ class YAML(unittest.TestCase): self.assertIsInstance(back[0], str) self.assertIsInstance(back[1], str) self.assertIsInstance(back[2], str) + + +class JSONBytes(unittest.TestCase): + """Tests for BytesJSONEncoder.""" + + def test_encode_bytes(self): + """BytesJSONEncoder can encode bytes.""" + data = { + b"hello": [1, b"cd"], + } + expected = { + u"hello": [1, u"cd"], + } + encoded = jsonbytes.dumps(data) + self.assertEqual(json.loads(encoded), expected) + self.assertEqual(jsonbytes.loads(encoded), expected) diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index ccff0958a..7ca18da15 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -72,6 +72,7 @@ PORTED_MODULES = [ "allmydata.util.hashutil", "allmydata.util.humanreadable", "allmydata.util.iputil", + "allmydata.util.jsonbytes", "allmydata.util.log", "allmydata.util.mathutil", "allmydata.util.namespace", diff --git a/src/allmydata/util/jsonbytes.py b/src/allmydata/util/jsonbytes.py new file mode 100644 index 000000000..406a471a0 --- /dev/null +++ b/src/allmydata/util/jsonbytes.py @@ -0,0 +1,51 @@ +""" +A JSON encoder than can serialize bytes. + +Ported to Python 3. +""" + +from __future__ import unicode_literals +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function + +from future.utils import PY2 +if PY2: + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + + +import json + + +class BytesJSONEncoder(json.JSONEncoder): + """ + A JSON encoder than can also encode bytes. + + The bytes are assumed to be UTF-8 encoded Unicode strings. + """ + def default(self, o): + if isinstance(o, bytes): + return o.decode("utf-8") + return json.JSONEncoder.default(self, o) + + +def dumps(obj, *args, **kwargs): + """Encode to JSON, supporting bytes as keys or values. + + The bytes are assumed to be UTF-8 encoded Unicode strings. + """ + if isinstance(obj, dict): + new_obj = {} + for k, v in obj.items(): + if isinstance(k, bytes): + k = k.decode("utf-8") + new_obj[k] = v + obj = new_obj + return json.dumps(obj, cls=BytesJSONEncoder, *args, **kwargs) + + +# To make this module drop-in compatible with json module: +loads = json.loads + + +__all__ = ["dumps", "loads"] diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index de2762df1..c87254f0d 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -1,7 +1,6 @@ from future.builtins import str import time -import json from twisted.web import ( http, @@ -32,6 +31,7 @@ from allmydata.interfaces import ( from allmydata.util import ( base32, dictutil, + jsonbytes as json, # Supporting dumping bytes ) From a4ce3b42ef31edc1e01e4842cc0687bc4d9dc117 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 5 Oct 2020 11:01:37 -0400 Subject: [PATCH 07/15] News fragment. --- newsfragments/3459.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3459.minor diff --git a/newsfragments/3459.minor b/newsfragments/3459.minor new file mode 100644 index 000000000..e69de29bb From dd863a003f87a458d9030f5b8fa574a5f3b6d9e3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 5 Oct 2020 11:12:06 -0400 Subject: [PATCH 08/15] Port test_checker.py to Python 3. --- src/allmydata/check_results.py | 7 ++++++- src/allmydata/test/test_checker.py | 17 ++++++++++++++--- src/allmydata/util/_python3.py | 1 + 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py index 068f77a25..f33c3afc0 100644 --- a/src/allmydata/check_results.py +++ b/src/allmydata/check_results.py @@ -1,3 +1,4 @@ +from past.builtins import unicode from zope.interface import implementer from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \ @@ -56,7 +57,11 @@ class CheckResults(object): self._list_incompatible_shares = list_incompatible_shares self._count_incompatible_shares = count_incompatible_shares - assert isinstance(summary, str) # should be a single string + # On Python 2, we can mix bytes and Unicode. On Python 3, we want + # unicode. + if isinstance(summary, bytes): + summary = unicode(summary, "utf-8") + assert isinstance(summary, unicode) # should be a single string self._summary = summary assert not isinstance(report, str) # should be list of strings self._report = report diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 0d5877abe..e8f7e3ce1 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -1,5 +1,16 @@ +""" +Ported to Python 3. +""" + +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + from future.utils import PY2 -from past.builtins import unicode +if PY2: + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + import json import os.path, shutil @@ -161,7 +172,7 @@ class WebResultsRendering(unittest.TestCase): "service-name": "storage", "anonymous-storage-FURL": furl, "permutation-seed-base32": "", - "nickname": unicode(nickname), + "nickname": str(nickname), "app-versions": {}, # need #466 and v2 introducer "my-version": "ver", "oldest-supported": "oldest", @@ -671,7 +682,7 @@ class BalancingAct(GridTestMixin, unittest.TestCase): "This little printing function is only meant for < 26 servers" shares_chart = {} names = dict(zip([ss.my_nodeid - for _,ss in self.g.servers_by_number.iteritems()], + for _,ss in self.g.servers_by_number.items()], letters)) for shnum, serverid, _ in self.find_uri_shares(uri): shares_chart.setdefault(shnum, []).append(names[serverid]) diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index 7ca18da15..f88f4f489 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -89,6 +89,7 @@ PORTED_TEST_MODULES = [ "allmydata.test.test_abbreviate", "allmydata.test.test_base32", "allmydata.test.test_base62", + "allmydata.test.test_checker", "allmydata.test.test_codec", "allmydata.test.test_common_util", "allmydata.test.test_configutil", From c680b1d971e3cf1ec5b8adca87f9747ed42d3400 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 5 Oct 2020 11:38:53 -0400 Subject: [PATCH 09/15] Lint fixes. --- src/allmydata/web/common.py | 1 - src/allmydata/web/operations.py | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 999256ad3..a5f29e263 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -1,7 +1,6 @@ import time import json -from functools import wraps from twisted.web import ( http, diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 6c1fb190f..f96feeda4 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,10 +1,6 @@ import time from nevow import url -from twisted.web.template import ( - renderer, - tags as T, -) from twisted.python.failure import Failure from twisted.internet import reactor, defer from twisted.web import resource From 3ea18ca3fc23967b745eff394160c289eb3924b2 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 13 Oct 2020 09:40:51 -0400 Subject: [PATCH 10/15] As better alternative to common_py3, make common.py import on Python 3. --- src/allmydata/web/check_results.py | 2 +- src/allmydata/web/common.py | 245 +++++++++++++++++++++++++++-- src/allmydata/web/common_py3.py | 226 +------------------------- src/allmydata/web/operations.py | 2 +- 4 files changed, 239 insertions(+), 236 deletions(-) diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index c87254f0d..dfaee38e4 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -14,7 +14,7 @@ from twisted.web.template import ( renderElement, tags, ) -from allmydata.web.common_py3 import ( +from allmydata.web.common import ( exception_to_child, get_arg, get_root, diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index a5f29e263..abba7c267 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -1,32 +1,56 @@ +from future.utils import PY2 +from past.builtins import unicode import time import json +from functools import wraps from twisted.web import ( http, resource, server, + template, ) +from twisted.web.template import tags as T +from twisted.web.iweb import IRequest as ITwistedRequest from twisted.python import log -from nevow import appserver -from nevow.inevow import IRequest +if PY2: + from nevow.appserver import DefaultExceptionHandler + from nevow.inevow import IRequest as INevowRequest +else: + class DefaultExceptionHandler: + def __init__(self, *args, **kwargs): + raise NotImplementedError("Still not ported to Python 3") + INevowRequest = None + +from allmydata import blacklist from allmydata.interfaces import ( + EmptyPathnameComponentError, + ExistingChildError, + FileTooLargeError, + MustBeDeepImmutableError, + MustBeReadonlyError, + MustNotBeUnknownRWError, + NoSharesError, + NoSuchChildError, + NotEnoughSharesError, MDMF_VERSION, SDMF_VERSION, ) +from allmydata.mutable.common import UnrecoverableFileError from allmydata.util.hashutil import timing_safe_compare from allmydata.util.time_format import ( format_delta, format_time, ) from allmydata.util.encodingutil import ( + quote_output, to_bytes, ) # Originally part of this module, so still part of its API: from .common_py3 import ( # noqa: F401 - get_arg, abbreviate_time, MultiFormatResource, WebError, humanize_exception, - SlotsSequenceElement, render_exception, get_root, exception_to_child + get_arg, abbreviate_time, MultiFormatResource, WebError, ) @@ -104,6 +128,16 @@ def parse_offset_arg(offset): return offset +def get_root(ctx_or_req): + if PY2: + req = INevowRequest(ctx_or_req) + else: + req = ITwistedRequest(ctx_or_req) + depth = len(req.prepath) + len(req.postpath) + link = "/".join([".."] * depth) + return link + + def convert_children_json(nodemaker, children_json): """I convert the JSON output of GET?t=json into the dict-of-nodes input to both dirnode.create_subdirectory() and @@ -197,6 +231,94 @@ def should_create_intermediate_directories(req): return bool(req.method in ("PUT", "POST") and t not in ("delete", "rename", "rename-form", "check")) +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 isinstance(exc, ExistingChildError): + return ("There was already a child by that name, and you asked me " + "to not replace it.", http.CONFLICT) + 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 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(exc) + return (t, http.GONE) + 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(exc) + return (t, http.GONE) + 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, " + "insufficient servers were connected, the URI was corrupt, or " + "that shares have been lost due to server departure, hard drive " + "failure, or disk corruption. You should perform a filecheck on " + "this object to learn more.") + return (t, http.GONE) + 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" + "If the cap is actually an immutable readcap, then using a " + "webapi server that supports a later version of Tahoe may help.\n\n" + "If you are using the webapi directly, then specifying an immutable " + "readcap in the read slot (ro_uri) of the JSON PROPDICT, and " + "omitting the write slot (rw_uri), would also work in this " + "case.") % quoted_name + else: + t = ("MustNotBeUnknownRWError: an operation to add a child named " + "%s to a directory was given an unknown cap in a write slot.\n" + "Using a webapi server that supports a later version of Tahoe " + "may help.\n\n" + "If you are using the webapi directly, specifying a readcap in " + "the read slot (ro_uri) of the JSON PROPDICT, as well as a " + "writecap in the write slot if desired, would also work in this " + "case.") % quoted_name + return (t, http.BAD_REQUEST) + 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 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 isinstance(exc, blacklist.FileProhibited): + t = "Access Prohibited: %s" % quote_output(exc.reason, encoding="utf-8", quotemarks=False) + return (t, http.FORBIDDEN) + 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) + def humanize_failure(f): """ @@ -211,9 +333,9 @@ def humanize_failure(f): return humanize_exception(f.value) -class MyExceptionHandler(appserver.DefaultExceptionHandler, object): +class MyExceptionHandler(DefaultExceptionHandler, object): def simple(self, ctx, text, code=http.BAD_REQUEST): - req = IRequest(ctx) + req = INevowRequest(ctx) req.setResponseCode(code) #req.responseHeaders.setRawHeaders("content-encoding", []) #req.responseHeaders.setRawHeaders("content-disposition", []) @@ -239,17 +361,17 @@ class MyExceptionHandler(appserver.DefaultExceptionHandler, object): # twisted.web.server.Request.render() has support for transforming # this into an appropriate 501 NOT_IMPLEMENTED or 405 NOT_ALLOWED # return code, but nevow does not. - req = IRequest(ctx) + req = INevowRequest(ctx) method = req.method return self.simple(ctx, "I don't know how to treat a %s request." % method, http.NOT_IMPLEMENTED) - req = IRequest(ctx) + req = INevowRequest(ctx) accept = req.getHeader("accept") if not accept: accept = "*/*" if "*/*" in accept or "text/*" in accept or "text/html" in accept: - super = appserver.DefaultExceptionHandler + super = DefaultExceptionHandler return super.renderHTTP_exception(self, ctx, f) # use plain text traceback = f.getTraceback() @@ -260,6 +382,49 @@ class NeedOperationHandleError(WebError): pass +class SlotsSequenceElement(template.Element): + """ + ``SlotsSequenceElement` is a minimal port of nevow's sequence renderer for + twisted.web.template. + + Tags passed in to be templated will have two renderers available: ``item`` + and ``tag``. + """ + + def __init__(self, tag, seq): + self.loader = template.TagLoader(tag) + self.seq = seq + + @template.renderer + def header(self, request, tag): + return tag + + @template.renderer + def item(self, request, tag): + """ + A template renderer for each sequence item. + + ``tag`` will be cloned for each item in the sequence provided, and its + slots filled from the sequence item. Each item must be dict-like enough + for ``tag.fillSlots(**item)``. Each cloned tag will be siblings with no + separator beween them. + """ + for item in self.seq: + yield tag.clone(deep=False).fillSlots(**item) + + @template.renderer + def empty(self, request, tag): + """ + A template renderer for empty sequences. + + This renderer will either return ``tag`` unmodified if the provided + sequence has no items, or return the empty string if there are any + items. + """ + if len(self.seq) > 0: + return u'' + else: + return tag class TokenOnlyWebApi(resource.Resource, object): @@ -314,3 +479,65 @@ class TokenOnlyWebApi(resource.Resource, object): 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 f(self, name, req) + except Exception as e: + description, status = humanize_exception(e) + return resource.ErrorPage(status, "Error", description) + return g + + +def render_exception(f): + """ + Decorate a ``render_*`` method with exception handling behavior to render + an error page reflecting the exception. + """ + @wraps(f) + def g(self, request): + try: + return f(self, request) + except Exception as e: + description, status = humanize_exception(e) + return resource.ErrorPage(status, "Error", description).render(request) + return g + + +class ReloadMixin(object): + REFRESH_TIME = 1*60 + + @template.renderer + def refresh(self, req, tag): + if self.monitor.is_finished(): + return "" + # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) + # but I can't tell if he's joking or not + tag.attributes["http-equiv"] = "refresh" + tag.attributes["content"] = str(self.REFRESH_TIME) + return tag + + @template.renderer + def reload(self, req, tag): + if self.monitor.is_finished(): + return b"" + # url.gethere would break a proxy, so the correct thing to do is + # req.path[-1] + queryargs + ophandle = req.prepath[-1] + reload_target = ophandle + b"?output=html" + cancel_target = ophandle + b"?t=cancel" + cancel_button = T.form(T.input(type="submit", value="Cancel"), + action=cancel_target, + method="POST", + enctype="multipart/form-data",) + + return (T.h2("Operation still running: ", + T.a("Reload", href=reload_target), + ), + cancel_button,) diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index a94401b45..73130cbab 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -6,32 +6,15 @@ Can eventually be merged back into allmydata.web.common. from future.utils import PY2 -from functools import wraps - if PY2: from nevow.inevow import IRequest as INevowRequest else: INevowRequest = None -from twisted.web import resource, http, template -from twisted.web.template import tags as T +from twisted.web import resource, http from twisted.web.iweb import IRequest -from allmydata import blacklist -from allmydata.interfaces import ( - EmptyPathnameComponentError, - ExistingChildError, - FileTooLargeError, - MustBeDeepImmutableError, - MustBeReadonlyError, - MustNotBeUnknownRWError, - NoSharesError, - NoSuchChildError, - NotEnoughSharesError, -) -from allmydata.mutable.common import UnrecoverableFileError from allmydata.util import abbreviate -from allmydata.util.encodingutil import quote_output class WebError(Exception): @@ -137,210 +120,3 @@ def abbreviate_time(data): if s >= 0.001: return "%.1fms" % (1000*s) return "%.0fus" % (1000000*s) - - -def render_exception(f): - """ - Decorate a ``render_*`` method with exception handling behavior to render - an error page reflecting the exception. - """ - @wraps(f) - def g(self, request): - try: - return f(self, request) - except Exception as e: - description, status = humanize_exception(e) - return resource.ErrorPage(status, "Error", description).render(request) - return g - - -class SlotsSequenceElement(template.Element): - """ - ``SlotsSequenceElement` is a minimal port of nevow's sequence renderer for - twisted.web.template. - - Tags passed in to be templated will have two renderers available: ``item`` - and ``tag``. - """ - - def __init__(self, tag, seq): - self.loader = template.TagLoader(tag) - self.seq = seq - - @template.renderer - def header(self, request, tag): - return tag - - @template.renderer - def item(self, request, tag): - """ - A template renderer for each sequence item. - - ``tag`` will be cloned for each item in the sequence provided, and its - slots filled from the sequence item. Each item must be dict-like enough - for ``tag.fillSlots(**item)``. Each cloned tag will be siblings with no - separator beween them. - """ - for item in self.seq: - yield tag.clone(deep=False).fillSlots(**item) - - @template.renderer - def empty(self, request, tag): - """ - A template renderer for empty sequences. - - This renderer will either return ``tag`` unmodified if the provided - sequence has no items, or return the empty string if there are any - items. - """ - if len(self.seq) > 0: - return u'' - else: - return tag - - -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 isinstance(exc, ExistingChildError): - return ("There was already a child by that name, and you asked me " - "to not replace it.", http.CONFLICT) - 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 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(exc) - return (t, http.GONE) - 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(exc) - return (t, http.GONE) - 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, " - "insufficient servers were connected, the URI was corrupt, or " - "that shares have been lost due to server departure, hard drive " - "failure, or disk corruption. You should perform a filecheck on " - "this object to learn more.") - return (t, http.GONE) - 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" - "If the cap is actually an immutable readcap, then using a " - "webapi server that supports a later version of Tahoe may help.\n\n" - "If you are using the webapi directly, then specifying an immutable " - "readcap in the read slot (ro_uri) of the JSON PROPDICT, and " - "omitting the write slot (rw_uri), would also work in this " - "case.") % quoted_name - else: - t = ("MustNotBeUnknownRWError: an operation to add a child named " - "%s to a directory was given an unknown cap in a write slot.\n" - "Using a webapi server that supports a later version of Tahoe " - "may help.\n\n" - "If you are using the webapi directly, specifying a readcap in " - "the read slot (ro_uri) of the JSON PROPDICT, as well as a " - "writecap in the write slot if desired, would also work in this " - "case.") % quoted_name - return (t, http.BAD_REQUEST) - 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 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 isinstance(exc, blacklist.FileProhibited): - t = "Access Prohibited: %s" % quote_output(exc.reason, encoding="utf-8", quotemarks=False) - return (t, http.FORBIDDEN) - 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) - - -def get_root(ctx_or_req): - if PY2: - req = INevowRequest(ctx_or_req) - else: - req = IRequest(ctx_or_req) - depth = len(req.prepath) + len(req.postpath) - link = "/".join([".."] * depth) - return link - - -class ReloadMixin(object): - REFRESH_TIME = 1*60 - - @template.renderer - def refresh(self, req, tag): - if self.monitor.is_finished(): - return "" - # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) - # but I can't tell if he's joking or not - tag.attributes["http-equiv"] = "refresh" - tag.attributes["content"] = str(self.REFRESH_TIME) - return tag - - @template.renderer - def reload(self, req, tag): - if self.monitor.is_finished(): - return b"" - # url.gethere would break a proxy, so the correct thing to do is - # req.path[-1] + queryargs - ophandle = req.prepath[-1] - reload_target = ophandle + b"?output=html" - cancel_target = ophandle + b"?t=cancel" - cancel_button = T.form(T.input(type="submit", value="Cancel"), - action=cancel_target, - method="POST", - enctype="multipart/form-data",) - - return (T.h2("Operation still running: ", - T.a("Reload", href=reload_target), - ), - cancel_button,) - - -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 f(self, name, req) - except Exception as e: - description, status = humanize_exception(e) - return resource.ErrorPage(status, "Error", description) - return g diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index f96feeda4..982f95b47 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -17,7 +17,7 @@ from allmydata.web.common import ( ) # Originally part of this module, so still part of its API: -from .common_py3 import ( # noqa: F401 +from .common import ( # noqa: F401 ReloadMixin, ) From 50925fcec1daec08a0d9e3a2e2918c0ed2efb6f4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 13 Oct 2020 09:49:39 -0400 Subject: [PATCH 11/15] Get rid of more no-longer-needed moves. --- src/allmydata/web/check_results.py | 2 +- src/allmydata/web/common.py | 34 -------------------- src/allmydata/web/operations.py | 50 +++++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index dfaee38e4..54130183b 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -22,8 +22,8 @@ from allmydata.web.common import ( WebError, MultiFormatResource, SlotsSequenceElement, - ReloadMixin, ) +from allmydata.web.operations import ReloadMixin from allmydata.interfaces import ( ICheckAndRepairResults, ICheckResults, diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index abba7c267..5c27f20ab 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -11,7 +11,6 @@ from twisted.web import ( server, template, ) -from twisted.web.template import tags as T from twisted.web.iweb import IRequest as ITwistedRequest from twisted.python import log if PY2: @@ -508,36 +507,3 @@ def render_exception(f): description, status = humanize_exception(e) return resource.ErrorPage(status, "Error", description).render(request) return g - - -class ReloadMixin(object): - REFRESH_TIME = 1*60 - - @template.renderer - def refresh(self, req, tag): - if self.monitor.is_finished(): - return "" - # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) - # but I can't tell if he's joking or not - tag.attributes["http-equiv"] = "refresh" - tag.attributes["content"] = str(self.REFRESH_TIME) - return tag - - @template.renderer - def reload(self, req, tag): - if self.monitor.is_finished(): - return b"" - # url.gethere would break a proxy, so the correct thing to do is - # req.path[-1] + queryargs - ophandle = req.prepath[-1] - reload_target = ophandle + b"?output=html" - cancel_target = ophandle + b"?t=cancel" - cancel_button = T.form(T.input(type="submit", value="Cancel"), - action=cancel_target, - method="POST", - enctype="multipart/form-data",) - - return (T.h2("Operation still running: ", - T.a("Reload", href=reload_target), - ), - cancel_button,) diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 982f95b47..a3f929278 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,6 +1,14 @@ - +from future.utils import PY2 import time -from nevow import url +if PY2: + from nevow import url +else: + # This module still needs porting to Python 3 + url = None +from twisted.web.template import ( + renderer, + tags as T, +) from twisted.python.failure import Failure from twisted.internet import reactor, defer from twisted.web import resource @@ -16,11 +24,6 @@ from allmydata.web.common import ( exception_to_child, ) -# Originally part of this module, so still part of its API: -from .common import ( # noqa: F401 - ReloadMixin, -) - MINUTE = 60 HOUR = 60*MINUTE DAY = 24*HOUR @@ -143,3 +146,36 @@ class OphandleTable(resource.Resource, service.Service): self.timers[ophandle].cancel() self.timers.pop(ophandle, None) self.handles.pop(ophandle, None) + + +class ReloadMixin(object): + REFRESH_TIME = 1*MINUTE + + @renderer + def refresh(self, req, tag): + if self.monitor.is_finished(): + return "" + # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) + # but I can't tell if he's joking or not + tag.attributes["http-equiv"] = "refresh" + tag.attributes["content"] = str(self.REFRESH_TIME) + return tag + + @renderer + def reload(self, req, tag): + if self.monitor.is_finished(): + return "" + # url.gethere would break a proxy, so the correct thing to do is + # req.path[-1] + queryargs + ophandle = req.prepath[-1] + reload_target = ophandle + "?output=html" + cancel_target = ophandle + "?t=cancel" + cancel_button = T.form(T.input(type="submit", value="Cancel"), + action=cancel_target, + method="POST", + enctype="multipart/form-data",) + + return (T.h2("Operation still running: ", + T.a("Reload", href=reload_target), + ), + cancel_button,) From c0f486a9f629c48cb3bf96a9d6615c2b7dc3d66f Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 13 Oct 2020 09:51:25 -0400 Subject: [PATCH 12/15] Work on Python 3. --- src/allmydata/web/operations.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index a3f929278..a409561af 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -164,12 +164,12 @@ class ReloadMixin(object): @renderer def reload(self, req, tag): if self.monitor.is_finished(): - return "" + return b"" # url.gethere would break a proxy, so the correct thing to do is # req.path[-1] + queryargs ophandle = req.prepath[-1] - reload_target = ophandle + "?output=html" - cancel_target = ophandle + "?t=cancel" + reload_target = ophandle + b"?output=html" + cancel_target = ophandle + b"?t=cancel" cancel_button = T.form(T.input(type="submit", value="Cancel"), action=cancel_target, method="POST", From e6a196c144b6f42c1f073b39a03a566976244f4b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 15 Oct 2020 08:33:51 -0400 Subject: [PATCH 13/15] Get rid of hopefully unnecessary sort. --- src/allmydata/immutable/filenode.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py index 9c7999a3e..9e13e1337 100644 --- a/src/allmydata/immutable/filenode.py +++ b/src/allmydata/immutable/filenode.py @@ -152,7 +152,6 @@ class CiphertextFileNode(object): for server in servers: sm.add(shnum, server) servers_responding.add(server) - servers_responding = sorted(servers_responding, key=lambda o: id(o)) good_hosts = len(reduce(set.union, sm.values(), set())) is_healthy = bool(len(sm) >= verifycap.total_shares) From 1c976990a16192f3541fc053a97fba313b1ba428 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 15 Oct 2020 08:34:56 -0400 Subject: [PATCH 14/15] Make comment more meaningful. --- src/allmydata/mutable/publish.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/mutable/publish.py b/src/allmydata/mutable/publish.py index 8816855e7..187aa98bd 100644 --- a/src/allmydata/mutable/publish.py +++ b/src/allmydata/mutable/publish.py @@ -702,8 +702,8 @@ class Publish(object): self.log("Pushing segment %d of %d" % (segnum + 1, self.num_segments)) + # XXX: Why does this return a list? data = self.data.read(segsize) - # XXX: This is dumb. Why return a list? data = b"".join(data) assert len(data) == segsize, len(data) From 707ab50606b5f867684b6b45056453a4611f09b7 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 15 Oct 2020 08:37:09 -0400 Subject: [PATCH 15/15] Test BytesJSONEncoder with Unicode. --- src/allmydata/test/test_util.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py index 4e1496141..6c64174bc 100644 --- a/src/allmydata/test/test_util.py +++ b/src/allmydata/test/test_util.py @@ -486,6 +486,16 @@ class JSONBytes(unittest.TestCase): expected = { u"hello": [1, u"cd"], } + # Bytes get passed through as if they were UTF-8 Unicode: encoded = jsonbytes.dumps(data) self.assertEqual(json.loads(encoded), expected) self.assertEqual(jsonbytes.loads(encoded), expected) + + + def test_encode_unicode(self): + """BytesJSONEncoder encodes Unicode string as usual.""" + expected = { + u"hello": [1, u"cd"], + } + encoded = jsonbytes.dumps(expected) + self.assertEqual(json.loads(encoded), expected)