diff --git a/newsfragments/4044.minor b/newsfragments/4044.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/cli/grid_manager.py b/src/allmydata/cli/grid_manager.py index a5e82fed3..dfefeb576 100644 --- a/src/allmydata/cli/grid_manager.py +++ b/src/allmydata/cli/grid_manager.py @@ -28,7 +28,7 @@ from allmydata.grid_manager import ( from allmydata.util import jsonbytes as json -@click.group() +@click.group() # type: ignore[arg-type] @click.option( '--config', '-c', type=click.Path(), @@ -71,7 +71,7 @@ def grid_manager(ctx, config): ctx.obj = Config() -@grid_manager.command() +@grid_manager.command() # type: ignore[attr-defined] @click.pass_context def create(ctx): """ @@ -91,7 +91,7 @@ def create(ctx): ) -@grid_manager.command() +@grid_manager.command() # type: ignore[attr-defined] @click.pass_obj def public_identity(config): """ @@ -103,7 +103,7 @@ def public_identity(config): click.echo(config.grid_manager.public_identity()) -@grid_manager.command() +@grid_manager.command() # type: ignore[arg-type, attr-defined] @click.argument("name") @click.argument("public_key", type=click.STRING) @click.pass_context @@ -132,7 +132,7 @@ def add(ctx, name, public_key): return 0 -@grid_manager.command() +@grid_manager.command() # type: ignore[arg-type, attr-defined] @click.argument("name") @click.pass_context def remove(ctx, name): @@ -155,7 +155,8 @@ def remove(ctx, name): save_grid_manager(fp, ctx.obj.grid_manager, create=False) -@grid_manager.command() # noqa: F811 +@grid_manager.command() # type: ignore[attr-defined] + # noqa: F811 @click.pass_context def list(ctx): """ @@ -175,7 +176,7 @@ def list(ctx): click.echo("expired {} ({})".format(cert.expires, abbreviate_time(delta))) -@grid_manager.command() +@grid_manager.command() # type: ignore[arg-type, attr-defined] @click.argument("name") @click.argument( "expiry_days", diff --git a/src/allmydata/client.py b/src/allmydata/client.py index e85ed4fe2..a1501f1ef 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -1,5 +1,5 @@ """ -Ported to Python 3. +Functionality related to operating a Tahoe-LAFS node (client _or_ server). """ from __future__ import annotations @@ -1029,14 +1029,14 @@ class _Client(node.Node, pollmixin.PollMixin): def init_web(self, webport): self.log("init_web(webport=%s)", args=(webport,)) - from allmydata.webish import WebishServer + from allmydata.webish import WebishServer, anonymous_tempfile_factory nodeurl_path = self.config.get_config_path("node.url") staticdir_config = self.config.get_config("node", "web.static", "public_html") staticdir = self.config.get_config_path(staticdir_config) ws = WebishServer( self, webport, - self._get_tempdir(), + anonymous_tempfile_factory(self._get_tempdir()), nodeurl_path, staticdir, ) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 08dce0ac0..42be0f50d 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -341,7 +341,7 @@ class WebMixin(TimezoneMixin): self.ws = webish.WebishServer( self.s, "0", - tempdir=tempdir.path, + webish.anonymous_tempfile_factory(tempdir.path), staticdir=self.staticdir, clock=self.clock, now_fn=lambda:self.fakeTime, diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 050f77d1c..523dfc878 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -1,23 +1,11 @@ """ Tests for ``allmydata.webish``. - -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 tempfile from uuid import ( uuid4, ) -from errno import ( - EACCES, -) from io import ( BytesIO, ) @@ -39,9 +27,6 @@ from testtools.matchers import ( HasLength, ) -from twisted.python.runtime import ( - platform, -) from twisted.python.filepath import ( FilePath, ) @@ -59,6 +44,7 @@ from ..common import ( from ...webish import ( TahoeLAFSRequest, TahoeLAFSSite, + anonymous_tempfile_factory, ) @@ -183,8 +169,14 @@ class TahoeLAFSSiteTests(SyncTestCase): :return: ``None`` if the logging looks good. """ logPath = self.mktemp() + tempdir = self.mktemp() + FilePath(tempdir).makedirs() - site = TahoeLAFSSite(self.mktemp(), Resource(), logPath=logPath) + site = TahoeLAFSSite( + anonymous_tempfile_factory(tempdir), + Resource(), + logPath=logPath, + ) site.startFactory() channel = DummyChannel() @@ -257,11 +249,17 @@ class TahoeLAFSSiteTests(SyncTestCase): Create and return a new ``TahoeLAFSRequest`` hooked up to a ``TahoeLAFSSite``. - :param bytes tempdir: The temporary directory to give to the site. + :param FilePath tempdir: The temporary directory to configure the site + to write large temporary request bodies to. The temporary files + will be named for ease of testing. :return TahoeLAFSRequest: The new request instance. """ - site = TahoeLAFSSite(tempdir.path, Resource(), logPath=self.mktemp()) + site = TahoeLAFSSite( + lambda: tempfile.NamedTemporaryFile(dir=tempdir.path), + Resource(), + logPath=self.mktemp(), + ) site.startFactory() channel = DummyChannel() @@ -275,6 +273,7 @@ class TahoeLAFSSiteTests(SyncTestCase): A request body smaller than 1 MiB is kept in memory. """ tempdir = FilePath(self.mktemp()) + tempdir.makedirs() request = self._create_request(tempdir) request.gotLength(request_body_size) self.assertThat( @@ -284,57 +283,21 @@ class TahoeLAFSSiteTests(SyncTestCase): def _large_request_test(self, request_body_size): """ - Assert that when a request with a body of of the given size is received - its content is written to the directory the ``TahoeLAFSSite`` is - configured with. + Assert that when a request with a body of the given size is + received its content is written a temporary file created by the given + tempfile factory. """ tempdir = FilePath(self.mktemp()) tempdir.makedirs() request = self._create_request(tempdir) - - # So. Bad news. The temporary file for the uploaded content is - # unnamed (and this isn't even necessarily a bad thing since it is how - # you get automatic on-process-exit cleanup behavior on POSIX). It's - # not visible by inspecting the filesystem. It has no name we can - # discover. Then how do we verify it is written to the right place? - # The question itself is meaningless if we try to be too precise. It - # *has* no filesystem location. However, it is still stored *on* some - # filesystem. We still want to make sure it is on the filesystem we - # specified because otherwise it might be on a filesystem that's too - # small or undesirable in some other way. - # - # I don't know of any way to ask a file descriptor which filesystem - # it's on, either, though. It might be the case that the [f]statvfs() - # result could be compared somehow to infer the filesystem but - # ... it's not clear what the failure modes might be there, across - # different filesystems and runtime environments. - # - # Another approach is to make the temp directory unwriteable and - # observe the failure when an attempt is made to create a file there. - # This is hardly a lovely solution but at least it's kind of simple. - # - # It would be nice if it worked consistently cross-platform but on - # Windows os.chmod is more or less broken. - if platform.isWindows(): - request.gotLength(request_body_size) - self.assertThat( - tempdir.children(), - HasLength(1), - ) - else: - tempdir.chmod(0o550) - with self.assertRaises(OSError) as ctx: - request.gotLength(request_body_size) - raise Exception( - "OSError not raised, instead tempdir.children() = {}".format( - tempdir.children(), - ), - ) - - self.assertThat( - ctx.exception.errno, - Equals(EACCES), - ) + request.gotLength(request_body_size) + # We can see the temporary file in the temporary directory we + # specified because _create_request makes a request that uses named + # temporary files instead of the usual anonymous temporary files. + self.assertThat( + tempdir.children(), + HasLength(1), + ) def test_unknown_request_size(self): """ diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index ec2582f80..ec5ad64c0 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -4,7 +4,7 @@ General web server-related utilities. from __future__ import annotations from six import ensure_str - +from typing import IO, Callable, Optional import re, time, tempfile from urllib.parse import parse_qsl, urlencode @@ -217,36 +217,50 @@ def censor(queryargs: bytes) -> bytes: return urlencode(result, safe="[]").encode("ascii") +def anonymous_tempfile_factory(tempdir: bytes) -> Callable[[], IO[bytes]]: + """ + Create a no-argument callable for creating a new temporary file in the + given directory. + + :param tempdir: The directory in which temporary files with be created. + + :return: The callable. + """ + return lambda: tempfile.TemporaryFile(dir=tempdir) + + class TahoeLAFSSite(Site, object): """ The HTTP protocol factory used by Tahoe-LAFS. Among the behaviors provided: - * A configurable temporary directory where large request bodies can be - written so they don't stay in memory. + * A configurable temporary file factory for large request bodies to avoid + keeping them in memory. * A log formatter that writes some access logs but omits capability strings to help keep them secret. """ requestFactory = TahoeLAFSRequest - def __init__(self, tempdir, *args, **kwargs): + def __init__(self, make_tempfile: Callable[[], IO[bytes]], *args, **kwargs): Site.__init__(self, *args, logFormatter=_logFormatter, **kwargs) - self._tempdir = tempdir + assert callable(make_tempfile) + with make_tempfile(): + pass + self._make_tempfile = make_tempfile - def getContentFile(self, length): + def getContentFile(self, length: Optional[int]) -> IO[bytes]: if length is None or length >= 1024 * 1024: - return tempfile.TemporaryFile(dir=self._tempdir) + return self._make_tempfile() return BytesIO() - class WebishServer(service.MultiService): # The type in Twisted for services is wrong in 22.10... # https://github.com/twisted/twisted/issues/10135 name = "webish" # type: ignore[assignment] - def __init__(self, client, webport, tempdir, nodeurl_path=None, staticdir=None, + def __init__(self, client, webport, make_tempfile, nodeurl_path=None, staticdir=None, clock=None, now_fn=time.time): service.MultiService.__init__(self) # the 'data' argument to all render() methods default to the Client @@ -256,7 +270,7 @@ class WebishServer(service.MultiService): # time in a deterministic manner. self.root = root.Root(client, clock, now_fn) - self.buildServer(webport, tempdir, nodeurl_path, staticdir) + self.buildServer(webport, make_tempfile, nodeurl_path, staticdir) # If set, clock is a twisted.internet.task.Clock that the tests # use to test ophandle expiration. @@ -266,9 +280,9 @@ class WebishServer(service.MultiService): self.root.putChild(b"storage-plugins", StoragePlugins(client)) - def buildServer(self, webport, tempdir, nodeurl_path, staticdir): + def buildServer(self, webport, make_tempfile, nodeurl_path, staticdir): self.webport = webport - self.site = TahoeLAFSSite(tempdir, self.root) + self.site = TahoeLAFSSite(make_tempfile, self.root) self.staticdir = staticdir # so tests can check if staticdir: self.root.putChild(b"static", static.File(staticdir)) @@ -346,4 +360,4 @@ class IntroducerWebishServer(WebishServer): def __init__(self, introducer, webport, nodeurl_path=None, staticdir=None): service.MultiService.__init__(self) self.root = introweb.IntroducerRoot(introducer) - self.buildServer(webport, tempfile.tempdir, nodeurl_path, staticdir) + self.buildServer(webport, tempfile.TemporaryFile, nodeurl_path, staticdir) diff --git a/tox.ini b/tox.ini index 2f245f2ed..127cd9178 100644 --- a/tox.ini +++ b/tox.ini @@ -100,6 +100,9 @@ deps = # Pin a specific version so we get consistent outcomes; update this # occasionally: ruff == 0.0.263 + # towncrier doesn't work with importlib_resources 6.0.0 + # https://github.com/twisted/towncrier/issues/528 + importlib_resources < 6.0.0 towncrier # On macOS, git inside of towncrier needs $HOME. passenv = HOME