From 0b45c9b1ccf45a83b08bff70aeb0415a31d4a7ff Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 15:41:56 -0500 Subject: [PATCH 01/16] news fragment --- newsfragments/3512.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3512.minor diff --git a/newsfragments/3512.minor b/newsfragments/3512.minor new file mode 100644 index 000000000..e69de29bb From b1244543f2f4b54c3c3fe651d0b0af8b08b9647f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 15:42:10 -0500 Subject: [PATCH 02/16] Bump to a Twisted that has Site.getContentFile support --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c27681ea8..c26805684 100644 --- a/setup.py +++ b/setup.py @@ -98,7 +98,9 @@ install_requires = [ # `pip install tahoe-lafs[sftp]` would not install requirements # specified by Twisted[conch]. Since this would be the *whole point* of # an sftp extra in Tahoe-LAFS, there is no point in having one. - "Twisted[tls,conch] >= 18.4.0", + # * Twisted 19.10 introduces Site.getContentFile which we use to get + # temporary upload files placed into a per-node temporary directory. + "Twisted[tls,conch] >= 19.10.0", "PyYAML >= 3.11", From 46955202e28091b9fbc87e22f8c3055c16b9c42b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 15:47:06 -0500 Subject: [PATCH 03/16] Hook into Twisted Web to control where request bodies are written --- src/allmydata/client.py | 24 +++++- src/allmydata/test/web/test_web.py | 10 ++- src/allmydata/test/web/test_webish.py | 104 +++++++++++++++++++++++++- src/allmydata/webish.py | 45 +++++++---- 4 files changed, 163 insertions(+), 20 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index a768ba354..88a74e186 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -33,6 +33,7 @@ from allmydata.introducer.client import IntroducerClient from allmydata.util import ( hashutil, base32, pollmixin, log, idlib, yamlutil, configutil, + fileutil, ) from allmydata.util.encodingutil import get_filesystem_encoding from allmydata.util.abbreviate import parse_abbreviated_size @@ -1043,6 +1044,21 @@ class _Client(node.Node, pollmixin.PollMixin): def set_default_mutable_keysize(self, keysize): self._key_generator.set_default_keysize(keysize) + def _get_tempdir(self): + """ + Determine the path to the directory where temporary files for this node + should be written. + + :return bytes: The path which will exist and be a directory. + """ + tempdir_config = self.config.get_config("node", "tempdir", "tmp") + if isinstance(tempdir_config, bytes): + tempdir_config = tempdir_config.decode('utf-8') + tempdir = self.config.get_config_path(tempdir_config) + if not os.path.exists(tempdir): + fileutil.make_dirs(tempdir) + return tempdir + def init_web(self, webport): self.log("init_web(webport=%s)", args=(webport,)) @@ -1050,7 +1066,13 @@ class _Client(node.Node, pollmixin.PollMixin): 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, nodeurl_path, staticdir) + ws = WebishServer( + self, + webport, + self._get_tempdir(), + nodeurl_path, + staticdir, + ) ws.setServiceParent(self) def init_ftp_server(self): diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index aa6d44ea4..27611e678 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -316,8 +316,14 @@ class WebMixin(TimezoneMixin): self.staticdir = self.mktemp() self.clock = Clock() self.fakeTime = 86460 # 1d 0h 1m 0s - self.ws = webish.WebishServer(self.s, "0", staticdir=self.staticdir, - clock=self.clock, now_fn=lambda:self.fakeTime) + self.ws = webish.WebishServer( + self.s, + "0", + tempdir=self.mktemp(), + staticdir=self.staticdir, + clock=self.clock, + now_fn=lambda:self.fakeTime, + ) self.ws.setServiceParent(self.s) self.webish_port = self.ws.getPortnum() self.webish_url = self.ws.getURL() diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 1e659812f..67bfca69e 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -5,6 +5,19 @@ Tests for ``allmydata.webish``. from uuid import ( uuid4, ) +from errno import ( + EACCES, +) +from io import ( + BytesIO, +) + +from hypothesis import ( + given, +) +from hypothesis.strategies import ( + integers, +) from testtools.matchers import ( AfterPreprocessing, @@ -12,6 +25,7 @@ from testtools.matchers import ( Equals, MatchesAll, Not, + IsInstance, ) from twisted.python.filepath import ( @@ -30,7 +44,7 @@ from ..common import ( from ...webish import ( TahoeLAFSRequest, - tahoe_lafs_site, + TahoeLAFSSite, ) @@ -96,7 +110,7 @@ class TahoeLAFSRequestTests(SyncTestCase): class TahoeLAFSSiteTests(SyncTestCase): """ - Tests for the ``Site`` created by ``tahoe_lafs_site``. + Tests for ``TahoeLAFSSite``. """ def _test_censoring(self, path, censored): """ @@ -112,7 +126,7 @@ class TahoeLAFSSiteTests(SyncTestCase): """ logPath = self.mktemp() - site = tahoe_lafs_site(Resource(), logPath=logPath) + site = TahoeLAFSSite(self.mktemp(), Resource(), logPath=logPath) site.startFactory() channel = DummyChannel() @@ -170,6 +184,90 @@ class TahoeLAFSSiteTests(SyncTestCase): b"/uri?uri=[CENSORED]", ) + def _get_request(self, tempdir): + """ + Create and return a new ``TahoeLAFSRequest`` hooked up to a + ``TahoeLAFSSite``. + + :param bytes tempdir: The temporary directory to give to the site. + + :return TahoeLAFSRequest: The new request instance. + """ + site = TahoeLAFSSite(tempdir.path, Resource(), logPath=self.mktemp()) + site.startFactory() + + channel = DummyChannel() + channel.site = site + request = TahoeLAFSRequest(channel) + return request + + @given(integers(min_value=0, max_value=1024 * 1024 - 1)) + def test_small_content(self, request_body_size): + """ + A request body smaller than 1 MiB is kept in memory. + """ + tempdir = FilePath(self.mktemp()) + request = self._get_request(tempdir) + request.gotLength(request_body_size) + self.assertThat( + request.content, + IsInstance(BytesIO), + ) + + 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. + """ + tempdir = FilePath(self.mktemp()) + tempdir.makedirs() + request = self._get_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. + tempdir.chmod(0o550) + with self.assertRaises(OSError) as ctx: + request.gotLength(request_body_size) + self.assertThat( + ctx.exception.errno, + Equals(EACCES), + ) + + def test_unknown_request_size(self): + """ + A request body with an unknown size is written to a file in the temporary + directory passed to ``TahoeLAFSSite``. + """ + self._large_request_test(None) + + @given(integers(min_value=1024 * 1024)) + def test_large_request(self, request_body_size): + """ + A request body of 1 MiB or more is written to a file in the temporary + directory passed to ``TahoeLAFSSite``. + """ + self._large_request_test(request_body_size) + + def param(name, value): return u"; {}={}".format(name, value) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index f94d6f7da..779458a59 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,13 +1,13 @@ from six import ensure_str -import re, time +import re, time, tempfile -from functools import ( - partial, -) from cgi import ( FieldStorage, ) +from io import ( + BytesIO, +) from twisted.application import service, strports, internet from twisted.web import static @@ -150,17 +150,34 @@ def _logFormatter(logDateTime, request): ) -tahoe_lafs_site = partial( - Site, - requestFactory=TahoeLAFSRequest, - logFormatter=_logFormatter, -) +class TahoeLAFSSite(Site): + """ + 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 log formatter that writes some access logs but omits capability + strings to help keep them secret. + """ + requestFactory = TahoeLAFSRequest + + def __init__(self, tempdir, *args, **kwargs): + Site.__init__(self, *args, logFormatter=_logFormatter, **kwargs) + self._tempdir = tempdir + + def getContentFile(self, length): + if length is None or length >= 1024 * 1024: + return tempfile.TemporaryFile(dir=self._tempdir) + return BytesIO() class WebishServer(service.MultiService): name = "webish" - def __init__(self, client, webport, nodeurl_path=None, staticdir=None, + def __init__(self, client, webport, tempdir, 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 @@ -170,7 +187,7 @@ class WebishServer(service.MultiService): # time in a deterministic manner. self.root = root.Root(client, clock, now_fn) - self.buildServer(webport, nodeurl_path, staticdir) + self.buildServer(webport, tempdir, nodeurl_path, staticdir) # If set, clock is a twisted.internet.task.Clock that the tests # use to test ophandle expiration. @@ -180,9 +197,9 @@ class WebishServer(service.MultiService): self.root.putChild(b"storage-plugins", StoragePlugins(client)) - def buildServer(self, webport, nodeurl_path, staticdir): + def buildServer(self, webport, tempdir, nodeurl_path, staticdir): self.webport = webport - self.site = tahoe_lafs_site(self.root) + self.site = TahoeLAFSSite(tempdir, self.root) self.staticdir = staticdir # so tests can check if staticdir: self.root.putChild("static", static.File(staticdir)) @@ -260,4 +277,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, nodeurl_path, staticdir) + self.buildServer(webport, tempfile.tempdir, nodeurl_path, staticdir) From 6d137ac257da5feae1928e3fe59fadd1d6e5c39c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 15:51:08 -0500 Subject: [PATCH 04/16] Get rid of the tempfile.tempdir hackery --- src/allmydata/node.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 9e7143fd4..8eb199ef3 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -740,8 +740,6 @@ class Node(service.MultiService): self._i2p_provider = i2p_provider self._tor_provider = tor_provider - self.init_tempdir() - self.create_log_tub() self.logSource = "Node" self.setup_logging() @@ -768,25 +766,6 @@ class Node(service.MultiService): """ return len(self.tub.getListeners()) > 0 - def init_tempdir(self): - """ - Initialize/create a directory for temporary files. - """ - tempdir_config = self.config.get_config("node", "tempdir", "tmp") - if isinstance(tempdir_config, bytes): - tempdir_config = tempdir_config.decode('utf-8') - tempdir = self.config.get_config_path(tempdir_config) - if not os.path.exists(tempdir): - fileutil.make_dirs(tempdir) - tempfile.tempdir = tempdir - # this should cause twisted.web.http (which uses - # tempfile.TemporaryFile) to put large request bodies in the given - # directory. Without this, the default temp dir is usually /tmp/, - # which is frequently too small. - temp_fd, test_name = tempfile.mkstemp() - _assert(os.path.dirname(test_name) == tempdir, test_name, tempdir) - os.close(temp_fd) # avoid leak of unneeded fd - # pull this outside of Node's __init__ too, see: # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2948 def create_log_tub(self): From 799e5a2a60758c627d8abba2a4355988936d1225 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 15:52:04 -0500 Subject: [PATCH 05/16] tweak comment about our test case --- src/allmydata/test/common.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index 1cf1d6428..02393b8df 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -1151,8 +1151,9 @@ class _TestCaseMixin(object): test (including setUp and tearDown messages). * trial-compatible mktemp method * unittest2-compatible assertRaises helper - * Automatic cleanup of tempfile.tempdir mutation (pervasive through the - Tahoe-LAFS test suite). + * Automatic cleanup of tempfile.tempdir mutation (once pervasive through + the Tahoe-LAFS test suite, perhaps gone now but someone should verify + this). """ def setUp(self): # Restore the original temporary directory. Node ``init_tempdir`` From 5b0d20c45349ff556b3d6c2893199c7c5ac4cd99 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 16:53:28 -0500 Subject: [PATCH 06/16] Everything should be new-style --- src/allmydata/webish.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 779458a59..b5e310fbc 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -150,7 +150,7 @@ def _logFormatter(logDateTime, request): ) -class TahoeLAFSSite(Site): +class TahoeLAFSSite(Site, object): """ The HTTP protocol factory used by Tahoe-LAFS. From 92691c1b32c33cb309767fa72994dc6a36156939 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 16:53:38 -0500 Subject: [PATCH 07/16] Be sure the temporary directory exists --- src/allmydata/test/web/test_web.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 27611e678..326569a26 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -6,6 +6,9 @@ import treq from bs4 import BeautifulSoup +from twisted.python.filepath import ( + FilePath, +) from twisted.application import service from twisted.internet import defer from twisted.internet.defer import inlineCallbacks, returnValue @@ -316,10 +319,12 @@ class WebMixin(TimezoneMixin): self.staticdir = self.mktemp() self.clock = Clock() self.fakeTime = 86460 # 1d 0h 1m 0s + tempdir = FilePath(self.mktemp()) + tempdir.makedirs() self.ws = webish.WebishServer( self.s, "0", - tempdir=self.mktemp(), + tempdir=tempdir.path, staticdir=self.staticdir, clock=self.clock, now_fn=lambda:self.fakeTime, From f240cb183f51d1a84643569954058032929176f7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 18:13:01 -0500 Subject: [PATCH 08/16] flake cleanup --- src/allmydata/node.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 8eb199ef3..3d41f9973 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -19,7 +19,6 @@ import os.path import re import types import errno -import tempfile from base64 import b32decode, b32encode # On Python 2 this will be the backported package. @@ -33,7 +32,6 @@ import foolscap.logging.log from allmydata.version_checks import get_package_versions, get_package_versions_string from allmydata.util import log from allmydata.util import fileutil, iputil -from allmydata.util.assertutil import _assert from allmydata.util.fileutil import abspath_expanduser_unicode from allmydata.util.encodingutil import get_filesystem_encoding, quote_output from allmydata.util import configutil From 4c19d9f1fa6dfe49cbd90e045e4d6fdc8016cc4e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 18:13:43 -0500 Subject: [PATCH 09/16] Target the non-duplicate ticket --- newsfragments/{3512.minor => 1549.minor} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{3512.minor => 1549.minor} (100%) diff --git a/newsfragments/3512.minor b/newsfragments/1549.minor similarity index 100% rename from newsfragments/3512.minor rename to newsfragments/1549.minor From cd1cc1f2cc2d235d86103649fbf35f392a3aed4f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 18 Nov 2020 21:04:53 -0500 Subject: [PATCH 10/16] Package our own Twisted 19.10 --- nix/overlays.nix | 3 +++ nix/twisted.nix | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 nix/twisted.nix diff --git a/nix/overlays.nix b/nix/overlays.nix index 4ee63a412..2bf58575e 100644 --- a/nix/overlays.nix +++ b/nix/overlays.nix @@ -15,6 +15,9 @@ self: super: { # Need version of pyutil that supports Python 3. The version in 19.09 # is too old. pyutil = python-super.callPackage ./pyutil.nix { }; + + # Need a newer version of Twisted, too. + twisted = python-super.callPackage ./twisted.nix { }; }; }; } diff --git a/nix/twisted.nix b/nix/twisted.nix new file mode 100644 index 000000000..3c11e3c71 --- /dev/null +++ b/nix/twisted.nix @@ -0,0 +1,63 @@ +{ stdenv +, buildPythonPackage +, fetchPypi +, python +, zope_interface +, incremental +, automat +, constantly +, hyperlink +, pyhamcrest +, attrs +, pyopenssl +, service-identity +, setuptools +, idna +, bcrypt +}: +buildPythonPackage rec { + pname = "Twisted"; + version = "19.10.0"; + + src = fetchPypi { + inherit pname version; + extension = "tar.bz2"; + sha256 = "7394ba7f272ae722a74f3d969dcf599bc4ef093bc392038748a490f1724a515d"; + }; + + propagatedBuildInputs = [ zope_interface incremental automat constantly hyperlink pyhamcrest attrs setuptools bcrypt ]; + + passthru.extras.tls = [ pyopenssl service-identity idna ]; + + # Patch t.p._inotify to point to libc. Without this, + # twisted.python.runtime.platform.supportsINotify() == False + patchPhase = stdenv.lib.optionalString stdenv.isLinux '' + substituteInPlace src/twisted/python/_inotify.py --replace \ + "ctypes.util.find_library('c')" "'${stdenv.glibc.out}/lib/libc.so.6'" + ''; + + # Generate Twisted's plug-in cache. Twisted users must do it as well. See + # http://twistedmatrix.com/documents/current/core/howto/plugin.html#auto3 + # and http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=477103 for + # details. + postFixup = '' + $out/bin/twistd --help > /dev/null + ''; + + checkPhase = '' + ${python.interpreter} -m unittest discover -s twisted/test + ''; + # Tests require network + doCheck = false; + + meta = with stdenv.lib; { + homepage = https://twistedmatrix.com/; + description = "Twisted, an event-driven networking engine written in Python"; + longDescription = '' + Twisted is an event-driven networking engine written in Python + and licensed under the MIT license. + ''; + license = licenses.mit; + maintainers = [ ]; + }; +} From d727ae4a86baa094e304ea9a4676626b0491f253 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 19 Nov 2020 08:50:44 -0500 Subject: [PATCH 11/16] Try to improve the failure mode --- src/allmydata/test/web/test_webish.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 67bfca69e..90eec40ad 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -247,6 +247,12 @@ class TahoeLAFSSiteTests(SyncTestCase): 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), From 1637769c8126228e504b55e32c2d3e62a403e89e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 19 Nov 2020 09:22:46 -0500 Subject: [PATCH 12/16] It's gonna be an installation change --- newsfragments/{1549.minor => 1549.installation} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{1549.minor => 1549.installation} (100%) diff --git a/newsfragments/1549.minor b/newsfragments/1549.installation similarity index 100% rename from newsfragments/1549.minor rename to newsfragments/1549.installation From ff8906ecb2812d9642a2c85be9c1d5b6160962c5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 19 Nov 2020 09:34:17 -0500 Subject: [PATCH 13/16] Describe the installation requirement change --- newsfragments/1549.installation | 1 + 1 file changed, 1 insertion(+) diff --git a/newsfragments/1549.installation b/newsfragments/1549.installation index e69de29bb..cbb91cea5 100644 --- a/newsfragments/1549.installation +++ b/newsfragments/1549.installation @@ -0,0 +1 @@ +Tahoe-LAFS now requires Twisted 19.10.0 or newer. As a result, it now has a transitive dependency on bcrypt. From 4ce2572ce914590f944aab64eb653ecf49ca1dfc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 19 Nov 2020 09:39:34 -0500 Subject: [PATCH 14/16] Does Windows behave if we restrict ourselves to *just* S_IREAD? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From CPython docs: > Note Although Windows supports chmod(), you can only set the file’s > read-only flag with it (via the stat.S_IWRITE and stat.S_IREAD constants or > a corresponding integer value). All other bits are ignored. --- src/allmydata/test/web/test_webish.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 90eec40ad..a510f26f2 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -244,7 +244,7 @@ class TahoeLAFSSiteTests(SyncTestCase): # 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. - tempdir.chmod(0o550) + tempdir.chmod(0o400) with self.assertRaises(OSError) as ctx: request.gotLength(request_body_size) raise Exception( From 1689804877ca226b1d2058ae99ff0ecc9173698f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 19 Nov 2020 10:15:36 -0500 Subject: [PATCH 15/16] Try doing some other thing in Windows --- src/allmydata/test/web/test_webish.py | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index a510f26f2..a4ccbc7b9 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -26,8 +26,12 @@ from testtools.matchers import ( MatchesAll, Not, IsInstance, + HasLength, ) +from twisted.python.runtime import ( + platform, +) from twisted.python.filepath import ( FilePath, ) @@ -244,19 +248,29 @@ class TahoeLAFSSiteTests(SyncTestCase): # 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. - tempdir.chmod(0o400) - with self.assertRaises(OSError) as ctx: + # + # 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) - raise Exception( - "OSError not raised, instead tempdir.children() = {}".format( - tempdir.children(), - ), + 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), - ) + self.assertThat( + ctx.exception.errno, + Equals(EACCES), + ) def test_unknown_request_size(self): """ From 520f4d15bf8d6c9ebd45c61a06385effef7579de Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 25 Nov 2020 16:09:53 -0500 Subject: [PATCH 16/16] Rename `_get_request` to more accurate `_create_request` --- src/allmydata/test/web/test_webish.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index a4ccbc7b9..e680acd04 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -188,7 +188,7 @@ class TahoeLAFSSiteTests(SyncTestCase): b"/uri?uri=[CENSORED]", ) - def _get_request(self, tempdir): + def _create_request(self, tempdir): """ Create and return a new ``TahoeLAFSRequest`` hooked up to a ``TahoeLAFSSite``. @@ -211,7 +211,7 @@ class TahoeLAFSSiteTests(SyncTestCase): A request body smaller than 1 MiB is kept in memory. """ tempdir = FilePath(self.mktemp()) - request = self._get_request(tempdir) + request = self._create_request(tempdir) request.gotLength(request_body_size) self.assertThat( request.content, @@ -226,7 +226,7 @@ class TahoeLAFSSiteTests(SyncTestCase): """ tempdir = FilePath(self.mktemp()) tempdir.makedirs() - request = self._get_request(tempdir) + 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