From ce50916ec586d381c9016f441f49bef66e9d020f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 20:21:43 -0500 Subject: [PATCH 01/24] Add newsfragment --- newsfragments/3536.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3536.minor diff --git a/newsfragments/3536.minor b/newsfragments/3536.minor new file mode 100644 index 000000000..e69de29bb From 57282d243111043aaa392f8a2edb5daa1c0749ea Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 20:38:44 -0500 Subject: [PATCH 02/24] Include contribution guidelines for real This warning should go away with this commit: WARNING: toctree contains reference to nonexisting document u'.github/CONTRIBUTING' --- docs/contributing.rst | 1 + docs/index.rst | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/contributing.rst diff --git a/docs/contributing.rst b/docs/contributing.rst new file mode 100644 index 000000000..15e1b6432 --- /dev/null +++ b/docs/contributing.rst @@ -0,0 +1 @@ +.. include:: ../.github/CONTRIBUTING.rst diff --git a/docs/index.rst b/docs/index.rst index 3d0a41302..e5db73db0 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -23,7 +23,7 @@ Contents: frontends/download-status known_issues - ../.github/CONTRIBUTING + contributing CODE_OF_CONDUCT servers From 97454242354072619988ead4067df537952b8b08 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 20:44:13 -0500 Subject: [PATCH 03/24] Include release checklist --- docs/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/index.rst b/docs/index.rst index e5db73db0..60a3aa5d4 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -25,6 +25,7 @@ Contents: known_issues contributing CODE_OF_CONDUCT + release-checklist servers helper From d0859b11017a7e049df94b509dda123db06828d0 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 21:04:13 -0500 Subject: [PATCH 04/24] Fix indentation in webapi docs Warning was: tahoe-lafs/docs/frontends/webapi.rst:2035: WARNING: Unexpected indentation. --- docs/frontends/webapi.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/frontends/webapi.rst b/docs/frontends/webapi.rst index 99fa44979..e00445fbf 100644 --- a/docs/frontends/webapi.rst +++ b/docs/frontends/webapi.rst @@ -2032,10 +2032,11 @@ potential for surprises when the file store structure is changed. Tahoe-LAFS provides a mutable file store, but the ways that the store can change are limited. The only things that can change are: - * the mapping from child names to child objects inside mutable directories - (by adding a new child, removing an existing child, or changing an - existing child to point to a different object) - * the contents of mutable files + +* the mapping from child names to child objects inside mutable directories + (by adding a new child, removing an existing child, or changing an + existing child to point to a different object) +* the contents of mutable files Obviously if you query for information about the file store and then act to change it (such as by getting a listing of the contents of a mutable From 4e56cc6d379c35b8a2d479b665b35756279be41b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 21:19:40 -0500 Subject: [PATCH 05/24] Fix nested bullet lists in release checklist doc --- docs/release-checklist.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/release-checklist.rst b/docs/release-checklist.rst index be32aea6c..da692ef61 100644 --- a/docs/release-checklist.rst +++ b/docs/release-checklist.rst @@ -40,23 +40,31 @@ Create Branch and Apply Updates - Create a branch for release-candidates (e.g. `XXXX.release-1.15.0.rc0`) - run `tox -e news` to produce a new NEWS.txt file (this does a commit) - create the news for the release + - newsfragments/.minor - commit it + - manually fix NEWS.txt + - proper title for latest release ("Release 1.15.0" instead of "Release ...post1432") - double-check date (maybe release will be in the future) - spot-check the release notes (these come from the newsfragments files though so don't do heavy editing) - commit these changes + - update "relnotes.txt" + - update all mentions of 1.14.0 -> 1.15.0 - update "previous release" statement and date - summarize major changes - commit it + - update "CREDITS" + - are there any new contributors in this release? - one way: git log release-1.14.0.. | grep Author | sort | uniq - commit it + - update "docs/known_issues.rst" if appropriate - update "docs/INSTALL.rst" references to the new release - Push the branch to github @@ -82,21 +90,32 @@ they will need to evaluate which contributors' signatures they trust. - (all steps above are completed) - sign the release + - git tag -s -u 0xE34E62D06D0E69CFCA4179FFBDE0D31D68666A7A -m "release Tahoe-LAFS-1.15.0rc0" tahoe-lafs-1.15.0rc0 - (replace the key-id above with your own) + - build all code locally - these should all pass: + - tox -e py27,codechecks,docs,integration + - these can fail (ideally they should not of course): + - tox -e deprecations,upcoming-deprecations + - build tarballs + - tox -e tarballs - confirm it at least exists: - ls dist/ | grep 1.15.0rc0 + - inspect and test the tarballs + - install each in a fresh virtualenv - run `tahoe` command + - when satisfied, sign the tarballs: + - gpg --pinentry=loopback --armor --sign dist/tahoe_lafs-1.15.0rc0-py2-none-any.whl - gpg --pinentry=loopback --armor --sign dist/tahoe_lafs-1.15.0rc0.tar.bz2 - gpg --pinentry=loopback --armor --sign dist/tahoe_lafs-1.15.0rc0.tar.gz @@ -129,6 +148,7 @@ need to be uploaded to https://tahoe-lafs.org in `~source/downloads` https://tahoe-lafs.org/downloads/ on the Web. - scp dist/*1.15.0* username@tahoe-lafs.org:/home/source/downloads - the following developers have access to do this: + - exarkun - meejah - warner @@ -139,6 +159,7 @@ uploaded to PyPI as well. - how to do this? - (original guide says only "twine upload dist/*") - the following developers have access to do this: + - warner - exarkun (partial?) - meejah (partial?) From 1063ee1c1f222276341b4596161455f4f9a7668b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 4 Jan 2021 21:21:43 -0500 Subject: [PATCH 06/24] Fix warning in release checklist doc Fix "WARNING: Inline emphasis start-string without end-string." --- docs/release-checklist.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-checklist.rst b/docs/release-checklist.rst index da692ef61..fabb69fc7 100644 --- a/docs/release-checklist.rst +++ b/docs/release-checklist.rst @@ -157,7 +157,7 @@ For the actual release, the tarball and signature files need to be uploaded to PyPI as well. - how to do this? -- (original guide says only "twine upload dist/*") +- (original guide says only `twine upload dist/*`) - the following developers have access to do this: - warner From f1cf9483566ae719d605df7754630e68fdb6f77f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 8 Jan 2021 13:45:16 -0500 Subject: [PATCH 07/24] Enable markdown support in Sphinx configuration The upstream Contributor Covenant is a markdown document. Since we prefer to keep things close to the original, enabling markdown support to render that document seems like a good idea. --- docs/conf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 34ddd1bd4..612c324a3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -28,7 +28,7 @@ import os # Add any Sphinx extension module names here, as strings. They can be # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. -extensions = [] +extensions = ['recommonmark'] # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -36,7 +36,7 @@ templates_path = ['_templates'] # The suffix(es) of source filenames. # You can specify multiple suffix as a list of string: # source_suffix = ['.rst', '.md'] -source_suffix = '.rst' +source_suffix = ['.rst', '.md'] # The encoding of source files. #source_encoding = 'utf-8-sig' From 65926d6e708b967759b1ad50999bb84ff05c13d2 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 8 Jan 2021 13:48:04 -0500 Subject: [PATCH 08/24] Install recommonmark in tox "docs" environment We're going to need markdown support to render contributor covenant. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index c61331885..d32f90b15 100644 --- a/tox.ini +++ b/tox.ini @@ -211,6 +211,7 @@ commands = deps = sphinx docutils==0.12 + recommonmark # normal install is not needed for docs, and slows things down skip_install = True commands = From f682d946d0ffffaf1f05d1b9157a83faea84ee5d Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 8 Jan 2021 13:48:23 -0500 Subject: [PATCH 09/24] Rename docs/README.md After enabling markdown extension in Sphinx configuration, there's a warning about docs/README.md not being included in the toc tree. Since docs/README.md is not to be included in the final rendered document, we'll keep it as a .txt document. --- docs/{README.md => README.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{README.md => README.txt} (100%) diff --git a/docs/README.md b/docs/README.txt similarity index 100% rename from docs/README.md rename to docs/README.txt From afcae42fd6d3f9df21cee943aeb1a65140d7013a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 15 Jan 2021 11:52:16 -0500 Subject: [PATCH 10/24] Notice that there's an error on the server, rather than continuing silently. --- integration/test_web.py | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/test_web.py b/integration/test_web.py index fe2137ff3..a322129f8 100644 --- a/integration/test_web.py +++ b/integration/test_web.py @@ -133,6 +133,7 @@ def test_deep_stats(alice): u"file": FILE_CONTENTS, }, ) + resp.raise_for_status() # confirm the file is in the directory resp = requests.get( From 3166545509867c158b822794dfc26d716ee8224a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 19 Jan 2021 13:52:12 -0500 Subject: [PATCH 11/24] Unit test reproducing the bug in the integration test. --- src/allmydata/test/web/test_web.py | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index ce9a40389..c7d866ad1 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -4757,6 +4757,39 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi op_url = self.webish_url + "/operations/134?t=status&output=JSON" yield self.assertHTTPError(op_url, 404, "unknown/expired handle '134'") + @inlineCallbacks + def test_upload_file_in_directory(self): + """Create a directory, then upload a file into it. + + Unit test reproducer for https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3590 + """ + def req(method, path, **kwargs): + return treq.request(method, self.webish_url + path, persistent=False, + **kwargs) + + response = yield req("POST", "/uri?format=sdmf&t=mkdir&redirect_to_result=true", + browser_like_redirects=True) + + uri = urllib.unquote(response.request.absoluteURI) + assert 'URI:DIR2:' in uri + dircap = uri[uri.find("URI:DIR2:"):].rstrip('/') + dircap_uri = "/uri/{}".format(urllib.quote(dircap)) + + # POST a file into this directory + FILE_CONTENTS = u"a file in a directory" + + body, headers = self.build_form(t="upload", when_done=".", name="file", + file=FILE_CONTENTS) + response = yield req( + "POST", + dircap_uri, + data=body, + headers=headers, + browser_like_redirects=True + ) + if response.code >= 400: + raise Error(response.code, response=response.content()) + def test_incident(self): d = self.POST("/report_incident", details="eek") def _done(res): From 6979cfa205ee13da0358dd92d0971d78c09ffefc Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 19 Jan 2021 14:28:00 -0500 Subject: [PATCH 12/24] Fix the redirect 'str has no render' bug. --- src/allmydata/web/root.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 0ef6b00d2..2e82e94ec 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -11,7 +11,7 @@ from twisted.web import ( resource, static, ) -from twisted.web.util import redirectTo +from twisted.web.util import redirectTo, Redirect from twisted.python.filepath import FilePath from twisted.web.template import ( Element, @@ -155,7 +155,7 @@ class URIHandler(resource.Resource, object): u = u.replace( path=(s for s in u.path if s), # remove empty segments ) - return redirectTo(u.to_uri().to_text().encode('utf8'), req) + return Redirect(u.to_uri().to_text().encode('utf8')) try: node = self.client.create_node_from_uri(name) return directory.make_handler_for(node, self.client) From e91d37e64b50579bd75edf23dd767161b699611d Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 19 Jan 2021 17:13:52 -0500 Subject: [PATCH 13/24] Fix unit test so it's actually testing the real bug. --- src/allmydata/test/web/test_web.py | 22 ++++++++-------------- src/allmydata/web/root.py | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index c7d866ad1..8dcb7d95b 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -4758,8 +4758,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi yield self.assertHTTPError(op_url, 404, "unknown/expired handle '134'") @inlineCallbacks - def test_upload_file_in_directory(self): - """Create a directory, then upload a file into it. + def test_uri_redirect(self): + """URI redirects don't cause failure. Unit test reproducer for https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3590 """ @@ -4767,26 +4767,20 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return treq.request(method, self.webish_url + path, persistent=False, **kwargs) - response = yield req("POST", "/uri?format=sdmf&t=mkdir&redirect_to_result=true", - browser_like_redirects=True) + response = yield req("POST", "/uri?format=sdmf&t=mkdir&redirect_to_result=true") uri = urllib.unquote(response.request.absoluteURI) assert 'URI:DIR2:' in uri dircap = uri[uri.find("URI:DIR2:"):].rstrip('/') - dircap_uri = "/uri/{}".format(urllib.quote(dircap)) + dircap_uri = "/uri/?uri={}&t=json".format(urllib.quote(dircap)) - # POST a file into this directory - FILE_CONTENTS = u"a file in a directory" - - body, headers = self.build_form(t="upload", when_done=".", name="file", - file=FILE_CONTENTS) response = yield req( - "POST", + "GET", dircap_uri, - data=body, - headers=headers, - browser_like_redirects=True ) + self.assertEqual( + response.request.absoluteURI, + self.webish_url + "/uri/{}?t=json".format(urllib.quote(dircap))) if response.code >= 400: raise Error(response.code, response=response.content()) diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 2e82e94ec..9fb3ac9d3 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -147,7 +147,7 @@ class URIHandler(resource.Resource, object): and creates and appropriate handler (depending on the kind of capability it was passed). """ - # this is in case a URI like "/uri/?cap=" is + # this is in case a URI like "/uri/?uri=" is # passed -- we re-direct to the non-trailing-slash version so # that there is just one valid URI for "uri" resource. if not name: From 7d2aa50894bb59043b5e722425d71726f871eb4c Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 19 Jan 2021 17:15:07 -0500 Subject: [PATCH 14/24] when_done is bad, at least here. --- integration/test_web.py | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/test_web.py b/integration/test_web.py index a322129f8..216d80d42 100644 --- a/integration/test_web.py +++ b/integration/test_web.py @@ -127,7 +127,6 @@ def test_deep_stats(alice): dircap_uri, data={ u"t": u"upload", - u"when_done": u".", }, files={ u"file": FILE_CONTENTS, From 53243540cc9ad5310dd9d97de0ca7e1afeed1068 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 19 Jan 2021 17:15:58 -0500 Subject: [PATCH 15/24] News file. --- 3590.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 3590.bugfix diff --git a/3590.bugfix b/3590.bugfix new file mode 100644 index 000000000..aa504a5e3 --- /dev/null +++ b/3590.bugfix @@ -0,0 +1 @@ +Fixed issue where redirecting old-style URIs (/uri/?uri=...) didn't work. \ No newline at end of file From 8be3678cb47f0902a94d2ed1b1d651842b738efd Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 20 Jan 2021 11:22:22 -0500 Subject: [PATCH 16/24] Directly test read_encrypted behavior --- src/allmydata/test/test_upload.py | 69 +++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/allmydata/test/test_upload.py b/src/allmydata/test/test_upload.py index 94d7575c3..7e41bfc24 100644 --- a/src/allmydata/test/test_upload.py +++ b/src/allmydata/test/test_upload.py @@ -14,6 +14,17 @@ if PY2: import os, shutil from io import BytesIO +from base64 import ( + b64encode, +) + +from hypothesis import ( + given, +) +from hypothesis.strategies import ( + just, + integers, +) from twisted.trial import unittest from twisted.python.failure import Failure @@ -2029,6 +2040,64 @@ class EncodingParameters(GridTestMixin, unittest.TestCase, SetDEPMixin, f.close() return None + +class EncryptAnUploadableTests(unittest.TestCase): + """ + Tests for ``EncryptAnUploadable``. + """ + def test_same_length(self): + """ + ``EncryptAnUploadable.read_encrypted`` returns ciphertext of the same + length as the underlying plaintext. + """ + plaintext = b"hello world" + uploadable = upload.FileHandle(BytesIO(plaintext), None) + uploadable.set_default_encoding_parameters({ + # These values shouldn't matter. + "k": 3, + "happy": 5, + "n": 10, + "max_segment_size": 128 * 1024, + }) + encrypter = upload.EncryptAnUploadable(uploadable) + ciphertext = b"".join(self.successResultOf(encrypter.read_encrypted(1024, False))) + self.assertEqual(len(ciphertext), len(plaintext)) + + @given(just(b"hello world"), integers(min_value=0, max_value=len(b"hello world"))) + def test_known_result(self, plaintext, split_at): + """ + ``EncryptAnUploadable.read_encrypted`` returns a known-correct ciphertext + string for certain inputs. The ciphertext is independent of the read + sizes. + """ + convergence = b"\x42" * 16 + uploadable = upload.FileHandle(BytesIO(plaintext), convergence) + uploadable.set_default_encoding_parameters({ + # The convergence key is a function of k, n, and max_segment_size + # (among other things). The value for happy doesn't matter + # though. + "k": 3, + "happy": 5, + "n": 10, + "max_segment_size": 128 * 1024, + }) + encrypter = upload.EncryptAnUploadable(uploadable) + def read(n): + return b"".join(self.successResultOf(encrypter.read_encrypted(n, False))) + + # Read the string in one or two pieces to make sure underlying state + # is maintained properly. + first = read(split_at) + second = read(len(plaintext) - split_at) + third = read(1) + ciphertext = first + second + third + + self.assertEqual( + b"Jd2LHCRXozwrEJc=", + b64encode(ciphertext), + ) + + # TODO: # upload with exactly 75 servers (shares_of_happiness) # have a download fail From f75f71cba6e98ac508cf06108523a9d0c1a4842f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 20 Jan 2021 11:23:35 -0500 Subject: [PATCH 17/24] news fragment --- newsfragments/3594.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3594.minor diff --git a/newsfragments/3594.minor b/newsfragments/3594.minor new file mode 100644 index 000000000..e69de29bb From 932481ad47c650cb00070394829a7cc268fdd00e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 20 Jan 2021 12:58:03 -0500 Subject: [PATCH 18/24] A helper for doing something repeatedly for a while --- src/allmydata/test/test_deferredutil.py | 55 +++++++++++++++++++++++++ src/allmydata/util/deferredutil.py | 30 ++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/src/allmydata/test/test_deferredutil.py b/src/allmydata/test/test_deferredutil.py index 6ebc93556..2a155089f 100644 --- a/src/allmydata/test/test_deferredutil.py +++ b/src/allmydata/test/test_deferredutil.py @@ -74,3 +74,58 @@ class DeferredUtilTests(unittest.TestCase, deferredutil.WaitForDelayedCallsMixin d = defer.succeed(None) d.addBoth(self.wait_for_delayed_calls) return d + + +class UntilTests(unittest.TestCase): + """ + Tests for ``deferredutil.until``. + """ + def test_exception(self): + """ + If the action raises an exception, the ``Deferred`` returned by ``until`` + fires with a ``Failure``. + """ + self.assertFailure( + deferredutil.until(lambda: 1/0, lambda: True), + ZeroDivisionError, + ) + + def test_stops_on_condition(self): + """ + The action is called repeatedly until ``condition`` returns ``True``. + """ + calls = [] + def action(): + calls.append(None) + + def condition(): + return len(calls) == 3 + + self.assertIs( + self.successResultOf( + deferredutil.until(action, condition), + ), + None, + ) + self.assertEqual(3, len(calls)) + + def test_waits_for_deferred(self): + """ + If the action returns a ``Deferred`` then it is called again when the + ``Deferred`` fires. + """ + counter = [0] + r1 = defer.Deferred() + r2 = defer.Deferred() + results = [r1, r2] + def action(): + counter[0] += 1 + return results.pop(0) + + def condition(): + return False + + deferredutil.until(action, condition) + self.assertEqual([1], counter) + r1.callback(None) + self.assertEqual([2], counter) diff --git a/src/allmydata/util/deferredutil.py b/src/allmydata/util/deferredutil.py index 1d13f61e6..ed2a11ee4 100644 --- a/src/allmydata/util/deferredutil.py +++ b/src/allmydata/util/deferredutil.py @@ -15,7 +15,18 @@ if PY2: import time +try: + from typing import ( + Callable, + Any, + ) +except ImportError: + pass + from foolscap.api import eventually +from eliot.twisted import ( + inline_callbacks, +) from twisted.internet import defer, reactor, error from twisted.python.failure import Failure @@ -201,3 +212,22 @@ class WaitForDelayedCallsMixin(PollMixin): d.addErrback(log.err, "error while waiting for delayed calls") d.addBoth(lambda ign: res) return d + +@inline_callbacks +def until( + action, # type: Callable[[], defer.Deferred[Any]] + condition, # type: Callable[[], bool] +): + # type: (...) -> defer.Deferred[None] + """ + Run a Deferred-returning function until a condition is true. + + :param action: The action to run. + :param condition: The predicate signaling stop. + + :return: A Deferred that fires after the condition signals stop. + """ + while True: + yield action() + if condition(): + break From 12087738d682c051aeeb75a7c90dd78d7b8ebfb0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 20 Jan 2021 13:54:37 -0500 Subject: [PATCH 19/24] Switch from fireEventually to `until` --- src/allmydata/immutable/upload.py | 102 +++++++++++++++++++++--------- src/allmydata/test/test_upload.py | 27 ++++++++ 2 files changed, 98 insertions(+), 31 deletions(-) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index adcdaed10..fe173b46c 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -13,19 +13,33 @@ if PY2: from past.builtins import long, unicode from six import ensure_str +try: + from typing import List +except ImportError: + pass + import os, time, weakref, itertools +from functools import ( + partial, +) + +import attr + from zope.interface import implementer from twisted.python import failure from twisted.internet import defer from twisted.application import service -from foolscap.api import Referenceable, Copyable, RemoteCopy, fireEventually +from foolscap.api import Referenceable, Copyable, RemoteCopy from allmydata.crypto import aes from allmydata.util.hashutil import file_renewal_secret_hash, \ file_cancel_secret_hash, bucket_renewal_secret_hash, \ bucket_cancel_secret_hash, plaintext_hasher, \ storage_index_hash, plaintext_segment_hasher, convergence_hasher -from allmydata.util.deferredutil import timeout_call +from allmydata.util.deferredutil import ( + timeout_call, + until, +) from allmydata import hashtree, uri from allmydata.storage.server import si_b2a from allmydata.immutable import encode @@ -900,13 +914,41 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): raise UploadUnhappinessError(msg) +@attr.s +class _Accum(object): + """ + Accumulate up to some known amount of ciphertext. + + :ivar remaining: The number of bytes still expected. + :ivar ciphertext: The bytes accumulated so far. + """ + remaining = attr.ib(validator=attr.validators.instance_of(int)) # type: int + ciphertext = attr.ib(default=attr.Factory(list)) # type: List[bytes] + + def extend(self, + size, # type: int + ciphertext, # type: List[bytes] + ): + """ + Accumulate some more ciphertext. + + :param size: The amount of data the new ciphertext represents towards + the goal. This may be more than the actual size of the given + ciphertext if the source has run out of data. + + :param ciphertext: The new ciphertext to accumulate. + """ + self.remaining -= size + self.ciphertext.extend(ciphertext) + + @implementer(IEncryptedUploadable) class EncryptAnUploadable(object): """This is a wrapper that takes an IUploadable and provides IEncryptedUploadable.""" CHUNKSIZE = 50*1024 - def __init__(self, original, log_parent=None, progress=None): + def __init__(self, original, log_parent=None, progress=None, chunk_size=None): precondition(original.default_params_set, "set_default_encoding_parameters not called on %r before wrapping with EncryptAnUploadable" % (original,)) self.original = IUploadable(original) @@ -920,6 +962,8 @@ class EncryptAnUploadable(object): self._ciphertext_bytes_read = 0 self._status = None self._progress = progress + if chunk_size is not None: + self.CHUNKSIZE = chunk_size def set_upload_status(self, upload_status): self._status = IUploadStatus(upload_status) @@ -1026,47 +1070,43 @@ class EncryptAnUploadable(object): # and size d.addCallback(lambda ignored: self.get_size()) d.addCallback(lambda ignored: self._get_encryptor()) - # then fetch and encrypt the plaintext. The unusual structure here - # (passing a Deferred *into* a function) is needed to avoid - # overflowing the stack: Deferreds don't optimize out tail recursion. - # We also pass in a list, to which _read_encrypted will append - # ciphertext. - ciphertext = [] - d2 = defer.Deferred() - d.addCallback(lambda ignored: - self._read_encrypted(length, ciphertext, hash_only, d2)) - d.addCallback(lambda ignored: d2) + + accum = _Accum(length) + action = partial(self._read_encrypted, accum, hash_only) + condition = lambda: accum.remaining == 0 + + d.addCallback(lambda ignored: until(action, condition)) + d.addCallback(lambda ignored: accum.ciphertext) return d - def _read_encrypted(self, remaining, ciphertext, hash_only, fire_when_done): - if not remaining: - fire_when_done.callback(ciphertext) - return None + def _read_encrypted(self, + ciphertext_accum, # type: _Accum + hash_only, # type: bool + ): + # type: (...) -> defer.Deferred + """ + Read the next chunk of plaintext, encrypt it, and extend the accumulator + with the resulting ciphertext. + """ # tolerate large length= values without consuming a lot of RAM by # reading just a chunk (say 50kB) at a time. This only really matters # when hash_only==True (i.e. resuming an interrupted upload), since # that's the case where we will be skipping over a lot of data. - size = min(remaining, self.CHUNKSIZE) - remaining = remaining - size + size = min(ciphertext_accum.remaining, self.CHUNKSIZE) + # read a chunk of plaintext.. d = defer.maybeDeferred(self.original.read, size) - # N.B.: if read() is synchronous, then since everything else is - # actually synchronous too, we'd blow the stack unless we stall for a - # tick. Once you accept a Deferred from IUploadable.read(), you must - # be prepared to have it fire immediately too. - d.addCallback(fireEventually) def _good(plaintext): # and encrypt it.. # o/' over the fields we go, hashing all the way, sHA! sHA! sHA! o/' ct = self._hash_and_encrypt_plaintext(plaintext, hash_only) - ciphertext.extend(ct) - self._read_encrypted(remaining, ciphertext, hash_only, - fire_when_done) - def _err(why): - fire_when_done.errback(why) + # Intentionally tell the accumulator about the expected size, not + # the actual size. If we run out of data we still want remaining + # to drop otherwise it will never reach 0 and the loop will never + # end. + ciphertext_accum.extend(size, ct) d.addCallback(_good) - d.addErrback(_err) - return None + return d def _hash_and_encrypt_plaintext(self, data, hash_only): assert isinstance(data, (tuple, list)), type(data) diff --git a/src/allmydata/test/test_upload.py b/src/allmydata/test/test_upload.py index 7e41bfc24..07ede2074 100644 --- a/src/allmydata/test/test_upload.py +++ b/src/allmydata/test/test_upload.py @@ -2097,6 +2097,33 @@ class EncryptAnUploadableTests(unittest.TestCase): b64encode(ciphertext), ) + def test_large_read(self): + """ + ``EncryptAnUploadable.read_encrypted`` succeeds even when the requested + data length is much larger than the chunk size. + """ + convergence = b"\x42" * 16 + # 4kB of plaintext + plaintext = b"\xde\xad\xbe\xef" * 1024 + uploadable = upload.FileHandle(BytesIO(plaintext), convergence) + uploadable.set_default_encoding_parameters({ + "k": 3, + "happy": 5, + "n": 10, + "max_segment_size": 128 * 1024, + }) + # Make the chunk size very small so we don't have to operate on a huge + # amount of data to exercise the relevant codepath. + encrypter = upload.EncryptAnUploadable(uploadable, chunk_size=1) + d = encrypter.read_encrypted(len(plaintext), False) + ciphertext = self.successResultOf(d) + self.assertEqual( + list(map(len, ciphertext)), + # Chunk size was specified as 1 above so we will get the whole + # plaintext in one byte chunks. + [1] * len(plaintext), + ) + # TODO: # upload with exactly 75 servers (shares_of_happiness) From 9c91261fa6675e2f92890c9cd1229474e49883db Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 20 Jan 2021 13:57:01 -0500 Subject: [PATCH 20/24] news fragment --- newsfragments/3595.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3595.minor diff --git a/newsfragments/3595.minor b/newsfragments/3595.minor new file mode 100644 index 000000000..e69de29bb From 23e52b12377e921bd5cf3b1d15b413cde24aedd3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 21 Jan 2021 09:58:58 -0500 Subject: [PATCH 21/24] Simplify the unit test. --- 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 8dcb7d95b..2f000b7a1 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 return treq.request(method, self.webish_url + path, persistent=False, **kwargs) - response = yield req("POST", "/uri?format=sdmf&t=mkdir&redirect_to_result=true") - - uri = urllib.unquote(response.request.absoluteURI) - assert 'URI:DIR2:' in uri - dircap = uri[uri.find("URI:DIR2:"):].rstrip('/') + response = yield req("POST", "/uri?format=sdmf&t=mkdir") + dircap = yield response.content() + assert dircap.startswith('URI:DIR2:') dircap_uri = "/uri/?uri={}&t=json".format(urllib.quote(dircap)) response = yield req( From 411ee141e98fc2527797bb461a5c08440b479daf Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 21 Jan 2021 13:55:51 -0500 Subject: [PATCH 22/24] Fix location for news fragment. --- 3590.bugfix => newsfragments/3590.bugfix | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename 3590.bugfix => newsfragments/3590.bugfix (100%) diff --git a/3590.bugfix b/newsfragments/3590.bugfix similarity index 100% rename from 3590.bugfix rename to newsfragments/3590.bugfix From 5a0c913f589de968e7543ba7175386454b509be3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 25 Jan 2021 08:21:39 -0500 Subject: [PATCH 23/24] document the new parameter --- src/allmydata/immutable/upload.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index fe173b46c..27cc923fd 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -949,6 +949,10 @@ class EncryptAnUploadable(object): CHUNKSIZE = 50*1024 def __init__(self, original, log_parent=None, progress=None, chunk_size=None): + """ + :param chunk_size: The number of bytes to read from the uploadable at a + time, or None for some default. + """ precondition(original.default_params_set, "set_default_encoding_parameters not called on %r before wrapping with EncryptAnUploadable" % (original,)) self.original = IUploadable(original) From e0fa2286228a46cd81b82868b56cf096d18d95dc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 25 Jan 2021 08:23:40 -0500 Subject: [PATCH 24/24] expand partial/lambda into full functions for clarity --- src/allmydata/immutable/upload.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index 27cc923fd..46e01184f 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -19,9 +19,6 @@ except ImportError: pass import os, time, weakref, itertools -from functools import ( - partial, -) import attr @@ -1076,8 +1073,18 @@ class EncryptAnUploadable(object): d.addCallback(lambda ignored: self._get_encryptor()) accum = _Accum(length) - action = partial(self._read_encrypted, accum, hash_only) - condition = lambda: accum.remaining == 0 + + def action(): + """ + Read some bytes into the accumulator. + """ + return self._read_encrypted(accum, hash_only) + + def condition(): + """ + Check to see if the accumulator has all the data. + """ + return accum.remaining == 0 d.addCallback(lambda ignored: until(action, condition)) d.addCallback(lambda ignored: accum.ciphertext)