From 574c63d3505f66fd49497d3bf05490d45fc0aa57 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 12 Oct 2020 07:25:44 -0400 Subject: [PATCH 01/83] Port immutable.offloaded to Python 3 --- newsfragments/3466.minor | 0 src/allmydata/immutable/offloaded.py | 15 +++++++++++++-- src/allmydata/test/test_helper.py | 3 +++ src/allmydata/util/_python3.py | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 newsfragments/3466.minor diff --git a/newsfragments/3466.minor b/newsfragments/3466.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index fb8c706a3..652e68b2f 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -1,3 +1,14 @@ +""" +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 +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 os, stat, time, weakref from zope.interface import implementer @@ -128,9 +139,9 @@ class CHKUploadHelper(Referenceable, upload.CHKUploader): peer selection, encoding, and share pushing. I read ciphertext from the remote AssistedUploader. """ - VERSION = { "http://allmydata.org/tahoe/protocols/helper/chk-upload/v1" : + VERSION = { b"http://allmydata.org/tahoe/protocols/helper/chk-upload/v1" : { }, - "application-version": str(allmydata.__full_version__), + b"application-version": allmydata.__full_version__.encode("utf-8"), } def __init__(self, storage_index, diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index c47da3277..0e628c81b 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -1,3 +1,6 @@ +""" +Ported to Python 3. +""" from __future__ import absolute_import from __future__ import division from __future__ import print_function diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index 8c2f0ebed..4002ec6e6 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -46,6 +46,7 @@ PORTED_MODULES = [ "allmydata.immutable.happiness_upload", "allmydata.immutable.layout", "allmydata.immutable.literal", + "allmydata.immutable.offloaded", "allmydata.immutable.upload", "allmydata.interfaces", "allmydata.introducer.interfaces", From 69c7c405107d19ac8c7b7430fcada1a7801e299c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 11:43:42 -0400 Subject: [PATCH 02/83] handle Deferred from render --- src/allmydata/test/web/test_grid.py | 5 + src/allmydata/web/common.py | 198 +++++++++++++++++++++------- src/allmydata/web/directory.py | 10 +- src/allmydata/web/filenode.py | 43 ++---- src/allmydata/web/operations.py | 20 +-- src/allmydata/web/unlinked.py | 5 +- 6 files changed, 188 insertions(+), 93 deletions(-) diff --git a/src/allmydata/test/web/test_grid.py b/src/allmydata/test/web/test_grid.py index 01eb93fa7..8f61781d4 100644 --- a/src/allmydata/test/web/test_grid.py +++ b/src/allmydata/test/web/test_grid.py @@ -18,6 +18,10 @@ from allmydata.storage.shares import get_share_file from allmydata.scripts.debug import CorruptShareOptions, corrupt_share from allmydata.immutable import upload from allmydata.mutable import publish + +from ...web.common import ( + render_exception, +) from .. import common_util as testutil from ..common import WebErrorMixin, ShouldFailMixin from ..no_network import GridTestMixin @@ -34,6 +38,7 @@ class CompletelyUnhandledError(Exception): pass class ErrorBoom(object, resource.Resource): + @render_exception def render(self, req): raise CompletelyUnhandledError("whoops") diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 102e67adc..9c88d832a 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -3,15 +3,43 @@ import time import json from functools import wraps +from hyperlink import ( + DecodedURL, +) + from twisted.web import ( http, resource, server, template, ) +from twisted.web.template import ( + tags, +) +from twisted.web.util import ( + FailureElement, + redirectTo, +) +from twisted.python.reflect import ( + fullyQualifiedName, +) +from twisted.python.urlpath import ( + URLPath, +) from twisted.python import log +from twisted.python.failure import ( + Failure, +) +from twisted.internet.defer import ( + Deferred, + maybeDeferred, +) +from twisted.web.resource import ( + IResource, +) from nevow import appserver from nevow.inevow import IRequest + from allmydata import blacklist from allmydata.interfaces import ( EmptyPathnameComponentError, @@ -320,48 +348,10 @@ def humanize_failure(f): class MyExceptionHandler(appserver.DefaultExceptionHandler, object): - def simple(self, ctx, text, code=http.BAD_REQUEST): - req = IRequest(ctx) - req.setResponseCode(code) - #req.responseHeaders.setRawHeaders("content-encoding", []) - #req.responseHeaders.setRawHeaders("content-disposition", []) - req.setHeader("content-type", "text/plain;charset=utf-8") - if isinstance(text, unicode): - text = text.encode("utf-8") - req.setHeader("content-length", b"%d" % len(text)) - req.write(text) - # TODO: consider putting the requested URL here - req.finishRequest(False) - def renderHTTP_exception(self, ctx, f): - try: - text, code = humanize_failure(f) - except: - log.msg("exception in humanize_failure") - log.msg("argument was %s" % (f,)) - log.err() - text, code = str(f), None - if code is not None: - return self.simple(ctx, text, code) - if f.check(server.UnsupportedMethod): - # 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) - method = req.method - return self.simple(ctx, - "I don't know how to treat a %s request." % method, - http.NOT_IMPLEMENTED) req = IRequest(ctx) - accept = req.getHeader("accept") - if not accept: - accept = "*/*" - if "*/*" in accept or "text/*" in accept or "text/html" in accept: - super = appserver.DefaultExceptionHandler - return super.renderHTTP_exception(self, ctx, f) - # use plain text - traceback = f.getTraceback() - return self.simple(ctx, traceback, http.INTERNAL_SERVER_ERROR) + req.write(_renderHTTP_exception(req, f)) + req.finishRequest(False) class NeedOperationHandleError(WebError): @@ -481,16 +471,130 @@ def exception_to_child(f): return g -def render_exception(f): +def render_exception(render): """ Decorate a ``render_*`` method with exception handling behavior to render an error page reflecting the exception. """ - @wraps(f) + @wraps(render) 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) + bound_render = render.__get__(self, type(self)) + result = maybeDeferred(bound_render, request) + if isinstance(result, Deferred): + result.addBoth(_finish, bound_render, request) + return server.NOT_DONE_YET + elif isinstance(result, bytes): + return result + elif result == server.NOT_DONE_YET: + return server.NOT_DONE_YET + else: + raise Exception("{!r} returned unusable {!r}".format( + fullyQualifiedName(bound_render), + result, + )) + return g + + +def _finish(result, render, request): + if isinstance(result, Failure): + _finish( + _renderHTTP_exception(request, result), + render, + request, + ) + elif IResource.providedBy(result): + # If result is also using @render_exception then we don't want to + # double-apply the logic. This leads to an attempt to double-finish + # the request. If it isn't using @render_exception then you should + # fix it so it is. + result.render(request) + elif isinstance(result, bytes): + request.write(result) + request.finish() + elif isinstance(result, URLPath): + if result.netloc == b"": + root = URLPath.fromRequest(request) + result.scheme = root.scheme + result.netloc = root.netloc + _finish(redirectTo(str(result), request), render, request) + elif isinstance(result, DecodedURL): + _finish(redirectTo(str(result), request), render, request) + elif result is None: + request.finish() + elif result == server.NOT_DONE_YET: + pass + else: + log.err("Request for {!r} handled by {!r} returned unusable {!r}".format( + request.uri, + fullyQualifiedName(render), + result, + )) + request.setResponseCode(http.INTERNAL_SERVER_ERROR) + _finish(b"", render, request) + + +def _renderHTTP_exception(request, failure): + try: + text, code = humanize_failure(failure) + except: + log.msg("exception in humanize_failure") + log.msg("argument was %s" % (failure,)) + log.err() + text = str(failure) + code = None + + if code is not None: + return _renderHTTP_exception_simple(request, text, code) + + if failure.check(server.UnsupportedMethod): + # 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. + method = request.method + return _renderHTTP_exception_simple( + request, + "I don't know how to treat a %s request." % (method,), + http.NOT_IMPLEMENTED, + ) + + accept = request.getHeader("accept") + if not accept: + accept = "*/*" + if "*/*" in accept or "text/*" in accept or "text/html" in accept: + request.setResponseCode(http.INTERNAL_SERVER_ERROR) + return template.renderElement( + request, + tags.html( + tags.head( + tags.title(u"Exception"), + ), + tags.body( + FailureElement(failure), + ), + ), + ) + + # use plain text + traceback = failure.getTraceback() + return _renderHTTP_exception_simple( + request, + traceback, + http.INTERNAL_SERVER_ERROR, + ) + + +def _renderHTTP_exception_simple(request, text, code): + request.setResponseCode(code) + request.setHeader("content-type", "text/plain;charset=utf-8") + if isinstance(text, unicode): + text = text.encode("utf-8") + request.setHeader("content-length", b"%d" % len(text)) + return text + + +def handle_when_done(req, d): + when_done = get_arg(req, "when_done", None) + if when_done: + d.addCallback(lambda res: DecodedURL.from_text(when_done.decode("utf-8"))) + return d diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 4d7aa1bd1..39bc297f5 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -58,6 +58,7 @@ from allmydata.web.common import ( SlotsSequenceElement, exception_to_child, render_exception, + handle_when_done, ) from allmydata.web.filenode import ReplaceMeMixin, \ FileNodeHandler, PlaceHolderNodeHandler @@ -206,6 +207,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): ) return make_handler_for(node, self.client, self.node, name) + @render_exception def render_DELETE(self, req): assert self.parentnode and self.name d = self.parentnode.delete(self.name) @@ -310,13 +312,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): else: raise WebError("POST to a directory with bad t=%s" % t) - when_done = get_arg(req, "when_done", None) - if when_done: - def done(res): - req.redirect(when_done) - return res - d.addCallback(done) - return d + return handle_when_done(req, d) def _POST_mkdir(self, req): name = get_arg(req, "name", "") diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 0ecc8cc52..b36ce0af8 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -1,6 +1,10 @@ import json +from hyperlink import ( + DecodedURL, +) + from twisted.web import http, static from twisted.internet import defer from twisted.web.resource import ( @@ -8,8 +12,6 @@ from twisted.web.resource import ( ErrorPage, ) -from nevow import url - from allmydata.interfaces import ExistingChildError from allmydata.monitor import Monitor from allmydata.immutable.upload import FileHandle @@ -34,8 +36,8 @@ from allmydata.web.common import ( render_exception, should_create_intermediate_directories, text_plain, - MyExceptionHandler, WebError, + handle_when_done, ) from allmydata.web.check_results import ( CheckResultsRenderer, @@ -150,10 +152,7 @@ class PlaceHolderNodeHandler(Resource, ReplaceMeMixin): # placeholder. raise WebError("POST to a file: bad t=%s" % t) - when_done = get_arg(req, "when_done", None) - if when_done: - d.addCallback(lambda res: when_done) - return d + return handle_when_done(req, d) class FileNodeHandler(Resource, ReplaceMeMixin, object): @@ -315,10 +314,7 @@ class FileNodeHandler(Resource, ReplaceMeMixin, object): else: raise WebError("POST to file: bad t=%s" % t) - when_done = get_arg(req, "when_done", None) - if when_done: - d.addCallback(lambda res: url.URL.fromString(when_done)) - return d + return handle_when_done(req, d) def _maybe_literal(self, res, Results_Class): if res: @@ -485,23 +481,11 @@ class FileDownloader(Resource, object): if req.method == "HEAD": return "" - finished = [] - def _request_finished(ign): - finished.append(True) - req.notifyFinish().addBoth(_request_finished) - d = self.filenode.read(req, first, size) - def _finished(ign): - if not finished: - req.finish() def _error(f): lp = log.msg("error during GET", facility="tahoe.webish", failure=f, level=log.UNUSUAL, umid="xSiF3w") - if finished: - log.msg("but it's too late to tell them", parent=lp, - level=log.UNUSUAL, umid="j1xIbw") - return req._tahoe_request_had_error = f # for HTTP-style logging if req.startedWriting: # The content-type is already set, and the response code has @@ -513,15 +497,16 @@ class FileDownloader(Resource, object): # error response be shorter than the intended results. # # We don't have a lot of options, unfortunately. - req.write("problem during download\n") - req.finish() + return b"problem during download\n" else: # We haven't written anything yet, so we can provide a # sensible error message. - eh = MyExceptionHandler() - eh.renderHTTP_exception(req, f) - d.addCallbacks(_finished, _error) - return req.deferred + return f + d.addCallbacks( + lambda ignored: None, + _error, + ) + return d def _file_json_metadata(req, filenode, edge_metadata): diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 21c2ec7ef..25e267038 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,10 +1,15 @@ import time -from nevow import url +from hyperlink import ( + DecodedURL, +) from twisted.web.template import ( renderer, tags as T, ) +from twisted.python.urlpath import ( + URLPath, +) from twisted.python.failure import Failure from twisted.internet import reactor, defer from twisted.web import resource @@ -84,17 +89,14 @@ class OphandleTable(resource.Resource, service.Service): """ :param allmydata.webish.MyRequest req: """ - ophandle = get_arg(req, "ophandle") + ophandle = get_arg(req, "ophandle").decode("utf-8") assert ophandle - target = get_root(req) + "/operations/" + ophandle + root = DecodedURL.from_text(unicode(URLPath.fromRequest(req))) + target = root.click(u"/").child(u"operations", ophandle) output = get_arg(req, "output") if output: - target = target + "?output=%s" % output - - # XXX: We have to use nevow.url here because nevow.appserver - # is unhappy with anything else; so this gets its own ticket. - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3314 - return url.URL.fromString(target) + target = target.add(u"output", output.decode("utf-8")) + return target @exception_to_child def getChild(self, name, req): diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index f94e32240..f63006e7e 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -2,6 +2,9 @@ import urllib from twisted.web import http from twisted.internet import defer +from twisted.python.urlpath import ( + URLPath, +) from twisted.python.filepath import FilePath from twisted.web.resource import Resource from twisted.web.template import ( @@ -66,7 +69,7 @@ def POSTUnlinkedCHK(req, client): def _done(upload_results, redir_to): if "%(uri)s" in redir_to: redir_to = redir_to.replace("%(uri)s", urllib.quote(upload_results.get_uri())) - return url.URL.fromString(redir_to) + return URLPath.fromString(redir_to) d.addCallback(_done, when_done) else: # return the Upload Results page, which includes the URI From df949868b6fe1b955246bf0d5ddbd57be0a459e0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 14:47:30 -0400 Subject: [PATCH 03/83] Stop explicitly finishing and then returning a string --- src/allmydata/web/unlinked.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index f63006e7e..9cf315235 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -163,7 +163,6 @@ def POSTUnlinkedCreateDirectory(req, client): new_url = "uri/" + urllib.quote(res.get_uri()) req.setResponseCode(http.SEE_OTHER) # 303 req.setHeader('location', new_url) - req.finish() return '' d.addCallback(_then_redir) else: @@ -182,7 +181,6 @@ def POSTUnlinkedCreateDirectoryWithChildren(req, client): new_url = "uri/" + urllib.quote(res.get_uri()) req.setResponseCode(http.SEE_OTHER) # 303 req.setHeader('location', new_url) - req.finish() return '' d.addCallback(_then_redir) else: @@ -201,7 +199,6 @@ def POSTUnlinkedCreateImmutableDirectory(req, client): new_url = "uri/" + urllib.quote(res.get_uri()) req.setResponseCode(http.SEE_OTHER) # 303 req.setHeader('location', new_url) - req.finish() return '' d.addCallback(_then_redir) else: From 07246b35092a7434ca264c0339ddb0ea66d0a199 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 15:15:32 -0400 Subject: [PATCH 04/83] Render requests more thoroughly --- src/allmydata/test/web/test_root.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/allmydata/test/web/test_root.py b/src/allmydata/test/web/test_root.py index 1a29b7a15..abc0444fb 100644 --- a/src/allmydata/test/web/test_root.py +++ b/src/allmydata/test/web/test_root.py @@ -25,6 +25,9 @@ from .common import ( assert_soup_has_tag_with_content, ) +from ..common_web import ( + render, +) from ..common import ( EMPTY_CLIENT_CONFIG, ) @@ -36,13 +39,6 @@ class RenderSlashUri(unittest.TestCase): """ def setUp(self): - self.request = DummyRequest(b"/uri") - self.request.fields = {} - - def prepathURL(): - return b"http://127.0.0.1.99999/" + b"/".join(self.request.prepath) - - self.request.prePathURL = prepathURL self.client = Mock() self.res = URIHandler(self.client) @@ -50,18 +46,20 @@ class RenderSlashUri(unittest.TestCase): """ A valid capbility does not result in error """ - self.request.args[b"uri"] = [( + uri = ( b"URI:CHK:nt2xxmrccp7sursd6yh2thhcky:" b"mukesarwdjxiyqsjinbfiiro6q7kgmmekocxfjcngh23oxwyxtzq:2:5:5874882" - )] - self.res.render_GET(self.request) + ) + return render(self.res, {b"uri": [uri]}) def test_invalid(self): """ A (trivially) invalid capbility is an error """ - self.request.args[b"uri"] = [b"not a capability"] - response_body = self.res.render_GET(self.request) + query_args = {b"uri": [b"not a capability"]} + response_body = self.successResultOf( + render(self.res, query_args), + ) soup = BeautifulSoup(response_body, 'html5lib') @@ -82,8 +80,11 @@ class RenderSlashUri(unittest.TestCase): """ Let hypothesis try a bunch of invalid capabilities """ - self.request.args[b"uri"] = [cap.encode('utf8')] - response_body = self.res.render_GET(self.request) + query_args = {b"uri": [cap.encode('utf8')]} + response_body = self.successResultOf( + render(self.res, query_args, + ), + ) soup = BeautifulSoup(response_body, 'html5lib') From d38ae4d6dd0b35b850c937b0fda45cba7035bf73 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 15:15:54 -0400 Subject: [PATCH 05/83] Stop reading server module attributes all the time It jumps through a ton of deprecation machinery that is at least tedious in the debugger, if not wasteful at runtime. --- src/allmydata/web/common.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 9c88d832a..441269d6e 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -10,12 +10,15 @@ from hyperlink import ( from twisted.web import ( http, resource, - server, template, ) from twisted.web.template import ( tags, ) +from twisted.web.server import ( + NOT_DONE_YET, + UnsupportedMethod, +) from twisted.web.util import ( FailureElement, redirectTo, @@ -426,7 +429,7 @@ class TokenOnlyWebApi(resource.Resource, object): def render(self, req): if req.method != 'POST': - raise server.UnsupportedMethod(('POST',)) + raise UnsupportedMethod(('POST',)) if req.args.get('token', False): raise WebError("Do not pass 'token' as URL argument", http.BAD_REQUEST) # not using get_arg() here because we *don't* want the token @@ -482,11 +485,11 @@ def render_exception(render): result = maybeDeferred(bound_render, request) if isinstance(result, Deferred): result.addBoth(_finish, bound_render, request) - return server.NOT_DONE_YET + return NOT_DONE_YET elif isinstance(result, bytes): return result - elif result == server.NOT_DONE_YET: - return server.NOT_DONE_YET + elif result == NOT_DONE_YET: + return NOT_DONE_YET else: raise Exception("{!r} returned unusable {!r}".format( fullyQualifiedName(bound_render), @@ -522,7 +525,7 @@ def _finish(result, render, request): _finish(redirectTo(str(result), request), render, request) elif result is None: request.finish() - elif result == server.NOT_DONE_YET: + elif result == NOT_DONE_YET: pass else: log.err("Request for {!r} handled by {!r} returned unusable {!r}".format( @@ -547,7 +550,7 @@ def _renderHTTP_exception(request, failure): if code is not None: return _renderHTTP_exception_simple(request, text, code) - if failure.check(server.UnsupportedMethod): + if failure.check(UnsupportedMethod): # 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. From f733a244aa2c9a33605fc6205a79afb988ccc887 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 15:59:05 -0400 Subject: [PATCH 06/83] Just gonna produce text instead --- src/allmydata/test/web/test_root.py | 52 ++++++----------------------- 1 file changed, 11 insertions(+), 41 deletions(-) diff --git a/src/allmydata/test/web/test_root.py b/src/allmydata/test/web/test_root.py index abc0444fb..6b0885eb2 100644 --- a/src/allmydata/test/web/test_root.py +++ b/src/allmydata/test/web/test_root.py @@ -18,9 +18,6 @@ from ...util.connection_status import ConnectionStatus from allmydata.web.root import URIHandler from allmydata.client import _Client -from hypothesis import given -from hypothesis.strategies import text - from .common import ( assert_soup_has_tag_with_content, ) @@ -46,11 +43,17 @@ class RenderSlashUri(unittest.TestCase): """ A valid capbility does not result in error """ - uri = ( + query_args = {b"uri": [ b"URI:CHK:nt2xxmrccp7sursd6yh2thhcky:" b"mukesarwdjxiyqsjinbfiiro6q7kgmmekocxfjcngh23oxwyxtzq:2:5:5874882" + ]} + response_body = self.successResultOf( + render(self.res, query_args), + ) + self.assertNotEqual( + response_body, + "Invalid capability", ) - return render(self.res, {b"uri": [uri]}) def test_invalid(self): """ @@ -60,42 +63,9 @@ class RenderSlashUri(unittest.TestCase): response_body = self.successResultOf( render(self.res, query_args), ) - - soup = BeautifulSoup(response_body, 'html5lib') - - assert_soup_has_tag_with_content( - self, soup, "title", "400 - Error", - ) - assert_soup_has_tag_with_content( - self, soup, "h1", "Error", - ) - assert_soup_has_tag_with_content( - self, soup, "p", "Invalid capability", - ) - - @given( - text() - ) - def test_hypothesis_error_caps(self, cap): - """ - Let hypothesis try a bunch of invalid capabilities - """ - query_args = {b"uri": [cap.encode('utf8')]} - response_body = self.successResultOf( - render(self.res, query_args, - ), - ) - - soup = BeautifulSoup(response_body, 'html5lib') - - assert_soup_has_tag_with_content( - self, soup, "title", "400 - Error", - ) - assert_soup_has_tag_with_content( - self, soup, "h1", "Error", - ) - assert_soup_has_tag_with_content( - self, soup, "p", "Invalid capability", + self.assertEqual( + response_body, + "Invalid capability", ) From fa02e46033d38a5350eb9b92fdf8da8d5761cd06 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 16:02:32 -0400 Subject: [PATCH 07/83] maybeDeferred always returns a Deferred --- src/allmydata/web/common.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 441269d6e..1de1fc073 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -483,18 +483,8 @@ def render_exception(render): def g(self, request): bound_render = render.__get__(self, type(self)) result = maybeDeferred(bound_render, request) - if isinstance(result, Deferred): - result.addBoth(_finish, bound_render, request) - return NOT_DONE_YET - elif isinstance(result, bytes): - return result - elif result == NOT_DONE_YET: - return NOT_DONE_YET - else: - raise Exception("{!r} returned unusable {!r}".format( - fullyQualifiedName(bound_render), - result, - )) + result.addBoth(_finish, bound_render, request) + return NOT_DONE_YET return g From 0339ba97b9c2e14bff59c7d243957e865b0363c5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 15 Oct 2020 16:14:06 -0400 Subject: [PATCH 08/83] Turn getChild None and Deferred results into something Twisted Web can manage --- src/allmydata/web/common.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 1de1fc073..25bb5fbba 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -20,6 +20,7 @@ from twisted.web.server import ( UnsupportedMethod, ) from twisted.web.util import ( + DeferredResource, FailureElement, redirectTo, ) @@ -459,21 +460,35 @@ class TokenOnlyWebApi(resource.Resource, object): raise WebError("'%s' invalid type for 't' arg" % (t,), http.BAD_REQUEST) -def exception_to_child(f): +def exception_to_child(getChild): """ Decorate ``getChild`` method with exception handling behavior to render an error page reflecting the exception. """ - @wraps(f) + @wraps(getChild) 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) + bound_getChild = getChild.__get__(self, type(self)) + result = maybeDeferred(bound_getChild, name, req) + result.addCallbacks( + _getChild_done, + _getChild_failed, + callbackArgs=(self,), + ) + return DeferredResource(result) return g +def _getChild_done(child, parent): + if child is None: + return resource.NoResource() + return child + + +def _getChild_failed(reason): + text, code = humanize_failure(reason) + return resource.ErrorPage(code, "Error", text) + + def render_exception(render): """ Decorate a ``render_*`` method with exception handling behavior to render From 0faa24d34462bfc835dcd0ae6ba9e3386eb13874 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 09:30:09 -0400 Subject: [PATCH 09/83] Add a mess of eliot logging to request handling --- src/allmydata/web/common.py | 71 +++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 25bb5fbba..35228374c 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -7,6 +7,14 @@ from hyperlink import ( DecodedURL, ) +from eliot import ( + Message, + start_action, +) +from eliot.twisted import ( + DeferredContext, +) + from twisted.web import ( http, resource, @@ -468,17 +476,31 @@ def exception_to_child(getChild): @wraps(getChild) def g(self, name, req): bound_getChild = getChild.__get__(self, type(self)) - result = maybeDeferred(bound_getChild, name, req) - result.addCallbacks( - _getChild_done, - _getChild_failed, - callbackArgs=(self,), + + action = start_action( + action_type=u"allmydata:web:common-getChild", + uri=req.uri, + method=req.method, + name=name, + handler=fullyQualifiedName(bound_getChild), ) + with action.context(): + result = DeferredContext(maybeDeferred(bound_getChild, name, req)) + result.addCallbacks( + _getChild_done, + _getChild_failed, + callbackArgs=(self,), + ) + result = result.addActionFinish() return DeferredResource(result) return g def _getChild_done(child, parent): + Message.log( + message_type=u"allmydata:web:common-getChild:result", + result=fullyQualifiedName(type(child)), + ) if child is None: return resource.NoResource() return child @@ -497,8 +519,17 @@ def render_exception(render): @wraps(render) def g(self, request): bound_render = render.__get__(self, type(self)) - result = maybeDeferred(bound_render, request) - result.addBoth(_finish, bound_render, request) + + action = start_action( + action_type=u"allmydata:web:common-render", + uri=request.uri, + method=request.method, + handler=fullyQualifiedName(bound_render), + ) + with action.context(): + result = DeferredContext(maybeDeferred(bound_render, request)) + result.addBoth(_finish, bound_render, request) + result.addActionFinish() return NOT_DONE_YET return g @@ -506,6 +537,10 @@ def render_exception(render): def _finish(result, render, request): if isinstance(result, Failure): + Message.log( + message_type=u"allmydata:web:common-render:failure", + message=result.getErrorMessage(), + ) _finish( _renderHTTP_exception(request, result), render, @@ -516,23 +551,45 @@ def _finish(result, render, request): # double-apply the logic. This leads to an attempt to double-finish # the request. If it isn't using @render_exception then you should # fix it so it is. + Message.log( + message_type=u"allmydata:web:common-render:resource", + resource=fullyQualifiedName(type(result)), + ) result.render(request) elif isinstance(result, bytes): + Message.log( + message_type=u"allmydata:web:common-render:bytes", + ) request.write(result) request.finish() elif isinstance(result, URLPath): + Message.log( + message_type=u"allmydata:web:common-render:URLPath", + ) if result.netloc == b"": root = URLPath.fromRequest(request) result.scheme = root.scheme result.netloc = root.netloc _finish(redirectTo(str(result), request), render, request) elif isinstance(result, DecodedURL): + Message.log( + message_type=u"allmydata:web:common-render:DecodedURL", + ) _finish(redirectTo(str(result), request), render, request) elif result is None: + Message.log( + message_type=u"allmydata:web:common-render:None", + ) request.finish() elif result == NOT_DONE_YET: + Message.log( + message_type=u"allmydata:web:common-render:NOT_DONE_YET", + ) pass else: + Message.log( + message_type=u"allmydata:web:common-render:unknown", + ) log.err("Request for {!r} handled by {!r} returned unusable {!r}".format( request.uri, fullyQualifiedName(render), From 31207e4b6b482171a95070d5226177f021c4cf5b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 09:30:22 -0400 Subject: [PATCH 10/83] don't double-apply the renderer logic since that leads to double-finishing requests too --- src/allmydata/web/common.py | 4 ++++ src/allmydata/web/directory.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 35228374c..5a196678e 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -526,6 +526,10 @@ def render_exception(render): method=request.method, handler=fullyQualifiedName(bound_render), ) + if getattr(request, "dont_apply_extra_processing", False): + with action: + return bound_render(request) + with action.context(): result = DeferredContext(maybeDeferred(bound_render, request)) result.addBoth(_finish, bound_render, request) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 39bc297f5..0d521951f 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -400,7 +400,10 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # delegate to it. We could return the resource back out of # DirectoryNodeHandler.renderHTTP, and nevow would recurse into it, # but the addCallback() that handles when_done= would break. - d.addCallback(lambda child: child.render(req)) + def render_child(child): + req.dont_apply_extra_processing = True + return child.render(req) + d.addCallback(render_child) return d def _POST_uri(self, req): From a1f1f00be7cc7d70c530622c781f8d2f49e8b2c2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 09:43:24 -0400 Subject: [PATCH 11/83] Use the more feaetureful rendering helper --- src/allmydata/test/test_checker.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 882356aeb..0c8f05ad4 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -32,6 +32,9 @@ from allmydata.mutable.publish import MutableData from .common import ( EMPTY_CLIENT_CONFIG, ) +from .common_web import ( + render, +) from .web.common import ( assert_soup_has_favicon, @@ -164,11 +167,11 @@ class WebResultsRendering(unittest.TestCase): return c def render_json(self, resource): - return resource.render(TestRequest(args={"output": ["json"]})) + return self.successResultOf(render(resource, {"output": ["json"]})) def render_element(self, element, args=None): d = flattenString(TestRequest(args), element) - return unittest.TestCase().successResultOf(d) + return self.successResultOf(d) def test_literal(self): lcr = web_check_results.LiteralCheckResultsRendererElement() From a73a919a20dc1802751937e2096f8e3c74868cd6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 10:22:42 -0400 Subject: [PATCH 12/83] flakes --- src/allmydata/test/web/test_root.py | 6 ------ src/allmydata/web/common.py | 1 - src/allmydata/web/filenode.py | 6 ------ src/allmydata/web/operations.py | 1 - src/allmydata/web/unlinked.py | 1 - 5 files changed, 15 deletions(-) diff --git a/src/allmydata/test/web/test_root.py b/src/allmydata/test/web/test_root.py index 6b0885eb2..139441a6c 100644 --- a/src/allmydata/test/web/test_root.py +++ b/src/allmydata/test/web/test_root.py @@ -2,8 +2,6 @@ from mock import Mock import time -from bs4 import BeautifulSoup - from twisted.trial import unittest from twisted.web.template import Tag from twisted.web.test.requesthelper import DummyRequest @@ -18,10 +16,6 @@ from ...util.connection_status import ConnectionStatus from allmydata.web.root import URIHandler from allmydata.client import _Client -from .common import ( - assert_soup_has_tag_with_content, -) - from ..common_web import ( render, ) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 5a196678e..d9300f818 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -43,7 +43,6 @@ from twisted.python.failure import ( Failure, ) from twisted.internet.defer import ( - Deferred, maybeDeferred, ) from twisted.web.resource import ( diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index b36ce0af8..7686b35b5 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -1,10 +1,6 @@ import json -from hyperlink import ( - DecodedURL, -) - from twisted.web import http, static from twisted.internet import defer from twisted.web.resource import ( @@ -484,8 +480,6 @@ class FileDownloader(Resource, object): d = self.filenode.read(req, first, size) def _error(f): - lp = log.msg("error during GET", facility="tahoe.webish", failure=f, - level=log.UNUSUAL, umid="xSiF3w") req._tahoe_request_had_error = f # for HTTP-style logging if req.startedWriting: # The content-type is already set, and the response code has diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 25e267038..d4f407b31 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -19,7 +19,6 @@ from twisted.application import service from allmydata.web.common import ( WebError, - get_root, get_arg, boolean_of_arg, exception_to_child, diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 9cf315235..03f582c15 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -13,7 +13,6 @@ from twisted.web.template import ( renderElement, tags, ) -from nevow import url from allmydata.immutable.upload import FileHandle from allmydata.mutable.publish import MutableFileHandle from allmydata.web.common import ( From 28505ab57d0bc05b4d007955fae3dedc227bb8a9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 10:38:58 -0400 Subject: [PATCH 13/83] news fragment --- newsfragments/3428.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3428.minor diff --git a/newsfragments/3428.minor b/newsfragments/3428.minor new file mode 100644 index 000000000..e69de29bb From 960d1152866b41f49d224beb853b8fe36ff92a33 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 11:25:37 -0400 Subject: [PATCH 14/83] news fragment --- newsfragments/3481.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3481.minor diff --git a/newsfragments/3481.minor b/newsfragments/3481.minor new file mode 100644 index 000000000..e69de29bb From 75b3bf1097e11c7bc6ca8dd83e530fae1bd3ae76 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 11:26:05 -0400 Subject: [PATCH 15/83] Refuse to continue if the introducer fURL has no location hints. --- integration/conftest.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/integration/conftest.py b/integration/conftest.py index 04e3dcb52..802deb79d 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -8,6 +8,10 @@ from os.path import join, exists from tempfile import mkdtemp, mktemp from functools import partial +from foolscap.furl import ( + decode_furl, +) + from eliot import ( to_file, log_call, @@ -226,6 +230,16 @@ def introducer_furl(introducer, temp_dir): print("Don't see {} yet".format(furl_fname)) sleep(.1) furl = open(furl_fname, 'r').read() + tubID, location_hints, name = decode_furl(furl) + if not location_hints: + # If there are no location hints then nothing can ever possibly + # connect to it and the only thing that can happen next is something + # will hang or time out. So just give up right now. + raise ValueError( + "Introducer ({!r}) fURL has no location hints!".format( + introducer_furl, + ), + ) return furl From 64a9e95319d5ef1bb3a52c6471e353aca0979703 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 11:27:13 -0400 Subject: [PATCH 16/83] Pass PATH (and other stuff) into the child process --- integration/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/util.py b/integration/util.py index bbcf5efc6..a64bcbf8e 100644 --- a/integration/util.py +++ b/integration/util.py @@ -1,7 +1,7 @@ import sys import time import json -from os import mkdir +from os import mkdir, environ from os.path import exists, join from six.moves import StringIO from functools import partial @@ -145,6 +145,7 @@ def _tahoe_runner_optional_coverage(proto, reactor, request, other_args): proto, sys.executable, args, + env=environ, ) From a22426011be6d0ed39a329bc9cb8794226f41af4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 12:49:36 -0400 Subject: [PATCH 17/83] import and naming cleanups post-merge --- src/allmydata/web/common.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 39073fe84..b5afe3fa2 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -50,9 +50,6 @@ from twisted.internet.defer import ( from twisted.web.resource import ( IResource, ) -from nevow import appserver -from nevow.inevow import IRequest - from twisted.web.iweb import IRequest as ITwistedRequest from twisted.python import log if PY2: @@ -376,7 +373,7 @@ def humanize_failure(f): class MyExceptionHandler(DefaultExceptionHandler, object): def renderHTTP_exception(self, ctx, f): - req = IRequest(ctx) + req = INevowRequest(ctx) req.write(_renderHTTP_exception(req, f)) req.finishRequest(False) From 7b02f58da0cbe00cb6e281622505ca2c194fc852 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 13:15:45 -0400 Subject: [PATCH 18/83] Make this test tolerant of more than one message It is observing the log system. It is reasonable to believe more than one log event might come through over the course of the test. We only need one, though. --- integration/test_streaming_logs.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/integration/test_streaming_logs.py b/integration/test_streaming_logs.py index 32b97644d..b921937f1 100644 --- a/integration/test_streaming_logs.py +++ b/integration/test_streaming_logs.py @@ -53,7 +53,12 @@ class _StreamingLogClientProtocol(WebSocketClientProtocol): self.factory.on_open.callback(self) def onMessage(self, payload, isBinary): - self.on_message.callback(payload) + if self.on_message is None: + # Already did our job, ignore it + return + on_message = self.on_message + self.on_message = None + on_message.callback(payload) def onClose(self, wasClean, code, reason): self.on_close.callback(reason) From 292f136547d209b87fb28b7f43f2fa02a1a72150 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 13:21:07 -0400 Subject: [PATCH 19/83] pyflakes --- src/allmydata/web/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index b5afe3fa2..4fb2372a5 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -51,7 +51,6 @@ from twisted.web.resource import ( IResource, ) from twisted.web.iweb import IRequest as ITwistedRequest -from twisted.python import log if PY2: from nevow.appserver import DefaultExceptionHandler from nevow.inevow import IRequest as INevowRequest From 9e26599a7677453722b1de7b1756cd1675f81603 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 13:44:37 -0400 Subject: [PATCH 20/83] Fix the race condition --- integration/test_streaming_logs.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/integration/test_streaming_logs.py b/integration/test_streaming_logs.py index b921937f1..52c813f9b 100644 --- a/integration/test_streaming_logs.py +++ b/integration/test_streaming_logs.py @@ -136,10 +136,13 @@ def _test_streaming_logs(reactor, temp_dir, alice): client.on_close = Deferred() client.on_message = Deferred() + # Capture this now before on_message perhaps goes away. + racing = _race(client.on_close, client.on_message) + # Provoke _some_ log event. yield treq.get(node_url) - result = yield _race(client.on_close, client.on_message) + result = yield racing assert isinstance(result, Right) json.loads(result.value) From 85bb0a7834ed003c32f474ec1cc7b8a2451cbb24 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 14:14:15 -0400 Subject: [PATCH 21/83] Get rid of the original token-based authorization helper It was only used by magic-folder and that's gone now. We have a different authorization helper for other things now, allmydata.web.private. --- src/allmydata/test/web/test_token.py | 105 --------------------------- src/allmydata/web/common.py | 54 -------------- 2 files changed, 159 deletions(-) delete mode 100644 src/allmydata/test/web/test_token.py diff --git a/src/allmydata/test/web/test_token.py b/src/allmydata/test/web/test_token.py deleted file mode 100644 index a439702af..000000000 --- a/src/allmydata/test/web/test_token.py +++ /dev/null @@ -1,105 +0,0 @@ -from zope.interface import implementer -from twisted.trial import unittest -from twisted.web import server -from nevow.inevow import IRequest -from allmydata.web import common - -# XXX FIXME when we introduce "mock" as a dependency, these can -# probably just be Mock instances -@implementer(IRequest) -class FakeRequest(object): - def __init__(self): - self.method = "POST" - self.fields = dict() - self.args = dict() - - -class FakeField(object): - def __init__(self, *values): - if len(values) == 1: - self.value = values[0] - else: - self.value = list(values) - - -class FakeClientWithToken(object): - token = 'a' * 32 - - def get_auth_token(self): - return self.token - - -class TestTokenOnlyApi(unittest.TestCase): - - def setUp(self): - self.client = FakeClientWithToken() - self.page = common.TokenOnlyWebApi(self.client) - - def test_not_post(self): - req = FakeRequest() - req.method = "GET" - - self.assertRaises( - server.UnsupportedMethod, - self.page.render, req, - ) - - def test_missing_token(self): - req = FakeRequest() - - exc = self.assertRaises( - common.WebError, - self.page.render, req, - ) - self.assertEquals(exc.text, "Missing token") - self.assertEquals(exc.code, 401) - - def test_token_in_get_args(self): - req = FakeRequest() - req.args['token'] = 'z' * 32 - - exc = self.assertRaises( - common.WebError, - self.page.render, req, - ) - self.assertEquals(exc.text, "Do not pass 'token' as URL argument") - self.assertEquals(exc.code, 400) - - def test_invalid_token(self): - wrong_token = 'b' * 32 - req = FakeRequest() - req.fields['token'] = FakeField(wrong_token) - - exc = self.assertRaises( - common.WebError, - self.page.render, req, - ) - self.assertEquals(exc.text, "Invalid token") - self.assertEquals(exc.code, 401) - - def test_valid_token_no_t_arg(self): - req = FakeRequest() - req.fields['token'] = FakeField(self.client.token) - - with self.assertRaises(common.WebError) as exc: - self.page.render(req) - self.assertEquals(exc.exception.text, "Must provide 't=' argument") - self.assertEquals(exc.exception.code, 400) - - def test_valid_token_invalid_t_arg(self): - req = FakeRequest() - req.fields['token'] = FakeField(self.client.token) - req.args['t'] = 'not at all json' - - with self.assertRaises(common.WebError) as exc: - self.page.render(req) - self.assertTrue("invalid type" in exc.exception.text) - self.assertEquals(exc.exception.code, 400) - - def test_valid(self): - req = FakeRequest() - req.fields['token'] = FakeField(self.client.token) - req.args['t'] = ['json'] - - result = self.page.render(req) - self.assertTrue(result == NotImplemented) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 5c27f20ab..16970e5e2 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -37,7 +37,6 @@ from allmydata.interfaces import ( 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, @@ -426,59 +425,6 @@ class SlotsSequenceElement(template.Element): return tag -class TokenOnlyWebApi(resource.Resource, object): - """ - I provide a rend.Page implementation that only accepts POST calls, - and only if they have a 'token=' arg with the correct - authentication token (see - :meth:`allmydata.client.Client.get_auth_token`). Callers must also - provide the "t=" argument to indicate the return-value (the only - valid value for this is "json") - - Subclasses should override 'post_json' which should process the - API call and return a string which encodes a valid JSON - object. This will only be called if the correct token is present - and valid (during renderHTTP processing). - """ - - def __init__(self, client): - self.client = client - - def post_json(self, req): - return NotImplemented - - def render(self, req): - if req.method != 'POST': - raise server.UnsupportedMethod(('POST',)) - if req.args.get('token', False): - raise WebError("Do not pass 'token' as URL argument", http.BAD_REQUEST) - # not using get_arg() here because we *don't* want the token - # argument to work if you passed it as a GET-style argument - token = None - if req.fields and 'token' in req.fields: - token = req.fields['token'].value.strip() - if not token: - raise WebError("Missing token", http.UNAUTHORIZED) - if not timing_safe_compare(token, self.client.get_auth_token()): - raise WebError("Invalid token", http.UNAUTHORIZED) - - t = get_arg(req, "t", "").strip() - if not t: - raise WebError("Must provide 't=' argument") - if t == u'json': - try: - return self.post_json(req) - except WebError as e: - req.setResponseCode(e.code) - return json.dumps({"error": e.text}) - except Exception as e: - 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 From 540004f3199cf67d6bbc53b545d7d3e43a7e0da7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 14:18:32 -0400 Subject: [PATCH 22/83] news fragment --- newsfragments/3482.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3482.minor diff --git a/newsfragments/3482.minor b/newsfragments/3482.minor new file mode 100644 index 000000000..e69de29bb From 84acf4e50fc0233ac070e07dbcfde9fb3c98e7f8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sun, 18 Oct 2020 10:58:09 -0400 Subject: [PATCH 23/83] Accept unicode return values and encode them to UTF-8 Nevow accepts unicode in most places it accepts bytes and does the usual sloppy Python 2 thing, lets one or the other get implicitly re-coded, typically using the ascii codec. We'll go with UTF-8 because that fails less often than ASCII. We may want to clean up the code at some point so we're not accidentally slinging both bytes and text around as if they were the same thing. --- src/allmydata/web/common.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 4fb2372a5..a3f1688d1 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -571,6 +571,12 @@ def _finish(result, render, request): resource=fullyQualifiedName(type(result)), ) result.render(request) + elif isinstance(result, unicode): + Message.log( + message_type=u"allmydata:web:common-render:unicode", + ) + request.write(result.encode("utf-8")) + request.finish() elif isinstance(result, bytes): Message.log( message_type=u"allmydata:web:common-render:bytes", From e710fd883a6ae9443dca7f515767161707e8caf1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sun, 18 Oct 2020 11:00:57 -0400 Subject: [PATCH 24/83] Add a Twisted Web-based renderer and start using it on Python 3 This could be the thing that eventually replaces the Nevow-based testing renderer on Python 2 as well. --- src/allmydata/test/common_nevow.py | 46 ++++++++++++++++++++++++ src/allmydata/test/common_tweb.py | 54 ++++++++++++++++++++++++++++ src/allmydata/test/common_web.py | 56 ++++++++---------------------- 3 files changed, 114 insertions(+), 42 deletions(-) create mode 100644 src/allmydata/test/common_nevow.py create mode 100644 src/allmydata/test/common_tweb.py diff --git a/src/allmydata/test/common_nevow.py b/src/allmydata/test/common_nevow.py new file mode 100644 index 000000000..dcf4dc095 --- /dev/null +++ b/src/allmydata/test/common_nevow.py @@ -0,0 +1,46 @@ +""" +General helpers related to Nevow. +""" + +from nevow.context import WebContext +from nevow.testutil import FakeRequest +from nevow.appserver import ( + processingFailed, + DefaultExceptionHandler, +) +from nevow.inevow import ( + ICanHandleException, + IRequest, + IResource as INevowResource, + IData, +) + +def render(resource, query_args): + """ + Render (in the manner of the Nevow appserver) a Nevow ``Page`` or a + Twisted ``Resource`` against a request with the given query arguments . + + :param resource: The page or resource to render. + + :param query_args: The query arguments to put into the request being + rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. + + :return Deferred: A Deferred that fires with the rendered response body as + ``bytes``. + """ + ctx = WebContext(tag=resource) + req = FakeRequest(args=query_args) + ctx.remember(DefaultExceptionHandler(), ICanHandleException) + ctx.remember(req, IRequest) + ctx.remember(None, IData) + + def maybe_concat(res): + if isinstance(res, bytes): + return req.v + res + return req.v + + resource = INevowResource(resource) + d = maybeDeferred(resource.renderHTTP, ctx) + d.addErrback(processingFailed, req, ctx) + d.addCallback(maybe_concat) + return d diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py new file mode 100644 index 000000000..2af9d4f53 --- /dev/null +++ b/src/allmydata/test/common_tweb.py @@ -0,0 +1,54 @@ +from zope.interface import ( + classImplements, +) +from twisted.python.reflect import ( + fullyQualifiedName, +) +from twisted.internet.defer import ( + succeed, +) +from twisted.web.test.requesthelper import ( + DummyRequest, +) +from twisted.web.iweb import ( + IRequest, +) +from twisted.web.server import ( + NOT_DONE_YET, +) + +classImplements(DummyRequest, IRequest) + +def render(resource, query_args): + """ + Render (in the manner of the Twisted Web Site) a Twisted ``Resource`` + against a request with the given query arguments . + + :param resource: The page or resource to render. + + :param query_args: The query arguments to put into the request being + rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. + + :return Deferred: A Deferred that fires with the rendered response body as + ``bytes``. + """ + request = DummyRequest([]) + request.args = query_args + result = resource.render(request) + if isinstance(result, bytes): + request.write(result) + done = succeed(None) + elif result == NOT_DONE_YET: + if request.finished: + done = succeed(None) + else: + done = request.notifyFinish() + else: + raise ValueError( + "{!r} returned {!r}, required bytes or NOT_DONE_YET.".format( + fullyQualifiedName(resource.render), + result, + ), + ) + done.addCallback(lambda ignored: b"".join(request.written)) + return done diff --git a/src/allmydata/test/common_web.py b/src/allmydata/test/common_web.py index e2ea57539..829eef153 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -1,3 +1,4 @@ +from future.utils import PY2 import treq from twisted.internet.defer import ( @@ -7,19 +8,6 @@ from twisted.internet.defer import ( ) from twisted.web.error import Error -from nevow.context import WebContext -from nevow.testutil import FakeRequest -from nevow.appserver import ( - processingFailed, - DefaultExceptionHandler, -) -from nevow.inevow import ( - ICanHandleException, - IRequest, - IResource as INevowResource, - IData, -) - @inlineCallbacks def do_http(method, url, **kwargs): response = yield treq.request(method, url, persistent=False, **kwargs) @@ -31,32 +19,16 @@ def do_http(method, url, **kwargs): returnValue(body) -def render(resource, query_args): - """ - Render (in the manner of the Nevow appserver) a Nevow ``Page`` or a - Twisted ``Resource`` against a request with the given query arguments . - - :param resource: The page or resource to render. - - :param query_args: The query arguments to put into the request being - rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. - - :return Deferred: A Deferred that fires with the rendered response body as - ``bytes``. - """ - ctx = WebContext(tag=resource) - req = FakeRequest(args=query_args) - ctx.remember(DefaultExceptionHandler(), ICanHandleException) - ctx.remember(req, IRequest) - ctx.remember(None, IData) - - def maybe_concat(res): - if isinstance(res, bytes): - return req.v + res - return req.v - - resource = INevowResource(resource) - d = maybeDeferred(resource.renderHTTP, ctx) - d.addErrback(processingFailed, req, ctx) - d.addCallback(maybe_concat) - return d +if PY2: + # We can only use Nevow on Python 2 and Tahoe-LAFS still *does* use Nevow + # so prefer the Nevow-based renderer if we can get it. + from .common_nevow import ( + render, + ) +else: + # However, Tahoe-LAFS *will* use Twisted Web before too much longer so go + # ahead and let some tests run against the Twisted Web-based renderer on + # Python 3. Later this will become the only codepath. + from .common_tweb import ( + render, + ) From 72e60f83011e0e52f3f8d7bd7855ec8986f671e5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sun, 18 Oct 2020 11:12:44 -0400 Subject: [PATCH 25/83] Fix imports --- src/allmydata/test/common_nevow.py | 4 ++++ src/allmydata/test/common_web.py | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/common_nevow.py b/src/allmydata/test/common_nevow.py index dcf4dc095..d6327f9c6 100644 --- a/src/allmydata/test/common_nevow.py +++ b/src/allmydata/test/common_nevow.py @@ -2,6 +2,10 @@ General helpers related to Nevow. """ +from twisted.internet.defer import ( + maybeDeferred, +) + from nevow.context import WebContext from nevow.testutil import FakeRequest from nevow.appserver import ( diff --git a/src/allmydata/test/common_web.py b/src/allmydata/test/common_web.py index 829eef153..08b356cd0 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -1,8 +1,13 @@ + +__all__ = [ + "do_http", + "render", +] + from future.utils import PY2 import treq from twisted.internet.defer import ( - maybeDeferred, inlineCallbacks, returnValue, ) From b68c08cff96b7593f47eab56fd455e9fee4bd731 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 19 Oct 2020 12:33:15 -0400 Subject: [PATCH 26/83] Yank direct support for URLPath from common.py --- src/allmydata/web/common.py | 43 +++++++++++++++++++++++++---------- src/allmydata/web/unlinked.py | 7 +++--- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index a3f1688d1..56bef9dfc 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -37,9 +37,6 @@ from twisted.web.util import ( from twisted.python.reflect import ( fullyQualifiedName, ) -from twisted.python.urlpath import ( - URLPath, -) from twisted.python import log from twisted.python.failure import ( Failure, @@ -583,15 +580,6 @@ def _finish(result, render, request): ) request.write(result) request.finish() - elif isinstance(result, URLPath): - Message.log( - message_type=u"allmydata:web:common-render:URLPath", - ) - if result.netloc == b"": - root = URLPath.fromRequest(request) - result.scheme = root.scheme - result.netloc = root.netloc - _finish(redirectTo(str(result), request), render, request) elif isinstance(result, DecodedURL): Message.log( message_type=u"allmydata:web:common-render:DecodedURL", @@ -684,3 +672,34 @@ def handle_when_done(req, d): if when_done: d.addCallback(lambda res: DecodedURL.from_text(when_done.decode("utf-8"))) return d + + +def url_for_string(req, url_string): + """ + Construct a universal URL using the given URL string. + + :param IRequest req: The request being served. If ``redir_to`` is not + absolute then this is used to determine the net location of this + server and the resulting URL is made to point at it. + + :param bytes url_string: A byte string giving a universal or absolute URL. + + :return DecodedURL: An absolute URL based on this server's net location + and the given URL string. + """ + url = DecodedURL.from_text(url_string.decode("utf-8")) + if url.host == b"": + root = req.URLPath() + netloc = root.netloc.split(b":", 1) + if len(netloc) == 1: + host = netloc + port = None + else: + host = netloc[0] + port = int(netloc[1]) + url = url.replace( + scheme=root.scheme.decode("ascii"), + host=host.decode("ascii"), + port=port, + ) + return url diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 03f582c15..e420a0371 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -1,10 +1,8 @@ import urllib + from twisted.web import http from twisted.internet import defer -from twisted.python.urlpath import ( - URLPath, -) from twisted.python.filepath import FilePath from twisted.web.resource import Resource from twisted.web.template import ( @@ -23,6 +21,7 @@ from allmydata.web.common import ( get_format, get_mutable_type, render_exception, + url_for_string, ) from allmydata.web import status @@ -68,7 +67,7 @@ def POSTUnlinkedCHK(req, client): def _done(upload_results, redir_to): if "%(uri)s" in redir_to: redir_to = redir_to.replace("%(uri)s", urllib.quote(upload_results.get_uri())) - return URLPath.fromString(redir_to) + return url_for_string(req, redir_to) d.addCallback(_done, when_done) else: # return the Upload Results page, which includes the URI From a9e9efb3361df9fca0621cfceb9338ca4274205c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 19 Oct 2020 14:26:12 -0400 Subject: [PATCH 27/83] direct tests for `@render_exception` --- src/allmydata/test/web/test_common.py | 211 ++++++++++++++++++++++++++ src/allmydata/web/common.py | 2 +- 2 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 src/allmydata/test/web/test_common.py diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py new file mode 100644 index 000000000..f9152e126 --- /dev/null +++ b/src/allmydata/test/web/test_common.py @@ -0,0 +1,211 @@ +""" +Tests for ``allmydata.web.common``. +""" + +from bs4 import ( + BeautifulSoup, +) +from hyperlink import ( + DecodedURL, +) + +from testtools.matchers import ( + Equals, + Contains, + MatchesPredicate, +) +from testtools.twistedsupport import ( + succeeded, + has_no_result, +) + +from twisted.internet.defer import ( + fail, +) +from twisted.web.server import ( + NOT_DONE_YET, +) +from twisted.web.resource import ( + Resource, +) + +from ...web.common import ( + render_exception, +) + +from ..common import ( + SyncTestCase, +) +from ..common_web import ( + render, +) +from .common import ( + assert_soup_has_tag_with_attributes, +) + +class StaticResource(Resource): + def __init__(self, response): + Resource.__init__(self) + self._response = response + + @render_exception + def render(self, request): + return self._response + + +class RenderExceptionTests(SyncTestCase): + """ + Tests for ``render_exception``. + """ + def test_exception(self): + """ + If the decorated method raises an exception then the exception is rendered + into the response. + """ + class R(Resource): + @render_exception + def render(self, request): + raise Exception("synthetic exception") + + self.assertThat( + render(R(), {}), + succeeded( + Contains(b"synthetic exception"), + ), + ) + + def test_failure(self): + """ + If the decorated method returns a ``Deferred`` that fires with a + ``Failure`` then the exception the ``Failure`` wraps is rendered into + the response. + """ + resource = StaticResource(fail(Exception("synthetic exception"))) + self.assertThat( + render(resource, {}), + succeeded( + Contains(b"synthetic exception"), + ), + ) + + def test_resource(self): + """ + If the decorated method returns an ``IResource`` provider then that + resource is used to render the response. + """ + resource = StaticResource(StaticResource(b"static result")) + self.assertThat( + render(resource, {}), + succeeded( + Equals(b"static result"), + ), + ) + + def test_unicode(self): + """ + If the decorated method returns a ``unicode`` string then that string is + UTF-8 encoded and rendered into the response. + """ + text = u"\N{SNOWMAN}" + resource = StaticResource(text) + self.assertThat( + render(resource, {}), + succeeded( + Equals(text.encode("utf-8")), + ), + ) + + def test_bytes(self): + """ + If the decorated method returns a ``bytes`` string then that string is + rendered into the response. + """ + data = b"hello world" + resource = StaticResource(data) + self.assertThat( + render(resource, {}), + succeeded( + Equals(data), + ), + ) + + def test_decodedurl(self): + """ + If the decorated method returns a ``DecodedURL`` then a redirect to that + location is rendered into the response. + """ + loc = u"http://example.invalid/foo?bar=baz" + resource = StaticResource(DecodedURL.from_text(loc)) + self.assertThat( + render(resource, {}), + succeeded( + MatchesPredicate( + lambda value: assert_soup_has_tag_with_attributes( + self, + BeautifulSoup(value), + "meta", + {"http-equiv": "refresh", + "content": "0;URL={}".format(loc.encode("ascii")), + }, + ) + # The assertion will raise if it has a problem, otherwise + # return None. Turn the None into something + # MatchesPredicate recognizes as success. + or True, + "did not find meta refresh tag in %r", + ), + ), + ) + + def test_none(self): + """ + If the decorated method returns ``None`` then the response is finished + with no additional content. + """ + self.assertThat( + render(StaticResource(None), {}), + succeeded( + Equals(b""), + ), + ) + + def test_not_done_yet(self): + """ + If the decorated method returns ``NOT_DONE_YET`` then the resource is + responsible for finishing the request itself. + """ + the_request = [] + class R(Resource): + @render_exception + def render(self, request): + the_request.append(request) + return NOT_DONE_YET + + d = render(R(), {}) + + self.assertThat( + d, + has_no_result(), + ) + + the_request[0].write(b"some content") + the_request[0].finish() + + self.assertThat( + d, + succeeded( + Equals(b"some content"), + ), + ) + + def test_unknown(self): + """ + If the decorated method returns something which is not explicitly + supported, an internal server error is rendered into the response. + """ + self.assertThat( + render(StaticResource(object()), {}), + succeeded( + Equals(b"Internal Server Error"), + ), + ) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 56bef9dfc..215abb46e 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -605,7 +605,7 @@ def _finish(result, render, request): result, )) request.setResponseCode(http.INTERNAL_SERVER_ERROR) - _finish(b"", render, request) + _finish(b"Internal Server Error", render, request) def _renderHTTP_exception(request, failure): From 3192715e37c0bc3fd4bb217efabc86c493723392 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 19 Oct 2020 14:37:03 -0400 Subject: [PATCH 28/83] everything newstyle --- src/allmydata/test/web/test_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py index f9152e126..b399b77c8 100644 --- a/src/allmydata/test/web/test_common.py +++ b/src/allmydata/test/web/test_common.py @@ -43,7 +43,7 @@ from .common import ( assert_soup_has_tag_with_attributes, ) -class StaticResource(Resource): +class StaticResource(Resource, object): def __init__(self, response): Resource.__init__(self) self._response = response From e7c04f8810639b71222da3fec30f6710d289c51c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 19 Oct 2020 15:41:38 -0400 Subject: [PATCH 29/83] another news fragment! --- newsfragments/3314.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3314.minor diff --git a/newsfragments/3314.minor b/newsfragments/3314.minor new file mode 100644 index 000000000..e69de29bb From 1ed74604c7e3fd4fbd02549d24a52707b88ddced Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:15:36 -0400 Subject: [PATCH 30/83] Use twisted.web.server.Request instead of DummyRequest in the tests Always prefer the real thing if possible --- src/allmydata/test/common_tweb.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py index 2af9d4f53..cfaf85c6c 100644 --- a/src/allmydata/test/common_tweb.py +++ b/src/allmydata/test/common_tweb.py @@ -8,7 +8,10 @@ from twisted.internet.defer import ( succeed, ) from twisted.web.test.requesthelper import ( - DummyRequest, + DummyChannel, +) +from twisted.web.server import ( + Request, ) from twisted.web.iweb import ( IRequest, @@ -17,8 +20,6 @@ from twisted.web.server import ( NOT_DONE_YET, ) -classImplements(DummyRequest, IRequest) - def render(resource, query_args): """ Render (in the manner of the Twisted Web Site) a Twisted ``Resource`` @@ -32,7 +33,8 @@ def render(resource, query_args): :return Deferred: A Deferred that fires with the rendered response body as ``bytes``. """ - request = DummyRequest([]) + channel = DummyChannel() + request = Request(channel) request.args = query_args result = resource.render(request) if isinstance(result, bytes): @@ -50,5 +52,9 @@ def render(resource, query_args): result, ), ) - done.addCallback(lambda ignored: b"".join(request.written)) + def get_body(ignored): + complete_response = channel.transport.written.getvalue() + header, body = complete_response.split(b"\r\n\r\n", 1) + return body + done.addCallback(get_body) return done From a91dba5f5bb07a68ea561880a8215f1516803e13 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:18:41 -0400 Subject: [PATCH 31/83] _finish docstring --- src/allmydata/web/common.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 215abb46e..3338b4a1f 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -548,6 +548,23 @@ def render_exception(render): def _finish(result, render, request): + """ + Try to finish rendering the response to a request. + + This implements extra convenience functionality not provided by Twisted + Web. Various resources in Tahoe-LAFS made use of this functionality when + it was provided by Nevow. Rather than making that application code do the + more tedious thing itself, we duplicate the functionality here. + + :param result: Something returned by a render method which we can turn + into a response. + + :param render: The original render method which produced the result. + + :param request: The request being responded to. + + :return: ``None`` + """ if isinstance(result, Failure): Message.log( message_type=u"allmydata:web:common-render:failure", From dbe2d4efd724afdb1e3cc39aad1b479bde09f429 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:22:00 -0400 Subject: [PATCH 32/83] It isn't the root, it's wherever we actually are --- src/allmydata/web/operations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index b19366ff6..0e53d075d 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -90,8 +90,8 @@ class OphandleTable(resource.Resource, service.Service): """ ophandle = get_arg(req, "ophandle").decode("utf-8") assert ophandle - root = DecodedURL.from_text(unicode(URLPath.fromRequest(req))) - target = root.click(u"/").child(u"operations", ophandle) + here = DecodedURL.from_text(unicode(URLPath.fromRequest(req))) + target = here.click(u"/").child(u"operations", ophandle) output = get_arg(req, "output") if output: target = target.add(u"output", output.decode("utf-8")) From d8b6e36c6f6c024d8227893ea27b19fd377247c1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:23:16 -0400 Subject: [PATCH 33/83] docstring for StaticResource --- src/allmydata/test/web/test_common.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py index b399b77c8..5bc8ce4f0 100644 --- a/src/allmydata/test/web/test_common.py +++ b/src/allmydata/test/web/test_common.py @@ -44,6 +44,11 @@ from .common import ( ) class StaticResource(Resource, object): + """ + ``StaticResource`` is a resource that returns whatever Python object it is + given from its render method. This is useful for testing + ``render_exception``\\ 's handling of different render results. + """ def __init__(self, response): Resource.__init__(self) self._response = response From e8761c98ad0ed989a2990514fd419c55c26942d4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:25:48 -0400 Subject: [PATCH 34/83] A comment about what `_finish` is here for --- src/allmydata/web/common.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 3338b4a1f..038340fe8 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -540,6 +540,8 @@ def render_exception(render): with action.context(): result = DeferredContext(maybeDeferred(bound_render, request)) + # Apply `_finish` all of our result handling logic to whatever it + # returned. result.addBoth(_finish, bound_render, request) result.addActionFinish() return NOT_DONE_YET From 7ce2122e71bf28949ceb703726d4f05638be8515 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:26:12 -0400 Subject: [PATCH 35/83] one more reference --- src/allmydata/test/web/test_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py index 5bc8ce4f0..6431ba610 100644 --- a/src/allmydata/test/web/test_common.py +++ b/src/allmydata/test/web/test_common.py @@ -60,7 +60,7 @@ class StaticResource(Resource, object): class RenderExceptionTests(SyncTestCase): """ - Tests for ``render_exception``. + Tests for ``render_exception`` (including the private helper ``_finish``). """ def test_exception(self): """ From f602382244bff91e92d17e67f603a031f5279575 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 07:34:27 -0400 Subject: [PATCH 36/83] Comments about __get__ calls --- src/allmydata/web/common.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 038340fe8..6c8b557fa 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -483,6 +483,8 @@ def exception_to_child(getChild): """ @wraps(getChild) def g(self, name, req): + # Bind the method to the instance so it has a better + # fullyQualifiedName later on. This is not necessary on Python 3. bound_getChild = getChild.__get__(self, type(self)) action = start_action( @@ -526,6 +528,8 @@ def render_exception(render): """ @wraps(render) def g(self, request): + # Bind the method to the instance so it has a better + # fullyQualifiedName later on. This is not necessary on Python 3. bound_render = render.__get__(self, type(self)) action = start_action( From 0dcc3e13c037ef0860636ad290a1eaeff8710213 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 08:21:29 -0400 Subject: [PATCH 37/83] Remove unused imports --- src/allmydata/test/common_tweb.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py index cfaf85c6c..37e24b5b8 100644 --- a/src/allmydata/test/common_tweb.py +++ b/src/allmydata/test/common_tweb.py @@ -1,6 +1,3 @@ -from zope.interface import ( - classImplements, -) from twisted.python.reflect import ( fullyQualifiedName, ) @@ -13,9 +10,6 @@ from twisted.web.test.requesthelper import ( from twisted.web.server import ( Request, ) -from twisted.web.iweb import ( - IRequest, -) from twisted.web.server import ( NOT_DONE_YET, ) From 7f021289732fd04f016bf3418bf3981bfe987a19 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 06:59:59 -0400 Subject: [PATCH 38/83] [wip] test form posts --- src/allmydata/test/web/test_webish.py | 138 ++++++++++++++++++++++++++ src/allmydata/webish.py | 24 ++++- 2 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 src/allmydata/test/web/test_webish.py diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py new file mode 100644 index 000000000..52ba049a0 --- /dev/null +++ b/src/allmydata/test/web/test_webish.py @@ -0,0 +1,138 @@ +""" +Tests for ``allmydata.webish``. +""" + +from io import ( + BytesIO, +) +from uuid import ( + uuid4, +) + +from testtools.matchers import ( + AfterPreprocessing, + Equals, +) + +from twisted.python.filepath import ( + FilePath, +) +from twisted.web.test.requesthelper import ( + DummyChannel, +) +from twisted.web.client import ( + FileBodyProducer, +) +from twisted.internet.task import ( + Cooperator, +) + +from treq.multipart import ( + MultiPartProducer, +) + +from ..common import ( + SyncTestCase, +) + +from ...webish import ( + TahoeLAFSRequest, +) + + +class TahoeLAFSRequestTests(SyncTestCase): + """ + Tests for ``TahoeLAFSRequest``. + """ + def _fields_test(self, method, request_headers, request_body, match_fields): + channel = DummyChannel() + request = TahoeLAFSRequest( + channel, + ) + for (k, v) in request_headers.items(): + request.requestHeaders.setRawHeaders(k, [v]) + request.gotLength(len(request_body)) + request.handleContentChunk(request_body) + request.requestReceived(method, b"/", b"HTTP/1.1") + + # We don't really care what happened to the request. What we do care + # about is what the `fields` attribute is set to. + self.assertThat( + request.fields, + match_fields, + ) + + def test_no_form_fields(self): + """ + When a ``GET`` request is received, ``TahoeLAFSRequest.fields`` is None. + """ + self._fields_test(b"GET", {}, b"", Equals(None)) + + def test_form_fields(self): + """ + When a ``POST`` request is received, form fields are parsed into + ``TahoeLAFSRequest.fields``. + """ + form_data, boundary = multipart_formdata([ + [param(u"name", u"foo"), + body(u"bar"), + ], + [param(u"name", u"baz"), + param(u"filename", u"quux"), + body(u"some file contents"), + ], + ]) + self._fields_test( + b"POST", + {b"content-type": b"multipart/form-data; boundary={}".format(boundary)}, + form_data.encode("ascii"), + AfterPreprocessing( + lambda fs: { + k: fs.getvalue(k) + for k + in fs.keys() + }, + Equals({ + b"foo": b"bar", + b"baz": b"some file contents", + }), + ), + ) + + +def param(name, value): + return u"; {}={}".format(name, value) + + +def body(value): + return u"\r\n\r\n{}".format(value) + + +def _field(field): + yield u"Content-Disposition: form-data" + for param in field: + yield param + + +def _multipart_formdata(fields): + for field in fields: + yield u"".join(_field(field)) + u"\r\n" + + +def multipart_formdata(fields): + """ + Serialize some simple fields into a multipart/form-data string. + + :param fields: A list of lists of unicode strings to assemble into the + result. See ``param`` and ``body``. + + :return unicode: The given fields combined into a multipart/form-data + string. + """ + boundary = str(uuid4()) + parts = list(_multipart_formdata(fields)) + parts.insert(0, u"") + return ( + (u"--" + boundary + u"\r\n").join(parts), + boundary, + ) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 432a8cf2f..b8b9de83a 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,6 +1,11 @@ import re, time + +from cgi import ( + FieldStorage, +) + from twisted.application import service, strports, internet -from twisted.web import http, static +from twisted.web import http, server, static from twisted.internet import defer from twisted.internet.address import ( IPv4Address, @@ -25,7 +30,7 @@ from .web.storage_plugins import ( # surgery may induce a dependency upon a particular version of twisted.web parse_qs = http.parse_qs -class MyRequest(appserver.NevowRequest, object): +class TahoeLAFSRequest(server.Request, object): fields = None _tahoe_request_had_error = None @@ -34,7 +39,7 @@ class MyRequest(appserver.NevowRequest, object): This method is not intended for users. """ - self.content.seek(0,0) + self.content.seek(0) self.args = {} self.stack = [] self.setHeader("Referrer-Policy", "no-referrer") @@ -53,6 +58,17 @@ class MyRequest(appserver.NevowRequest, object): # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options self.responseHeaders.setRawHeaders("X-Frame-Options", ["DENY"]) + if self.method == 'POST': + self.fields = FieldStorage( + self.content, + { + name.lower(): value[-1] + for (name, value) + in self.requestHeaders.getAllRawHeaders() + }, + environ={'REQUEST_METHOD': 'POST'}) + self.content.seek(0) + # Argument processing. ## The original twisted.web.http.Request.requestReceived code parsed the @@ -176,7 +192,7 @@ class WebishServer(service.MultiService): def buildServer(self, webport, nodeurl_path, staticdir): self.webport = webport self.site = site = appserver.NevowSite(self.root) - self.site.requestFactory = MyRequest + self.site.requestFactory = TahoeLAFSRequest self.site.remember(MyExceptionHandler(), inevow.ICanHandleException) self.staticdir = staticdir # so tests can check if staticdir: From 743ead71a435b007514be87d090724a7c837f5e4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 08:50:37 -0400 Subject: [PATCH 39/83] De-Nevow ``get_arg`` --- src/allmydata/web/common_py3.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index 73130cbab..b4c8c1942 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -6,13 +6,7 @@ Can eventually be merged back into allmydata.web.common. from future.utils import PY2 -if PY2: - from nevow.inevow import IRequest as INevowRequest -else: - INevowRequest = None - from twisted.web import resource, http -from twisted.web.iweb import IRequest from allmydata.util import abbreviate @@ -23,7 +17,7 @@ class WebError(Exception): self.code = code -def get_arg(ctx_or_req, argname, default=None, multiple=False): +def get_arg(req, argname, default=None, multiple=False): """Extract an argument from either the query args (req.args) or the form body fields (req.fields). If multiple=False, this returns a single value (or the default, which defaults to None), and the query args take @@ -31,16 +25,10 @@ def get_arg(ctx_or_req, argname, default=None, multiple=False): empty), starting with all those in the query args. """ results = [] - if PY2: - req = INevowRequest(ctx_or_req) - if argname in req.args: - results.extend(req.args[argname]) - if req.fields and argname in req.fields: - results.append(req.fields[argname].value) - else: - req = IRequest(ctx_or_req) - if argname in req.args: - results.extend(req.args[argname]) + if argname in req.args: + results.extend(req.args[argname]) + if req.fields and argname in req.fields: + results.append(req.fields[argname].value) if multiple: return tuple(results) if results: From 304a9880e6cf4a33c68c3d43da84d06b2368f269 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 08:50:53 -0400 Subject: [PATCH 40/83] De-Nevow ``get_root`` --- src/allmydata/web/common.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index c9d59e7ae..54c1aad97 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -47,7 +47,6 @@ from twisted.internet.defer import ( from twisted.web.resource import ( IResource, ) -from twisted.web.iweb import IRequest as ITwistedRequest if PY2: from nevow.appserver import DefaultExceptionHandler from nevow.inevow import IRequest as INevowRequest @@ -161,11 +160,7 @@ 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) +def get_root(req): depth = len(req.prepath) + len(req.postpath) link = "/".join([".."] * depth) return link From 637bb2e576f7e7f299d93bfb312b5577816dde86 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 08:51:25 -0400 Subject: [PATCH 41/83] De-Nevow ``render`` --- src/allmydata/test/common_nevow.py | 50 ------------------------------ src/allmydata/test/common_web.py | 19 +++--------- 2 files changed, 4 insertions(+), 65 deletions(-) delete mode 100644 src/allmydata/test/common_nevow.py diff --git a/src/allmydata/test/common_nevow.py b/src/allmydata/test/common_nevow.py deleted file mode 100644 index d6327f9c6..000000000 --- a/src/allmydata/test/common_nevow.py +++ /dev/null @@ -1,50 +0,0 @@ -""" -General helpers related to Nevow. -""" - -from twisted.internet.defer import ( - maybeDeferred, -) - -from nevow.context import WebContext -from nevow.testutil import FakeRequest -from nevow.appserver import ( - processingFailed, - DefaultExceptionHandler, -) -from nevow.inevow import ( - ICanHandleException, - IRequest, - IResource as INevowResource, - IData, -) - -def render(resource, query_args): - """ - Render (in the manner of the Nevow appserver) a Nevow ``Page`` or a - Twisted ``Resource`` against a request with the given query arguments . - - :param resource: The page or resource to render. - - :param query_args: The query arguments to put into the request being - rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. - - :return Deferred: A Deferred that fires with the rendered response body as - ``bytes``. - """ - ctx = WebContext(tag=resource) - req = FakeRequest(args=query_args) - ctx.remember(DefaultExceptionHandler(), ICanHandleException) - ctx.remember(req, IRequest) - ctx.remember(None, IData) - - def maybe_concat(res): - if isinstance(res, bytes): - return req.v + res - return req.v - - resource = INevowResource(resource) - d = maybeDeferred(resource.renderHTTP, ctx) - d.addErrback(processingFailed, req, ctx) - d.addCallback(maybe_concat) - return d diff --git a/src/allmydata/test/common_web.py b/src/allmydata/test/common_web.py index 08b356cd0..33c4a41c1 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -13,6 +13,10 @@ from twisted.internet.defer import ( ) from twisted.web.error import Error +from .common_tweb import ( + render, +) + @inlineCallbacks def do_http(method, url, **kwargs): response = yield treq.request(method, url, persistent=False, **kwargs) @@ -22,18 +26,3 @@ def do_http(method, url, **kwargs): if 400 <= response.code < 600: raise Error(response.code, response=body) returnValue(body) - - -if PY2: - # We can only use Nevow on Python 2 and Tahoe-LAFS still *does* use Nevow - # so prefer the Nevow-based renderer if we can get it. - from .common_nevow import ( - render, - ) -else: - # However, Tahoe-LAFS *will* use Twisted Web before too much longer so go - # ahead and let some tests run against the Twisted Web-based renderer on - # Python 3. Later this will become the only codepath. - from .common_tweb import ( - render, - ) From 538cefed3d8c0e7cb8c19b2e52387af2e8b01304 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 08:51:45 -0400 Subject: [PATCH 42/83] Update Twisted Web-based `render` to use our request Also to do one other thing Twisted Web does for resources automatically - handle UnsupportedMethod --- src/allmydata/test/common_tweb.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py index 37e24b5b8..5a0114cab 100644 --- a/src/allmydata/test/common_tweb.py +++ b/src/allmydata/test/common_tweb.py @@ -7,13 +7,21 @@ from twisted.internet.defer import ( from twisted.web.test.requesthelper import ( DummyChannel, ) -from twisted.web.server import ( - Request, +from twisted.web.error import ( + UnsupportedMethod, +) +from twisted.web.http import ( + NOT_ALLOWED, ) from twisted.web.server import ( NOT_DONE_YET, ) +from ..webish import ( + TahoeLAFSRequest, +) + + def render(resource, query_args): """ Render (in the manner of the Twisted Web Site) a Twisted ``Resource`` @@ -28,9 +36,15 @@ def render(resource, query_args): ``bytes``. """ channel = DummyChannel() - request = Request(channel) + request = TahoeLAFSRequest(channel) + request.method = b"GET" request.args = query_args - result = resource.render(request) + try: + result = resource.render(request) + except UnsupportedMethod: + request.setResponseCode(NOT_ALLOWED) + result = b"" + if isinstance(result, bytes): request.write(result) done = succeed(None) From c64a71a64271a8b9db348c8c8018071fb2e12d74 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:45:11 -0400 Subject: [PATCH 43/83] Re-arrange some of the requestReceived logic and switch to t.web Site --- src/allmydata/webish.py | 86 +++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 54 deletions(-) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index b8b9de83a..bd00b4094 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -5,44 +5,43 @@ from cgi import ( ) from twisted.application import service, strports, internet -from twisted.web import http, server, static +from twisted.web import static +from twisted.web.http import ( + parse_qs, +) +from twisted.web.server import ( + Request, + Site, +) from twisted.internet import defer from twisted.internet.address import ( IPv4Address, IPv6Address, ) -from nevow import appserver, inevow from allmydata.util import log, fileutil from allmydata.web import introweb, root -from allmydata.web.common import MyExceptionHandler from allmydata.web.operations import OphandleTable from .web.storage_plugins import ( StoragePlugins, ) -# we must override twisted.web.http.Request.requestReceived with a version -# that doesn't use cgi.parse_multipart() . Since we actually use Nevow, we -# override the nevow-specific subclass, nevow.appserver.NevowRequest . This -# is an exact copy of twisted.web.http.Request (from SVN HEAD on 10-Aug-2007) -# that modifies the way form arguments are parsed. Note that this sort of -# surgery may induce a dependency upon a particular version of twisted.web - -parse_qs = http.parse_qs -class TahoeLAFSRequest(server.Request, object): +class TahoeLAFSRequest(Request, object): fields = None _tahoe_request_had_error = None def requestReceived(self, command, path, version): - """Called by channel when all data has been received. + """ + Called by channel when all data has been received. - This method is not intended for users. + Override the base implementation to apply certain site-wide policies + and to provide less memory-intensive multipart/form-post handling for + large file uploads. """ self.content.seek(0) self.args = {} self.stack = [] - self.setHeader("Referrer-Policy", "no-referrer") self.method, self.uri = command, path self.clientproto = version @@ -54,11 +53,10 @@ class TahoeLAFSRequest(server.Request, object): self.path, argstring = x self.args = parse_qs(argstring, 1) - # Adding security headers. These will be sent for *all* HTTP requests. - # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options - self.responseHeaders.setRawHeaders("X-Frame-Options", ["DENY"]) - if self.method == 'POST': + # We use FieldStorage here because it performs better than + # cgi.parse_multipart(self.content, pdict) which is what + # twisted.web.http.Request uses. self.fields = FieldStorage( self.content, { @@ -69,42 +67,23 @@ class TahoeLAFSRequest(server.Request, object): environ={'REQUEST_METHOD': 'POST'}) self.content.seek(0) - # Argument processing. + self._tahoeLAFSSecurityPolicy() -## The original twisted.web.http.Request.requestReceived code parsed the -## content and added the form fields it found there to self.args . It -## did this with cgi.parse_multipart, which holds the arguments in RAM -## and is thus unsuitable for large file uploads. The Nevow subclass -## (nevow.appserver.NevowRequest) uses cgi.FieldStorage instead (putting -## the results in self.fields), which is much more memory-efficient. -## Since we know we're using Nevow, we can anticipate these arguments -## appearing in self.fields instead of self.args, and thus skip the -## parse-content-into-self.args step. - -## args = self.args -## ctype = self.getHeader('content-type') -## if self.method == "POST" and ctype: -## mfd = 'multipart/form-data' -## key, pdict = cgi.parse_header(ctype) -## if key == 'application/x-www-form-urlencoded': -## args.update(parse_qs(self.content.read(), 1)) -## elif key == mfd: -## try: -## args.update(cgi.parse_multipart(self.content, pdict)) -## except KeyError, e: -## if e.args[0] == 'content-disposition': -## # Parse_multipart can't cope with missing -## # content-dispostion headers in multipart/form-data -## # parts, so we catch the exception and tell the client -## # it was a bad request. -## self.channel.transport.write( -## "HTTP/1.1 400 Bad Request\r\n\r\n") -## self.channel.transport.loseConnection() -## return -## raise self.processing_started_timestamp = time.time() self.process() + def _tahoeLAFSSecurityPolicy(self): + """ + Set response properties related to Tahoe-LAFS-imposed security policy. + This will ensure that all HTTP requests received by the Tahoe-LAFS + HTTP server have this policy imposed, regardless of other + implementation details. + """ + # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options + self.responseHeaders.setRawHeaders("X-Frame-Options", ["DENY"]) + # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy + self.setHeader("Referrer-Policy", "no-referrer") + def _logger(self): # we build up a log string that hides most of the cap, to preserve # user privacy. We retain the query args so we can identify things @@ -191,15 +170,14 @@ class WebishServer(service.MultiService): def buildServer(self, webport, nodeurl_path, staticdir): self.webport = webport - self.site = site = appserver.NevowSite(self.root) + self.site = Site(self.root) self.site.requestFactory = TahoeLAFSRequest - self.site.remember(MyExceptionHandler(), inevow.ICanHandleException) self.staticdir = staticdir # so tests can check if staticdir: self.root.putChild("static", static.File(staticdir)) if re.search(r'^\d', webport): webport = "tcp:"+webport # twisted warns about bare "0" or "3456" - s = strports.service(webport, site) + s = strports.service(webport, self.site) s.setServiceParent(self) self._scheme = None From c8db069960f623ec7a7d0646a7ff1abe542f6fb7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:45:41 -0400 Subject: [PATCH 44/83] Get rid of no-longer used Nevow exception handler --- src/allmydata/web/common.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 54c1aad97..382aaf330 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -47,14 +47,6 @@ from twisted.internet.defer import ( from twisted.web.resource import ( IResource, ) -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 ( @@ -361,13 +353,6 @@ def humanize_failure(f): return humanize_exception(f.value) -class MyExceptionHandler(DefaultExceptionHandler, object): - def renderHTTP_exception(self, ctx, f): - req = INevowRequest(ctx) - req.write(_renderHTTP_exception(req, f)) - req.finishRequest(False) - - class NeedOperationHandleError(WebError): pass From 27c2fd80c812129d57c21cfe1f8e2ef852bab9e7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:46:00 -0400 Subject: [PATCH 45/83] re-use our other renderer --- src/allmydata/test/test_storage_web.py | 30 +++++++------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/allmydata/test/test_storage_web.py b/src/allmydata/test/test_storage_web.py index aa1f19936..72028d77f 100644 --- a/src/allmydata/test/test_storage_web.py +++ b/src/allmydata/test/test_storage_web.py @@ -26,14 +26,6 @@ from twisted.internet import defer from twisted.application import service from twisted.web.template import flattenString -# 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 twisted.web.server import Request from twisted.web.test.requesthelper import DummyChannel from zope.interface import implementer @@ -52,6 +44,10 @@ from allmydata.web.storage import ( ) from .common_util import FakeCanary +from .common_web import ( + render, +) + def remove_tags(s): s = re.sub(br'<[^>]*>', b' ', s) s = re.sub(br'\s+', b' ', s) @@ -75,20 +71,10 @@ def renderDeferred(ss): return flattenString(None, elem) def renderJSON(resource): - """Render a JSON from the given resource.""" - - @implementer(IRequest) - class JSONRequest(Request): - """ - A Request with t=json argument added to it. This is useful to - invoke a Resouce.render_JSON() method. - """ - def __init__(self): - Request.__init__(self, DummyChannel()) - self.args = {"t": ["json"]} - self.fields = {} - - return resource.render(JSONRequest()) + """ + Render a JSON from the given resource. + """ + return render(resource, {"t": ["json"]}) class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): From 7d54af79285cca2cbfbacdd10ea3762ec6843f51 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:46:14 -0400 Subject: [PATCH 46/83] re-use our other renderer --- src/allmydata/test/test_checker.py | 49 +++++++++++++----------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 3813ac199..3e16830e4 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -20,18 +20,16 @@ from bs4 import BeautifulSoup from twisted.trial import unittest from twisted.internet import defer -# 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 from twisted.web.template import flattenString +from twisted.web.resource import ( + Resource, +) +from twisted.web.template import ( + renderElement, +) from allmydata import check_results, uri from allmydata import uri as tahoe_uri @@ -41,6 +39,9 @@ from allmydata.interfaces import ( ICheckAndRepairResults, ) from allmydata.util import base32 +from allmydata.webish import ( + TahoeLAFSRequest, +) from allmydata.web import check_results as web_check_results from allmydata.storage_client import StorageFarmBroker, NativeStorageServer from allmydata.storage.server import storage_index_to_dir @@ -65,24 +66,6 @@ class FakeClient(object): def get_storage_broker(self): return self.storage_broker -@implementer(IRequest) -class TestRequest(Request, object): - """ - A minimal Request class to use in tests. - - XXX: We have to have this class because `common.get_arg()` expects - a `nevow.inevow.IRequest`, which `twisted.web.server.Request` - isn't. The request needs to have `args`, `fields`, `prepath`, and - `postpath` properties so that `allmydata.web.common.get_arg()` - won't complain. - """ - def __init__(self, args=None, fields=None): - super(TestRequest, self).__init__(DummyChannel()) - self.args = args or {} - self.fields = fields or {} - self.prepath = [b""] - self.postpath = [b""] - @implementer(IServer) class FakeServer(object): @@ -154,6 +137,15 @@ class FakeCheckAndRepairResults(object): return self._repair_success +class ElementResource(Resource, object): + def __init__(self, element): + Resource.__init__(self) + self.element = element + + def render(self, request): + return renderElement(request, self.element) + + class WebResultsRendering(unittest.TestCase): @staticmethod @@ -190,8 +182,9 @@ class WebResultsRendering(unittest.TestCase): return self.successResultOf(render(resource, {"output": ["json"]})) def render_element(self, element, args=None): - d = flattenString(TestRequest(args), element) - return self.successResultOf(d) + if args is None: + args = {} + return self.successResultOf(render(ElementResource(element), args)) def test_literal(self): lcr = web_check_results.LiteralCheckResultsRendererElement() From 80549f5f02dc53d040a3c4c9b0ffcfaa713a7f06 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:46:21 -0400 Subject: [PATCH 47/83] Make this request a little more realistic --- src/allmydata/test/common_tweb.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py index 5a0114cab..77c00dd02 100644 --- a/src/allmydata/test/common_tweb.py +++ b/src/allmydata/test/common_tweb.py @@ -39,6 +39,8 @@ def render(resource, query_args): request = TahoeLAFSRequest(channel) request.method = b"GET" request.args = query_args + request.prepath = [b""] + request.postpath = [] try: result = resource.render(request) except UnsupportedMethod: From a577f1e48dda2cd0a0f16146185d970643e36e3a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 09:49:03 -0400 Subject: [PATCH 48/83] pyflakes --- src/allmydata/test/common_web.py | 2 -- src/allmydata/test/test_checker.py | 6 ------ src/allmydata/test/test_storage_web.py | 4 ---- src/allmydata/test/web/test_webish.py | 16 ---------------- src/allmydata/web/common.py | 1 - src/allmydata/web/common_py3.py | 2 -- 6 files changed, 31 deletions(-) diff --git a/src/allmydata/test/common_web.py b/src/allmydata/test/common_web.py index 33c4a41c1..0c3826f7f 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -4,8 +4,6 @@ __all__ = [ "render", ] -from future.utils import PY2 - import treq from twisted.internet.defer import ( inlineCallbacks, diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 3e16830e4..85b894b1f 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -21,9 +21,6 @@ from twisted.trial import unittest from twisted.internet import defer from zope.interface import implementer -from twisted.web.server import Request -from twisted.web.test.requesthelper import DummyChannel -from twisted.web.template import flattenString from twisted.web.resource import ( Resource, ) @@ -39,9 +36,6 @@ from allmydata.interfaces import ( ICheckAndRepairResults, ) from allmydata.util import base32 -from allmydata.webish import ( - TahoeLAFSRequest, -) from allmydata.web import check_results as web_check_results from allmydata.storage_client import StorageFarmBroker, NativeStorageServer from allmydata.storage.server import storage_index_to_dir diff --git a/src/allmydata/test/test_storage_web.py b/src/allmydata/test/test_storage_web.py index 72028d77f..706618bec 100644 --- a/src/allmydata/test/test_storage_web.py +++ b/src/allmydata/test/test_storage_web.py @@ -26,10 +26,6 @@ from twisted.internet import defer from twisted.application import service from twisted.web.template import flattenString -from twisted.web.server import Request -from twisted.web.test.requesthelper import DummyChannel -from zope.interface import implementer - from foolscap.api import fireEventually from allmydata.util import fileutil, hashutil, base32, pollmixin from allmydata.storage.common import storage_index_to_dir, \ diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 52ba049a0..33d1b48c3 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -2,9 +2,6 @@ Tests for ``allmydata.webish``. """ -from io import ( - BytesIO, -) from uuid import ( uuid4, ) @@ -14,22 +11,9 @@ from testtools.matchers import ( Equals, ) -from twisted.python.filepath import ( - FilePath, -) from twisted.web.test.requesthelper import ( DummyChannel, ) -from twisted.web.client import ( - FileBodyProducer, -) -from twisted.internet.task import ( - Cooperator, -) - -from treq.multipart import ( - MultiPartProducer, -) from ..common import ( SyncTestCase, diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 382aaf330..66c827552 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -1,4 +1,3 @@ -from future.utils import PY2 from past.builtins import unicode import time diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index b4c8c1942..a906a6e61 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -4,8 +4,6 @@ Common utilities that are available from Python 3. Can eventually be merged back into allmydata.web.common. """ -from future.utils import PY2 - from twisted.web import resource, http from allmydata.util import abbreviate From 2ba34a475963a272c00b81be28d6c6bbae053d43 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:01:46 -0400 Subject: [PATCH 49/83] Talk about Nevow as a proper noun where necessary --- src/allmydata/web/common.py | 2 +- src/allmydata/web/directory.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 66c827552..085e8809b 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -358,7 +358,7 @@ class NeedOperationHandleError(WebError): class SlotsSequenceElement(template.Element): """ - ``SlotsSequenceElement` is a minimal port of nevow's sequence renderer for + ``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`` diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 0d521951f..e67281980 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -114,7 +114,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # Rejecting URIs that contain empty path pieces (for example: # "/uri/URI:DIR2:../foo//new.txt" or "/uri/URI:DIR2:..//") was - # the old nevow behavior and it is encoded in the test suite; + # the old Nevow behavior and it is encoded in the test suite; # we will follow suit. for segment in req.prepath: if not segment: From 816ca79d8a1df34d500020cd19f3dd19d5ee3a3e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:01:57 -0400 Subject: [PATCH 50/83] Talk about os.stat and tracebacks instead of Nevow --- src/allmydata/test/web/test_web.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 508fc82d4..aa6d44ea4 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -4767,11 +4767,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_static_missing(self): # self.staticdir does not exist yet, because we used self.mktemp() d = self.assertFailure(self.GET("/static"), error.Error) - # nevow.static throws an exception when it tries to os.stat the - # missing directory, which gives the client a 500 Internal Server - # Error, and the traceback reveals the parent directory name. By - # switching to plain twisted.web.static, this gives a normal 404 that - # doesn't reveal anything. This addresses #1720. + # If os.stat raises an exception for the missing directory and the + # traceback reveals the parent directory name we don't want to see + # that parent directory name in the response. This addresses #1720. d.addCallback(lambda e: self.assertEquals(str(e), "404 Not Found")) return d From 6500f742dcc6b4a32f49518b744a452da5b6f145 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:02:10 -0400 Subject: [PATCH 51/83] Twisted Web handles UnsupportedMethod for us --- src/allmydata/web/common.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 085e8809b..1bb89f75d 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -569,17 +569,6 @@ def _renderHTTP_exception(request, failure): if code is not None: return _renderHTTP_exception_simple(request, text, code) - if failure.check(UnsupportedMethod): - # 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. - method = request.method - return _renderHTTP_exception_simple( - request, - "I don't know how to treat a %s request." % (method,), - http.NOT_IMPLEMENTED, - ) - accept = request.getHeader("accept") if not accept: accept = "*/*" From 7eecf51dc51c01bf673a888f2e9defaf08f82c3f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:02:22 -0400 Subject: [PATCH 52/83] Just talk about rendering, not Nevow --- src/allmydata/web/directory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index e67281980..558bfb634 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -398,8 +398,8 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): d.addBoth(_maybe_got_node) # now we have a placeholder or a filenodehandler, and we can just # delegate to it. We could return the resource back out of - # DirectoryNodeHandler.renderHTTP, and nevow would recurse into it, - # but the addCallback() that handles when_done= would break. + # DirectoryNodeHandler.render_POST and it would get rendered but the + # addCallback() that handles when_done= would break. def render_child(child): req.dont_apply_extra_processing = True return child.render(req) From 03a144755d5b18b1c27f02011199ff70987b29e2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:02:32 -0400 Subject: [PATCH 53/83] Nevow is no longer a dependency --- src/allmydata/_auto_deps.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/_auto_deps.py b/src/allmydata/_auto_deps.py index cf98aae96..17f39cfcc 100644 --- a/src/allmydata/_auto_deps.py +++ b/src/allmydata/_auto_deps.py @@ -11,7 +11,6 @@ package_imports = [ ('foolscap', 'foolscap'), ('zfec', 'zfec'), ('Twisted', 'twisted'), - ('Nevow', 'nevow'), ('zope.interface', 'zope.interface'), ('python', None), ('platform', None), @@ -72,7 +71,6 @@ runtime_warning_messages = [ ] warning_imports = [ - 'nevow', 'twisted.persisted.sob', 'twisted.python.filepath', ] From 4305777f8863678ddbbae8d5052a69808d84e6a8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:03:06 -0400 Subject: [PATCH 54/83] Nevow is no longer a dependency --- setup.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/setup.py b/setup.py index 4151545f7..874cc1258 100644 --- a/setup.py +++ b/setup.py @@ -38,8 +38,7 @@ install_requires = [ "zfec >= 1.1.0", # zope.interface >= 3.6.0 is required for Twisted >= 12.1.0. - # zope.interface 3.6.3 and 3.6.4 are incompatible with Nevow (#1435). - "zope.interface >= 3.6.0, != 3.6.3, != 3.6.4", + "zope.interface >= 3.6.0", # * foolscap < 0.5.1 had a performance bug which spent O(N**2) CPU for # transferring large mutable files of size N. @@ -70,7 +69,6 @@ install_requires = [ # rekeying bug # * The FTP frontend depends on Twisted >= 11.1.0 for # filepath.Permissions - # * Nevow 0.11.1 depends on Twisted >= 13.0.0. # * The SFTP frontend and manhole depend on the conch extra. However, we # can't explicitly declare that without an undesirable dependency on gmpy, # as explained in ticket #2740. @@ -102,9 +100,6 @@ install_requires = [ # an sftp extra in Tahoe-LAFS, there is no point in having one. "Twisted[tls,conch] >= 18.4.0", - # We need Nevow >= 0.11.1 which can be installed using pip. - "Nevow >= 0.11.1", - "PyYAML >= 3.11", "six >= 1.10.0", From e028c696423b12891b3ab0a870024717b380fd0f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:03:38 -0400 Subject: [PATCH 55/83] News fragment --- newsfragments/3433.installation | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3433.installation diff --git a/newsfragments/3433.installation b/newsfragments/3433.installation new file mode 100644 index 000000000..3c06e53d3 --- /dev/null +++ b/newsfragments/3433.installation @@ -0,0 +1 @@ +Tahoe-LAFS no longer depends on Nevow. \ No newline at end of file From 14b9dc090d172f117d6d7292eb8e74954433a3c3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:12:48 -0400 Subject: [PATCH 56/83] Replace some `ctx` names with `req` --- src/allmydata/web/directory.py | 6 +++--- src/allmydata/web/operations.py | 2 -- src/allmydata/web/root.py | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 558bfb634..f83defd6a 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -522,9 +522,9 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): d.addCallback(self._maybe_literal, CheckResultsRenderer) return d - def _start_operation(self, monitor, renderer, ctx): - self._operations.add_monitor(ctx, monitor, renderer) - return self._operations.redirect_to(ctx) + def _start_operation(self, monitor, renderer, req): + self._operations.add_monitor(req, monitor, renderer) + return self._operations.redirect_to(req) def _POST_start_deep_check(self, req): # check this directory and everything reachable from it diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 0e53d075d..2ba87c5ec 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -152,8 +152,6 @@ class ReloadMixin(object): 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 diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 78daadef4..91f14bd91 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -189,7 +189,7 @@ class FileHandler(resource.Resource, object): return filenode.FileNodeDownloadHandler(self.client, node) @render_exception - def render_GET(self, ctx): + def render_GET(self, req): raise WebError("/file must be followed by a file-cap and a name", http.NOT_FOUND) From 9b8b7a5d86649be428577db18c2759a30e5dafbc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:16:44 -0400 Subject: [PATCH 57/83] Remove Nevow from the static script --- newsfragments/3434.minor | 0 static/tahoe.py | 20 ++++++++------------ 2 files changed, 8 insertions(+), 12 deletions(-) create mode 100644 newsfragments/3434.minor diff --git a/newsfragments/3434.minor b/newsfragments/3434.minor new file mode 100644 index 000000000..e69de29bb diff --git a/static/tahoe.py b/static/tahoe.py index cac53bdfa..c18f60e2c 100644 --- a/static/tahoe.py +++ b/static/tahoe.py @@ -3,23 +3,19 @@ # Import this first to suppress deprecation warnings. import allmydata -# nevow requires all these for its voodoo module import time adaptor registrations -from nevow import accessors, appserver, static, rend, url, util, query, i18n, flat -from nevow import guard, stan, testutil, context -from nevow.flat import flatmdom, flatstan, twist -from formless import webform, processors, annotate, iformless from decimal import Decimal from xml.dom import minidom import allmydata.web -# junk to appease pyflakes's outrage -[ - accessors, appserver, static, rend, url, util, query, i18n, flat, guard, stan, testutil, - context, flatmdom, flatstan, twist, webform, processors, annotate, iformless, Decimal, - minidom, allmydata, -] +# We import these things to give PyInstaller's dependency resolver some hints +# about what it needs to include. We don't use them otherwise _here_ but +# other parts of the codebase do. pyflakes points out that they are unused +# unless we use them. So ... use them. +Decimal +minidom +allmydata from allmydata.scripts import runner -runner.run() \ No newline at end of file +runner.run() From 5f10f1e38284efbd0b7c7c69e2de2a8e93a23900 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:17:21 -0400 Subject: [PATCH 58/83] Remove Nevow from the tool version helper --- misc/build_helpers/show-tool-versions.py | 1 - newsfragments/3435.minor | 0 2 files changed, 1 deletion(-) create mode 100644 newsfragments/3435.minor diff --git a/misc/build_helpers/show-tool-versions.py b/misc/build_helpers/show-tool-versions.py index c4fb79eff..f70183ae1 100644 --- a/misc/build_helpers/show-tool-versions.py +++ b/misc/build_helpers/show-tool-versions.py @@ -143,7 +143,6 @@ print_py_pkg_ver('coverage') print_py_pkg_ver('cryptography') print_py_pkg_ver('foolscap') print_py_pkg_ver('mock') -print_py_pkg_ver('Nevow', 'nevow') print_py_pkg_ver('pyasn1') print_py_pkg_ver('pycparser') print_py_pkg_ver('cryptography') diff --git a/newsfragments/3435.minor b/newsfragments/3435.minor new file mode 100644 index 000000000..e69de29bb From e2013e9ebb8afed5904b36191233ff926190069c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:17:43 -0400 Subject: [PATCH 59/83] news fragment --- newsfragments/3432.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3432.minor diff --git a/newsfragments/3432.minor b/newsfragments/3432.minor new file mode 100644 index 000000000..e69de29bb From 9146d2a0b97e61efbfbe551678acc905328f71d7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 10:25:23 -0400 Subject: [PATCH 60/83] Remove Nevow dependency from the Nix packaging --- nix/nevow.nix | 45 --------------------------------------------- nix/overlays.nix | 5 +---- nix/tahoe-lafs.nix | 4 ++-- 3 files changed, 3 insertions(+), 51 deletions(-) delete mode 100644 nix/nevow.nix diff --git a/nix/nevow.nix b/nix/nevow.nix deleted file mode 100644 index 202a59722..000000000 --- a/nix/nevow.nix +++ /dev/null @@ -1,45 +0,0 @@ -{ stdenv, buildPythonPackage, fetchPypi, isPy3k, twisted }: - -buildPythonPackage rec { - pname = "Nevow"; - version = "0.14.5"; - name = "${pname}-${version}"; - disabled = isPy3k; - - src = fetchPypi { - inherit pname; - inherit version; - sha256 = "1wr3fai01h1bcp4qpia6indg4qmxvywwv3q1iibm669mln2vmdmg"; - }; - - propagatedBuildInputs = [ twisted ]; - - checkInputs = [ twisted ]; - - checkPhase = '' - trial formless nevow - ''; - - meta = with stdenv.lib; { - description = "Nevow, a web application construction kit for Python"; - longDescription = '' - Nevow - Pronounced as the French "nouveau", or "noo-voh", Nevow - is a web application construction kit written in Python. It is - designed to allow the programmer to express as much of the view - logic as desired in Python, and includes a pure Python XML - expression syntax named stan to facilitate this. However it - also provides rich support for designer-edited templates, using - a very small XML attribute language to provide bi-directional - template manipulation capability. - - Nevow also includes formless, a declarative syntax for - specifying the types of method parameters and exposing these - methods to the web. Forms can be rendered automatically, and - form posts will be validated and input coerced, rendering error - pages if appropriate. Once a form post has validated - successfully, the method will be called with the coerced values. - ''; - homepage = https://github.com/twisted/nevow; - license = licenses.mit; - }; -} diff --git a/nix/overlays.nix b/nix/overlays.nix index ba3c9c885..4ee63a412 100644 --- a/nix/overlays.nix +++ b/nix/overlays.nix @@ -3,10 +3,7 @@ self: super: { packageOverrides = python-self: python-super: { # eliot is not part of nixpkgs at all at this time. eliot = python-self.callPackage ./eliot.nix { }; - # The packaged version of Nevow is very slightly out of date but also - # conflicts with the packaged version of Twisted. Supply our own - # slightly newer version. - nevow = python-super.callPackage ./nevow.nix { }; + # NixOS autobahn package has trollius as a dependency, although # it is optional. Trollius is unmaintained and fails on CI. autobahn = python-super.callPackage ./autobahn.nix { }; diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index f2e61d6c2..a7f8fcbf7 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -1,6 +1,6 @@ { fetchFromGitHub, lib , nettools, python -, twisted, foolscap, nevow, zfec +, twisted, foolscap, zfec , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs , beautifulsoup4, eliot, autobahn, cryptography @@ -46,7 +46,7 @@ python.pkgs.buildPythonPackage rec { ]; propagatedBuildInputs = with python.pkgs; [ - twisted foolscap nevow zfec appdirs + twisted foolscap zfec appdirs setuptoolsTrial pyasn1 zope_interface service-identity pyyaml magic-wormhole treq eliot autobahn cryptography setuptools From c31300fd0d81115a0103acd3d95c0a02052f0615 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 12:21:01 -0400 Subject: [PATCH 61/83] Handle interrupted connections --- src/allmydata/test/web/test_common.py | 33 +++++++++++++++++++++++++++ src/allmydata/web/common.py | 15 +++++++++++- src/allmydata/web/filenode.py | 5 +++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py index 6431ba610..dd859befb 100644 --- a/src/allmydata/test/web/test_common.py +++ b/src/allmydata/test/web/test_common.py @@ -13,13 +13,22 @@ from testtools.matchers import ( Equals, Contains, MatchesPredicate, + AfterPreprocessing, ) from testtools.twistedsupport import ( + failed, succeeded, has_no_result, ) +from twisted.python.failure import ( + Failure, +) +from twisted.internet.error import ( + ConnectionDone, +) from twisted.internet.defer import ( + Deferred, fail, ) from twisted.web.server import ( @@ -52,9 +61,11 @@ class StaticResource(Resource, object): def __init__(self, response): Resource.__init__(self) self._response = response + self._request = None @render_exception def render(self, request): + self._request = request return self._response @@ -214,3 +225,25 @@ class RenderExceptionTests(SyncTestCase): Equals(b"Internal Server Error"), ), ) + + def test_disconnected(self): + """ + If the transport is disconnected before the response is available, nothing + is written to the request. + """ + result = Deferred() + resource = StaticResource(result) + d = render(resource, {}) + + resource._request.connectionLost(Failure(ConnectionDone())) + result.callback(b"Some result") + + self.assertThat( + d, + failed( + AfterPreprocessing( + lambda reason: reason.type, + Equals(ConnectionDone), + ), + ), + ) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 1bb89f75d..49595756f 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -41,6 +41,7 @@ from twisted.python.failure import ( Failure, ) from twisted.internet.defer import ( + CancelledError, maybeDeferred, ) from twisted.web.resource import ( @@ -472,7 +473,17 @@ def render_exception(render): # Apply `_finish` all of our result handling logic to whatever it # returned. result.addBoth(_finish, bound_render, request) - result.addActionFinish() + d = result.addActionFinish() + + # If the connection is lost then there's no point running our _finish + # logic because it has nowhere to send anything. There may also be no + # point in finishing whatever operation was being performed because + # the client cannot be informed of its result. Also, Twisted Web + # raises exceptions from some Request methods if they're used after + # the connection is lost. + request.notifyFinish().addErrback( + lambda ignored: d.cancel(), + ) return NOT_DONE_YET return g @@ -497,6 +508,8 @@ def _finish(result, render, request): :return: ``None`` """ if isinstance(result, Failure): + if result.check(CancelledError): + return Message.log( message_type=u"allmydata:web:common-render:failure", message=result.getErrorMessage(), diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 7686b35b5..f65977460 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -480,7 +480,10 @@ class FileDownloader(Resource, object): d = self.filenode.read(req, first, size) def _error(f): - req._tahoe_request_had_error = f # for HTTP-style logging + if f.check(defer.CancelledError): + # The HTTP connection was lost and we no longer have anywhere + # to send our result. Let this pass through. + return f if req.startedWriting: # The content-type is already set, and the response code has # already been sent, so we can't provide a clean error From 37016f4ab520245ab120eb7b07215c12d3d6585c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 12:21:44 -0400 Subject: [PATCH 62/83] pyflakes --- src/allmydata/web/common.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 49595756f..fef36f029 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -26,7 +26,6 @@ from twisted.web.template import ( ) from twisted.web.server import ( NOT_DONE_YET, - UnsupportedMethod, ) from twisted.web.util import ( DeferredResource, From 444c3e6ce42d70f7fb56a364981ea5b78a95cad0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 12:23:43 -0400 Subject: [PATCH 63/83] typo fix --- src/allmydata/test/test_storage_web.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_storage_web.py b/src/allmydata/test/test_storage_web.py index 706618bec..ca0cd85fc 100644 --- a/src/allmydata/test/test_storage_web.py +++ b/src/allmydata/test/test_storage_web.py @@ -11,7 +11,7 @@ from __future__ import unicode_literals from future.utils import PY2 if PY2: - # Omitted list sinc it broke a test on Python 2. Shouldn't require further + # Omitted list since it broke a test on Python 2. Shouldn't require further # work, when we switch to Python 3 we'll be dropping this, anyway. from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, object, range, str, max, min # noqa: F401 From 62f5fb9d28f14c8fbebf89b6f4c8e71b9d6eae87 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 12:27:08 -0400 Subject: [PATCH 64/83] Make sure the JSON is given back as bytes --- src/allmydata/web/storage.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index cf3264dac..51624a409 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,3 +1,4 @@ +from future.utils import PY2 import time, json from twisted.python.filepath import FilePath @@ -317,4 +318,7 @@ class StorageStatus(MultiFormatResource): "lease-checker": self._storage.lease_checker.get_state(), "lease-checker-progress": self._storage.lease_checker.get_progress(), } - return json.dumps(d, indent=1) + "\n" + result = json.dumps(d, indent=1) + "\n" + if PY2: + result = result.decode("utf-8") + return result.encode("utf-8") From b2999d283fb4a3d560c783626103b9d0498153df Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 12:37:34 -0400 Subject: [PATCH 65/83] Somewhat clarify this test --- src/allmydata/test/web/test_common.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py index dd859befb..2a0ebd3d8 100644 --- a/src/allmydata/test/web/test_common.py +++ b/src/allmydata/test/web/test_common.py @@ -2,6 +2,8 @@ Tests for ``allmydata.web.common``. """ +import gc + from bs4 import ( BeautifulSoup, ) @@ -228,8 +230,8 @@ class RenderExceptionTests(SyncTestCase): def test_disconnected(self): """ - If the transport is disconnected before the response is available, nothing - is written to the request. + If the transport is disconnected before the response is available, no + ``RuntimeError`` is logged for finishing a disconnected request. """ result = Deferred() resource = StaticResource(result) @@ -247,3 +249,9 @@ class RenderExceptionTests(SyncTestCase): ), ), ) + + # Since we're not a trial TestCase we don't have flushLoggedErrors. + # The next best thing is to make sure any dangling Deferreds have been + # garbage collected and then let the generic trial logic for failing + # tests with logged errors kick in. + gc.collect() From d1599a924eb2df7e360c9783da32afdb578db9f0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 14:42:30 -0400 Subject: [PATCH 66/83] Test and fix cap censoring in HTTP access logs --- src/allmydata/test/web/test_webish.py | 77 +++++++++++++++++++ src/allmydata/webish.py | 102 +++++++++++++------------- 2 files changed, 129 insertions(+), 50 deletions(-) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 33d1b48c3..45519fd33 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -8,12 +8,21 @@ from uuid import ( from testtools.matchers import ( AfterPreprocessing, + Contains, Equals, + MatchesAll, + Not, ) +from twisted.python.filepath import ( + FilePath, +) from twisted.web.test.requesthelper import ( DummyChannel, ) +from twisted.web.resource import ( + Resource, +) from ..common import ( SyncTestCase, @@ -21,6 +30,7 @@ from ..common import ( from ...webish import ( TahoeLAFSRequest, + tahoe_lafs_site, ) @@ -84,6 +94,73 @@ class TahoeLAFSRequestTests(SyncTestCase): ) +class TahoeLAFSSiteTests(SyncTestCase): + """ + Tests for the ``Site`` created by ``tahoe_lafs_site``. + """ + def _test_censoring(self, path, censored): + """ + """ + logPath = self.mktemp() + + site = tahoe_lafs_site(Resource(), logPath=logPath) + site.startFactory() + + channel = DummyChannel() + channel.factory = site + request = TahoeLAFSRequest(channel) + + request.gotLength(None) + request.requestReceived(b"GET", path, b"HTTP/1.1") + + self.assertThat( + FilePath(logPath).getContent(), + MatchesAll( + Contains(censored), + Not(Contains(path)), + ), + ) + + def test_uri_censoring(self): + """ + The log event for a request for **/uri/** has the capability value + censored. + """ + self._test_censoring( + b"/uri/URI:CHK:aaa:bbb", + b"/uri/[CENSORED]", + ) + + def test_file_censoring(self): + """ + The log event for a request for **/file/** has the capability value + censored. + """ + self._test_censoring( + b"/file/URI:CHK:aaa:bbb", + b"/file/[CENSORED]", + ) + + def test_named_censoring(self): + """ + The log event for a request for **/named/** has the capability value + censored. + """ + self._test_censoring( + b"/named/URI:CHK:aaa:bbb", + b"/named/[CENSORED]", + ) + + def test_uri_queryarg_censoring(self): + """ + The log event for a request for **/uri?cap=** has the capability + value censored. + """ + self._test_censoring( + b"/uri?uri=URI:CHK:aaa:bbb", + b"/uri?uri=[CENSORED]", + ) + def param(name, value): return u"; {}={}".format(name, value) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index bd00b4094..75f0d03dc 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,5 +1,8 @@ import re, time +from functools import ( + partial, +) from cgi import ( FieldStorage, ) @@ -84,54 +87,6 @@ class TahoeLAFSRequest(Request, object): # See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy self.setHeader("Referrer-Policy", "no-referrer") - def _logger(self): - # we build up a log string that hides most of the cap, to preserve - # user privacy. We retain the query args so we can identify things - # like t=json. Then we send it to the flog. We make no attempt to - # match apache formatting. TODO: when we move to DSA dirnodes and - # shorter caps, consider exposing a few characters of the cap, or - # maybe a few characters of its hash. - x = self.uri.split("?", 1) - if len(x) == 1: - # no query args - path = self.uri - queryargs = "" - else: - path, queryargs = x - # there is a form handler which redirects POST /uri?uri=FOO into - # GET /uri/FOO so folks can paste in non-HTTP-prefixed uris. Make - # sure we censor these too. - if queryargs.startswith("uri="): - queryargs = "[uri=CENSORED]" - queryargs = "?" + queryargs - if path.startswith("/uri"): - path = "/uri/[CENSORED].." - elif path.startswith("/file"): - path = "/file/[CENSORED].." - elif path.startswith("/named"): - path = "/named/[CENSORED].." - - uri = path + queryargs - - error = "" - if self._tahoe_request_had_error: - error = " [ERROR]" - - log.msg( - format=( - "web: %(clientip)s %(method)s %(uri)s %(code)s " - "%(length)s%(error)s" - ), - clientip=_get_client_ip(self), - method=self.method, - uri=uri, - code=self.code, - length=(self.sentLength or "-"), - error=error, - facility="tahoe.webish", - level=log.OPERATIONAL, - ) - def _get_client_ip(request): try: @@ -145,6 +100,54 @@ def _get_client_ip(request): return None +def _logFormatter(logDateTime, request): + # we build up a log string that hides most of the cap, to preserve + # user privacy. We retain the query args so we can identify things + # like t=json. Then we send it to the flog. We make no attempt to + # match apache formatting. TODO: when we move to DSA dirnodes and + # shorter caps, consider exposing a few characters of the cap, or + # maybe a few characters of its hash. + x = request.uri.split("?", 1) + if len(x) == 1: + # no query args + path = request.uri + queryargs = "" + else: + path, queryargs = x + # there is a form handler which redirects POST /uri?uri=FOO into + # GET /uri/FOO so folks can paste in non-HTTP-prefixed uris. Make + # sure we censor these too. + if queryargs.startswith("uri="): + queryargs = "uri=[CENSORED]" + queryargs = "?" + queryargs + if path.startswith("/uri/"): + path = "/uri/[CENSORED]" + elif path.startswith("/file/"): + path = "/file/[CENSORED]" + elif path.startswith("/named/"): + path = "/named/[CENSORED]" + + uri = path + queryargs + + template = "web: %(clientip)s %(method)s %(uri)s %(code)s %(length)s" + return template % dict( + clientip=_get_client_ip(request), + method=request.method, + uri=uri, + code=request.code, + length=(request.sentLength or "-"), + facility="tahoe.webish", + level=log.OPERATIONAL, + ) + + +tahoe_lafs_site = partial( + Site, + requestFactory=TahoeLAFSRequest, + logFormatter=_logFormatter, +) + + class WebishServer(service.MultiService): name = "webish" @@ -170,8 +173,7 @@ class WebishServer(service.MultiService): def buildServer(self, webport, nodeurl_path, staticdir): self.webport = webport - self.site = Site(self.root) - self.site.requestFactory = TahoeLAFSRequest + self.site = tahoe_lafs_site(self.root) self.staticdir = staticdir # so tests can check if staticdir: self.root.putChild("static", static.File(staticdir)) From 0f574dc019e8497fb9d20d0c14be642074cbfc7a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 21 Oct 2020 14:47:56 -0400 Subject: [PATCH 67/83] docstring for the helper --- src/allmydata/test/web/test_webish.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 45519fd33..1e659812f 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -100,6 +100,15 @@ class TahoeLAFSSiteTests(SyncTestCase): """ def _test_censoring(self, path, censored): """ + Verify that the event logged for a request for ``path`` does not include + ``path`` but instead includes ``censored``. + + :param bytes path: A request path. + + :param bytes censored: A replacement value for the request path in the + access log. + + :return: ``None`` if the logging looks good. """ logPath = self.mktemp() From 612cbb583ac878c915ed11e6150cc1d1d3270043 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:13:13 -0400 Subject: [PATCH 68/83] combine common_web and common_tweb now that there is no nevow renderer --- src/allmydata/test/common_tweb.py | 70 ---------------------------- src/allmydata/test/common_web.py | 77 +++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 74 deletions(-) delete mode 100644 src/allmydata/test/common_tweb.py diff --git a/src/allmydata/test/common_tweb.py b/src/allmydata/test/common_tweb.py deleted file mode 100644 index 77c00dd02..000000000 --- a/src/allmydata/test/common_tweb.py +++ /dev/null @@ -1,70 +0,0 @@ -from twisted.python.reflect import ( - fullyQualifiedName, -) -from twisted.internet.defer import ( - succeed, -) -from twisted.web.test.requesthelper import ( - DummyChannel, -) -from twisted.web.error import ( - UnsupportedMethod, -) -from twisted.web.http import ( - NOT_ALLOWED, -) -from twisted.web.server import ( - NOT_DONE_YET, -) - -from ..webish import ( - TahoeLAFSRequest, -) - - -def render(resource, query_args): - """ - Render (in the manner of the Twisted Web Site) a Twisted ``Resource`` - against a request with the given query arguments . - - :param resource: The page or resource to render. - - :param query_args: The query arguments to put into the request being - rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. - - :return Deferred: A Deferred that fires with the rendered response body as - ``bytes``. - """ - channel = DummyChannel() - request = TahoeLAFSRequest(channel) - request.method = b"GET" - request.args = query_args - request.prepath = [b""] - request.postpath = [] - try: - result = resource.render(request) - except UnsupportedMethod: - request.setResponseCode(NOT_ALLOWED) - result = b"" - - if isinstance(result, bytes): - request.write(result) - done = succeed(None) - elif result == NOT_DONE_YET: - if request.finished: - done = succeed(None) - else: - done = request.notifyFinish() - else: - raise ValueError( - "{!r} returned {!r}, required bytes or NOT_DONE_YET.".format( - fullyQualifiedName(resource.render), - result, - ), - ) - def get_body(ignored): - complete_response = channel.transport.written.getvalue() - header, body = complete_response.split(b"\r\n\r\n", 1) - return body - done.addCallback(get_body) - return done diff --git a/src/allmydata/test/common_web.py b/src/allmydata/test/common_web.py index 0c3826f7f..033b77c98 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -4,15 +4,36 @@ __all__ = [ "render", ] -import treq from twisted.internet.defer import ( inlineCallbacks, returnValue, ) -from twisted.web.error import Error +from twisted.web.error import ( + Error, +) +from twisted.python.reflect import ( + fullyQualifiedName, +) +from twisted.internet.defer import ( + succeed, +) +from twisted.web.test.requesthelper import ( + DummyChannel, +) +from twisted.web.error import ( + UnsupportedMethod, +) +from twisted.web.http import ( + NOT_ALLOWED, +) +from twisted.web.server import ( + NOT_DONE_YET, +) -from .common_tweb import ( - render, +import treq + +from ..webish import ( + TahoeLAFSRequest, ) @inlineCallbacks @@ -24,3 +45,51 @@ def do_http(method, url, **kwargs): if 400 <= response.code < 600: raise Error(response.code, response=body) returnValue(body) + + +def render(resource, query_args): + """ + Render (in the manner of the Twisted Web Site) a Twisted ``Resource`` + against a request with the given query arguments . + + :param resource: The page or resource to render. + + :param query_args: The query arguments to put into the request being + rendered. A mapping from ``bytes`` to ``list`` of ``bytes``. + + :return Deferred: A Deferred that fires with the rendered response body as + ``bytes``. + """ + channel = DummyChannel() + request = TahoeLAFSRequest(channel) + request.method = b"GET" + request.args = query_args + request.prepath = [b""] + request.postpath = [] + try: + result = resource.render(request) + except UnsupportedMethod: + request.setResponseCode(NOT_ALLOWED) + result = b"" + + if isinstance(result, bytes): + request.write(result) + done = succeed(None) + elif result == NOT_DONE_YET: + if request.finished: + done = succeed(None) + else: + done = request.notifyFinish() + else: + raise ValueError( + "{!r} returned {!r}, required bytes or NOT_DONE_YET.".format( + fullyQualifiedName(resource.render), + result, + ), + ) + def get_body(ignored): + complete_response = channel.transport.written.getvalue() + header, body = complete_response.split(b"\r\n\r\n", 1) + return body + done.addCallback(get_body) + return done From c8b9a0265a31bc57127675655ff8768683058d2f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:16:57 -0400 Subject: [PATCH 69/83] `get_root` docs --- src/allmydata/web/common.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index fef36f029..bcdf21581 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -152,6 +152,17 @@ def parse_offset_arg(offset): def get_root(req): + """ + Get a relative path with parent directory segments that refers to the root + location known to the given request. This seems a lot like the constant + absolute path **/** but it will behave differently if the Tahoe-LAFS HTTP + server is reverse-proxied and mounted somewhere other than at the root. + + :param twisted.web.iweb.IRequest req: The request to consider. + + :return: A string like ``../../..`` with the correct number of segments to + reach the root. + """ depth = len(req.prepath) + len(req.postpath) link = "/".join([".."] * depth) return link From e3b1d4f536029fa383cde5746b37af862ac8c72b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:17:52 -0400 Subject: [PATCH 70/83] enforce the type requirement --- src/allmydata/web/common.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index bcdf21581..d970cc918 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -21,6 +21,9 @@ from twisted.web import ( resource, template, ) +from twisted.web.iweb import ( + IRequest, +) from twisted.web.template import ( tags, ) @@ -163,6 +166,10 @@ def get_root(req): :return: A string like ``../../..`` with the correct number of segments to reach the root. """ + if not IRequest.providedBy(req): + raise TypeError( + "get_root requires IRequest provider, got {!r}".format(req), + ) depth = len(req.prepath) + len(req.postpath) link = "/".join([".."] * depth) return link From 8401547b3555b2d6b2ca2047dd553049edaa23ff Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:22:41 -0400 Subject: [PATCH 71/83] docstring for TahoeLAFSRequest --- src/allmydata/webish.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 75f0d03dc..8f36a6e15 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -31,6 +31,14 @@ from .web.storage_plugins import ( ) class TahoeLAFSRequest(Request, object): + """ + ``TahoeLAFSRequest`` adds several features to a Twisted Web ``Request`` + that are useful for Tahoe-LAFS. + + :ivar NoneType|FieldStorage fields: For POST requests, a structured + representation of the contents of the request body. For anything + else, ``None``. + """ fields = None _tahoe_request_had_error = None From e97b5f6bb42960eaa0cc41a52cfd29b456148209 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:22:51 -0400 Subject: [PATCH 72/83] document get_arg req parameter --- src/allmydata/web/common_py3.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/web/common_py3.py b/src/allmydata/web/common_py3.py index a906a6e61..22f235790 100644 --- a/src/allmydata/web/common_py3.py +++ b/src/allmydata/web/common_py3.py @@ -21,6 +21,8 @@ def get_arg(req, argname, default=None, multiple=False): (or the default, which defaults to None), and the query args take precedence. If multiple=True, this returns a tuple of arguments (possibly empty), starting with all those in the query args. + + :param TahoeLAFSRequest req: The request to consider. """ results = [] if argname in req.args: From a58fd5c88e62e6c8b74255f479afb3e1bc741b62 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 07:23:02 -0400 Subject: [PATCH 73/83] remove unused attribute --- src/allmydata/webish.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 8f36a6e15..b07d06be1 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -40,7 +40,6 @@ class TahoeLAFSRequest(Request, object): else, ``None``. """ fields = None - _tahoe_request_had_error = None def requestReceived(self, command, path, version): """ From 8239d92892f5448de380e900942e95bed33390a9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:04:48 -0400 Subject: [PATCH 74/83] news fragment --- newsfragments/3483.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3483.minor diff --git a/newsfragments/3483.minor b/newsfragments/3483.minor new file mode 100644 index 000000000..e69de29bb From bc8c2c46896b6a03e7b82436ef23e7f544534c01 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:04:59 -0400 Subject: [PATCH 75/83] Put all CircleCI jobs into the "dockerhub-auth" context --- .circleci/config.yml | 88 ++++++++++++++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 24 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1327a524b..9d9d0b6d3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -14,44 +14,73 @@ version: 2.1 workflows: ci: jobs: - # Platforms - - "debian-9" + # Start with jobs testing various platforms. + + # Every job that pulls a Docker image from Docker Hub needs to provide + # credentials for that pull operation to avoid being subjected to + # unauthenticated pull limits shared across all of CircleCI. Use this + # first job to define a yaml anchor that can be used to supply a + # CircleCI job context which makes Docker Hub credentials available in + # the environment. + # + # Contexts are managed in the CircleCI web interface: + # + # https://app.circleci.com/settings/organization/github/tahoe-lafs/contexts + - "debian-9": &DOCKERHUB_CONTEXT + context: "dockerhub-auth" + - "debian-8": + <<: *DOCKERHUB_CONTEXT requires: - "debian-9" - - "ubuntu-20-04" + - "ubuntu-20-04": + <<: *DOCKERHUB_CONTEXT - "ubuntu-18-04": + <<: *DOCKERHUB_CONTEXT requires: - "ubuntu-20-04" - "ubuntu-16-04": + <<: *DOCKERHUB_CONTEXT requires: - "ubuntu-20-04" - - "fedora-29" + - "fedora-29": + <<: *DOCKERHUB_CONTEXT - "fedora-28": + <<: *DOCKERHUB_CONTEXT requires: - "fedora-29" - - "centos-8" + - "centos-8": + <<: *DOCKERHUB_CONTEXT - - "nixos-19-09" + - "nixos-19-09": + <<: *DOCKERHUB_CONTEXT # Test against PyPy 2.7 - - "pypy27-buster" + - "pypy27-buster": + <<: *DOCKERHUB_CONTEXT # Just one Python 3.6 configuration while the port is in-progress. - - "python36" + - "python36": + <<: *DOCKERHUB_CONTEXT # Other assorted tasks and configurations - - "lint" - - "pyinstaller" - - "deprecations" - - "c-locale" + - "lint": + <<: *DOCKERHUB_CONTEXT + - "pyinstaller": + <<: *DOCKERHUB_CONTEXT + - "deprecations": + <<: *DOCKERHUB_CONTEXT + - "c-locale": + <<: *DOCKERHUB_CONTEXT # Any locale other than C or UTF-8. - - "another-locale" + - "another-locale": + <<: *DOCKERHUB_CONTEXT - "integration": + <<: *DOCKERHUB_CONTEXT requires: # If the unit test suite doesn't pass, don't bother running the # integration tests. @@ -59,7 +88,8 @@ workflows: # Generate the underlying data for a visualization to aid with Python 3 # porting. - - "build-porting-depgraph" + - "build-porting-depgraph": + <<: *DOCKERHUB_CONTEXT images: # Build the Docker images used by the ci jobs. This makes the ci jobs @@ -74,16 +104,26 @@ workflows: - "master" jobs: - - "build-image-debian-8" - - "build-image-debian-9" - - "build-image-ubuntu-16-04" - - "build-image-ubuntu-18-04" - - "build-image-ubuntu-20-04" - - "build-image-fedora-28" - - "build-image-fedora-29" - - "build-image-centos-8" - - "build-image-pypy27-buster" - - "build-image-python36-ubuntu" + - "build-image-debian-8": + <<: *DOCKERHUB_CONTEXT + - "build-image-debian-9": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-16-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-18-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-20-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-fedora-28": + <<: *DOCKERHUB_CONTEXT + - "build-image-fedora-29": + <<: *DOCKERHUB_CONTEXT + - "build-image-centos-8": + <<: *DOCKERHUB_CONTEXT + - "build-image-pypy27-buster": + <<: *DOCKERHUB_CONTEXT + - "build-image-python36-ubuntu": + <<: *DOCKERHUB_CONTEXT jobs: From 22921e2b1dcfb2c41edaababc0071cf4268bc2a3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:08:30 -0400 Subject: [PATCH 76/83] Use secrets from the context to authenticate with Docker Hub --- .circleci/config.yml | 101 ++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d9d0b6d3..12cdb843d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -127,9 +127,28 @@ workflows: jobs: - lint: + dockerhub-auth-template: + # This isn't a real job. It doesn't get scheduled as part of any + # workhlow. Instead, it's just a place we can hang a yaml anchor to + # finish the Docker Hub authentication configuration. Workflow jobs using + # the DOCKERHUB_CONTEXT anchor will have access to the environment + # variables used here. These variables will allow the Docker Hub image + # pull to be authenticated and hopefully avoid hitting and rate limits. docker: - - image: "circleci/python:2" + - image: "null" + auth: &DOCKERHUB_AUTH + username: $DOCKERHUB_USERNAME + password: $DOCKERHUB_PASSWORD + + steps: + - run: + name: "Schema conformity" + command: | + + lint: + docker: + - <<: *DOCKERHUB_AUTH + image: "circleci/python:2" steps: - "checkout" @@ -146,7 +165,8 @@ jobs: pyinstaller: docker: - - image: "circleci/python:2" + - <<: *DOCKERHUB_AUTH + image: "circleci/python:2" steps: - "checkout" @@ -171,7 +191,8 @@ jobs: debian-9: &DEBIAN docker: - - image: "tahoelafsci/debian:9-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/debian:9-py2.7" user: "nobody" environment: &UTF_8_ENVIRONMENT @@ -252,14 +273,16 @@ jobs: debian-8: <<: *DEBIAN docker: - - image: "tahoelafsci/debian:8-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/debian:8-py2.7" user: "nobody" pypy27-buster: <<: *DEBIAN docker: - - image: "tahoelafsci/pypy:buster-py2" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/pypy:buster-py2" user: "nobody" environment: @@ -320,21 +343,24 @@ jobs: ubuntu-16-04: <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:16.04-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:16.04-py2.7" user: "nobody" ubuntu-18-04: &UBUNTU_18_04 <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:18.04-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:18.04-py2.7" user: "nobody" python36: <<: *UBUNTU_18_04 docker: - - image: "tahoelafsci/ubuntu:18.04-py3" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:18.04-py3" user: "nobody" environment: @@ -349,13 +375,15 @@ jobs: ubuntu-20-04: <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:20.04" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:20.04" user: "nobody" centos-8: &RHEL_DERIV docker: - - image: "tahoelafsci/centos:8-py2" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/centos:8-py2" user: "nobody" environment: *UTF_8_ENVIRONMENT @@ -377,21 +405,24 @@ jobs: fedora-28: <<: *RHEL_DERIV docker: - - image: "tahoelafsci/fedora:28-py" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/fedora:28-py" user: "nobody" fedora-29: <<: *RHEL_DERIV docker: - - image: "tahoelafsci/fedora:29-py" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/fedora:29-py" user: "nobody" nixos-19-09: docker: # Run in a highly Nix-capable environment. - - image: "nixorg/nix:circleci" + - <<: *DOCKERHUB_AUTH + image: "nixorg/nix:circleci" environment: NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-19.09-small.tar.gz" @@ -448,7 +479,8 @@ jobs: # # https://circleci.com/blog/how-to-build-a-docker-image-on-circleci-2-0/ docker: - - image: "docker:17.05.0-ce-git" + - <<: *DOCKERHUB_AUTH + image: "docker:17.05.0-ce-git" environment: DISTRO: "tahoelafsci/:foo-py2" @@ -458,47 +490,10 @@ jobs: steps: - "checkout" - "setup_remote_docker" - - run: - name: "Get openssl" - command: | - apk add --no-cache openssl - - run: - name: "Get Dockerhub secrets" - command: | - # If you create an encryption key like this: - # - # openssl enc -aes-256-cbc -k secret -P -md sha256 - - # From the output that looks like: - # - # salt=... - # key=... - # iv =... - # - # extract just the value for ``key``. - - # then you can re-generate ``secret-env-cipher`` locally using the - # command: - # - # openssl aes-256-cbc -e -md sha256 -in secret-env-plain -out .circleci/secret-env-cipher -pass env:KEY - # - # Make sure the key is set as the KEY environment variable in the - # CircleCI web interface. You can do this by visiting - # - # after logging in to CircleCI with an account in the tahoe-lafs - # CircleCI team. - # - # Then you can recover the environment plaintext (for example, to - # change and re-encrypt it) like just like CircleCI recovers it - # here: - # - openssl aes-256-cbc -d -md sha256 -in .circleci/secret-env-cipher -pass env:KEY >> ~/.env - run: name: "Log in to Dockerhub" command: | - . ~/.env - # TAHOELAFSCI_PASSWORD come from the secret env. - docker login -u tahoelafsci -p ${TAHOELAFSCI_PASSWORD} + docker login -u ${DOCKERHUB_USERNAME} -p ${DOCKERHUB_PASSWORD} - run: name: "Build image" command: | From e778c8ab8462be01118448b5244e62fb346cd372 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:09:09 -0400 Subject: [PATCH 77/83] This is no longer used by anything --- .circleci/secret-env-cipher | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .circleci/secret-env-cipher diff --git a/.circleci/secret-env-cipher b/.circleci/secret-env-cipher deleted file mode 100644 index 2facc470c..000000000 --- a/.circleci/secret-env-cipher +++ /dev/null @@ -1 +0,0 @@ -Salted__GP)|![U[vS,Fm:~Y[U_Fxג%4lֻ81/l`n^Z]q&݂%Tn \ No newline at end of file From 5e1d3db72e87f21c40170bb4fa049ddc67c7f401 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:12:58 -0400 Subject: [PATCH 78/83] Correct whitespace --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 12cdb843d..316cad4c8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -145,7 +145,7 @@ jobs: name: "Schema conformity" command: | - lint: + lint: docker: - <<: *DOCKERHUB_AUTH image: "circleci/python:2" From 1303a852858b201c04146cdcafb04b0be9024c26 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:21:11 -0400 Subject: [PATCH 79/83] Attempt to get the Docker Hub auth into the right place --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 316cad4c8..a5def0f2a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -134,9 +134,9 @@ jobs: # the DOCKERHUB_CONTEXT anchor will have access to the environment # variables used here. These variables will allow the Docker Hub image # pull to be authenticated and hopefully avoid hitting and rate limits. - docker: + docker: &DOCKERHUB_AUTH - image: "null" - auth: &DOCKERHUB_AUTH + auth: username: $DOCKERHUB_USERNAME password: $DOCKERHUB_PASSWORD From e2f03e00ba409f440e4a78afd07c6f2d6d9351d9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:27:22 -0400 Subject: [PATCH 80/83] typo --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a5def0f2a..b097812cb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -129,7 +129,7 @@ workflows: jobs: dockerhub-auth-template: # This isn't a real job. It doesn't get scheduled as part of any - # workhlow. Instead, it's just a place we can hang a yaml anchor to + # workflow. Instead, it's just a place we can hang a yaml anchor to # finish the Docker Hub authentication configuration. Workflow jobs using # the DOCKERHUB_CONTEXT anchor will have access to the environment # variables used here. These variables will allow the Docker Hub image From 81428d083940f46503387a03b44d5c443d529851 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 14:46:26 -0400 Subject: [PATCH 81/83] explain "Schema conformity" a bit more --- .circleci/config.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b097812cb..afa3fafa1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -142,8 +142,12 @@ jobs: steps: - run: - name: "Schema conformity" + name: "CircleCI YAML schema conformity" command: | + # This isn't a real command. We have to have something in this + # space, though, or the CircleCI yaml schema validator gets angry. + # Since this job is never scheduled this step is never run so the + # actual value here is irrelevant. lint: docker: From da75fa40694511641a5be4bbef602a0be57f3b5e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 14:47:18 -0400 Subject: [PATCH 82/83] make all the image builders run too, to see if they will --- .circleci/config.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index afa3fafa1..6bb02ce95 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -91,19 +91,19 @@ workflows: - "build-porting-depgraph": <<: *DOCKERHUB_CONTEXT - images: - # Build the Docker images used by the ci jobs. This makes the ci jobs - # faster and takes various spurious failures out of the critical path. - triggers: - # Build once a day - - schedule: - cron: "0 0 * * *" - filters: - branches: - only: - - "master" + # images: + # # Build the Docker images used by the ci jobs. This makes the ci jobs + # # faster and takes various spurious failures out of the critical path. + # triggers: + # # Build once a day + # - schedule: + # cron: "0 0 * * *" + # filters: + # branches: + # only: + # - "master" - jobs: + # jobs: - "build-image-debian-8": <<: *DOCKERHUB_CONTEXT - "build-image-debian-9": From dddf49ff71cdea5977d2d1784e30650967a7a5d8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 17:00:02 -0400 Subject: [PATCH 83/83] Restore original image configuration --- .circleci/config.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6bb02ce95..afa3fafa1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -91,19 +91,19 @@ workflows: - "build-porting-depgraph": <<: *DOCKERHUB_CONTEXT - # images: - # # Build the Docker images used by the ci jobs. This makes the ci jobs - # # faster and takes various spurious failures out of the critical path. - # triggers: - # # Build once a day - # - schedule: - # cron: "0 0 * * *" - # filters: - # branches: - # only: - # - "master" + images: + # Build the Docker images used by the ci jobs. This makes the ci jobs + # faster and takes various spurious failures out of the critical path. + triggers: + # Build once a day + - schedule: + cron: "0 0 * * *" + filters: + branches: + only: + - "master" - # jobs: + jobs: - "build-image-debian-8": <<: *DOCKERHUB_CONTEXT - "build-image-debian-9":