diff --git a/integration/test_streaming_logs.py b/integration/test_streaming_logs.py index 32b97644d..52c813f9b 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) @@ -131,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) diff --git a/newsfragments/3314.minor b/newsfragments/3314.minor new file mode 100644 index 000000000..e69de29bb diff --git a/newsfragments/3428.minor b/newsfragments/3428.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/test/common_nevow.py b/src/allmydata/test/common_nevow.py new file mode 100644 index 000000000..d6327f9c6 --- /dev/null +++ b/src/allmydata/test/common_nevow.py @@ -0,0 +1,50 @@ +""" +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_tweb.py b/src/allmydata/test/common_tweb.py new file mode 100644 index 000000000..37e24b5b8 --- /dev/null +++ b/src/allmydata/test/common_tweb.py @@ -0,0 +1,54 @@ +from twisted.python.reflect import ( + fullyQualifiedName, +) +from twisted.internet.defer import ( + succeed, +) +from twisted.web.test.requesthelper import ( + DummyChannel, +) +from twisted.web.server import ( + Request, +) +from twisted.web.server import ( + NOT_DONE_YET, +) + +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 = Request(channel) + 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, + ), + ) + 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 e2ea57539..08b356cd0 100644 --- a/src/allmydata/test/common_web.py +++ b/src/allmydata/test/common_web.py @@ -1,25 +1,18 @@ +__all__ = [ + "do_http", + "render", +] + +from future.utils import PY2 + import treq from twisted.internet.defer import ( - maybeDeferred, inlineCallbacks, returnValue, ) 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 +24,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, + ) diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index e8f7e3ce1..3813ac199 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -52,6 +52,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, @@ -184,11 +187,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() diff --git a/src/allmydata/test/web/test_common.py b/src/allmydata/test/web/test_common.py new file mode 100644 index 000000000..6431ba610 --- /dev/null +++ b/src/allmydata/test/web/test_common.py @@ -0,0 +1,216 @@ +""" +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, 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 + + @render_exception + def render(self, request): + return self._response + + +class RenderExceptionTests(SyncTestCase): + """ + Tests for ``render_exception`` (including the private helper ``_finish``). + """ + 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/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/test/web/test_root.py b/src/allmydata/test/web/test_root.py index 1a29b7a15..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,13 +16,9 @@ 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, +from ..common_web import ( + render, ) - from ..common import ( EMPTY_CLIENT_CONFIG, ) @@ -36,13 +30,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,51 +37,29 @@ class RenderSlashUri(unittest.TestCase): """ A valid capbility does not result in error """ - self.request.args[b"uri"] = [( + query_args = {b"uri": [ b"URI:CHK:nt2xxmrccp7sursd6yh2thhcky:" b"mukesarwdjxiyqsjinbfiiro6q7kgmmekocxfjcngh23oxwyxtzq:2:5:5874882" - )] - self.res.render_GET(self.request) + ]} + response_body = self.successResultOf( + render(self.res, query_args), + ) + self.assertNotEqual( + response_body, + "Invalid capability", + ) 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) - - soup = BeautifulSoup(response_body, 'html5lib') - - assert_soup_has_tag_with_content( - self, soup, "title", "400 - Error", + query_args = {b"uri": [b"not a capability"]} + response_body = self.successResultOf( + render(self.res, query_args), ) - 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 - """ - self.request.args[b"uri"] = [cap.encode('utf8')] - response_body = self.res.render_GET(self.request) - - 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", ) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 16970e5e2..c9d59e7ae 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -5,14 +5,49 @@ import time import json from functools import wraps +from hyperlink import ( + DecodedURL, +) + +from eliot import ( + Message, + start_action, +) +from eliot.twisted import ( + DeferredContext, +) + from twisted.web import ( http, resource, - server, template, ) -from twisted.web.iweb import IRequest as ITwistedRequest +from twisted.web.template import ( + tags, +) +from twisted.web.server import ( + NOT_DONE_YET, + UnsupportedMethod, +) +from twisted.web.util import ( + DeferredResource, + FailureElement, + redirectTo, +) +from twisted.python.reflect import ( + fullyQualifiedName, +) from twisted.python import log +from twisted.python.failure import ( + Failure, +) +from twisted.internet.defer import ( + maybeDeferred, +) +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 @@ -332,48 +367,10 @@ def humanize_failure(f): class MyExceptionHandler(DefaultExceptionHandler, object): - def simple(self, ctx, text, code=http.BAD_REQUEST): - req = INevowRequest(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 = INevowRequest(ctx) - method = req.method - return self.simple(ctx, - "I don't know how to treat a %s request." % method, - http.NOT_IMPLEMENTED) req = INevowRequest(ctx) - accept = req.getHeader("accept") - if not accept: - accept = "*/*" - if "*/*" in accept or "text/*" in accept or "text/html" in accept: - super = 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): @@ -425,31 +422,253 @@ class SlotsSequenceElement(template.Element): return tag -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) + # 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( + 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 render_exception(f): +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 + + +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 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) + # 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( + action_type=u"allmydata:web:common-render", + uri=request.uri, + 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)) + # Apply `_finish` all of our result handling logic to whatever it + # returned. + result.addBoth(_finish, bound_render, request) + result.addActionFinish() + return NOT_DONE_YET + return g + + +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", + message=result.getErrorMessage(), + ) + _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. + Message.log( + message_type=u"allmydata:web:common-render:resource", + 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", + ) + request.write(result) + request.finish() + 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), + result, + )) + request.setResponseCode(http.INTERNAL_SERVER_ERROR) + _finish(b"Internal Server Error", 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(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 + + +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/directory.py b/src/allmydata/web/directory.py index 4d7aa1bd1..0d521951f 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", "") @@ -404,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): diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 0ecc8cc52..7686b35b5 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -8,8 +8,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 +32,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 +148,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 +310,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 +477,9 @@ 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 +491,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 a409561af..0e53d075d 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,14 +1,15 @@ -from future.utils import PY2 + import time -if PY2: - from nevow import url -else: - # This module still needs porting to Python 3 - url = None +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 @@ -18,7 +19,6 @@ from twisted.application import service from allmydata.web.common import ( WebError, - get_root, get_arg, boolean_of_arg, exception_to_child, @@ -88,17 +88,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 + 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 + "?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..e420a0371 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -1,5 +1,6 @@ import urllib + from twisted.web import http from twisted.internet import defer from twisted.python.filepath import FilePath @@ -10,7 +11,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 ( @@ -21,6 +21,7 @@ from allmydata.web.common import ( get_format, get_mutable_type, render_exception, + url_for_string, ) from allmydata.web import status @@ -66,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 url.URL.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 @@ -160,7 +161,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: @@ -179,7 +179,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: @@ -198,7 +197,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: