From 3bc015ad81c8127586bc13ca01f35ed75e94a07b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 10 Feb 2020 15:24:17 -0500 Subject: [PATCH 01/61] news fragment --- newsfragments/3283.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3283.minor diff --git a/newsfragments/3283.minor b/newsfragments/3283.minor new file mode 100644 index 000000000..e69de29bb From 2daf0fe6127b7622b05e75a7299fad4d17a5ef96 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 10 Feb 2020 15:26:12 -0500 Subject: [PATCH 02/61] Add myself --- docs/backdoors.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/backdoors.rst b/docs/backdoors.rst index 97716fcad..102642cdc 100644 --- a/docs/backdoors.rst +++ b/docs/backdoors.rst @@ -64,3 +64,5 @@ Peter Secor Shawn Willden Terrell Russell + +Jean-Paul Calderone From 406261b9921be7a59d8189c91c7b254516984594 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 10 Feb 2020 13:47:43 -0700 Subject: [PATCH 03/61] meejah --- docs/backdoors.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/backdoors.rst b/docs/backdoors.rst index 102642cdc..bc30c39f8 100644 --- a/docs/backdoors.rst +++ b/docs/backdoors.rst @@ -66,3 +66,6 @@ Shawn Willden Terrell Russell Jean-Paul Calderone + +meejah + From 80e3f709e72ed0f99e1a2f9cecdb023e099d9c9f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 10 Feb 2020 16:53:35 -0500 Subject: [PATCH 04/61] Sign backdoor statement Related to ticket:3283. --- docs/backdoors.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/backdoors.rst b/docs/backdoors.rst index bc30c39f8..0fec9efbc 100644 --- a/docs/backdoors.rst +++ b/docs/backdoors.rst @@ -69,3 +69,4 @@ Jean-Paul Calderone meejah +Sajith Sasidharan From 574c63d3505f66fd49497d3bf05490d45fc0aa57 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 12 Oct 2020 07:25:44 -0400 Subject: [PATCH 05/61] Port immutable.offloaded to Python 3 --- newsfragments/3466.minor | 0 src/allmydata/immutable/offloaded.py | 15 +++++++++++++-- src/allmydata/test/test_helper.py | 3 +++ src/allmydata/util/_python3.py | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 newsfragments/3466.minor diff --git a/newsfragments/3466.minor b/newsfragments/3466.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index fb8c706a3..652e68b2f 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -1,3 +1,14 @@ +""" +Ported to Python 3. +""" +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +from future.utils import PY2 +if PY2: + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import os, stat, time, weakref from zope.interface import implementer @@ -128,9 +139,9 @@ class CHKUploadHelper(Referenceable, upload.CHKUploader): peer selection, encoding, and share pushing. I read ciphertext from the remote AssistedUploader. """ - VERSION = { "http://allmydata.org/tahoe/protocols/helper/chk-upload/v1" : + VERSION = { b"http://allmydata.org/tahoe/protocols/helper/chk-upload/v1" : { }, - "application-version": str(allmydata.__full_version__), + b"application-version": allmydata.__full_version__.encode("utf-8"), } def __init__(self, storage_index, diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index c47da3277..0e628c81b 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -1,3 +1,6 @@ +""" +Ported to Python 3. +""" from __future__ import absolute_import from __future__ import division from __future__ import print_function diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index 8c2f0ebed..4002ec6e6 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -46,6 +46,7 @@ PORTED_MODULES = [ "allmydata.immutable.happiness_upload", "allmydata.immutable.layout", "allmydata.immutable.literal", + "allmydata.immutable.offloaded", "allmydata.immutable.upload", "allmydata.interfaces", "allmydata.introducer.interfaces", From 0eb0d6888695b76f4a17f796fde9a585fcb9a0e0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 09:40:20 -0400 Subject: [PATCH 06/61] news fragment --- newsfragments/3468.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3468.minor diff --git a/newsfragments/3468.minor b/newsfragments/3468.minor new file mode 100644 index 000000000..e69de29bb From c88c2846e234a4236f0383ce7555f52cf2a45630 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 09:40:26 -0400 Subject: [PATCH 07/61] Point folks at docs for this --- src/allmydata/immutable/offloaded.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index fb8c706a3..ceb2e520b 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -569,6 +569,9 @@ class Helper(Referenceable): return self.VERSION def remote_upload_chk(self, storage_index): + """ + See ``RIHelper.upload_chk`` + """ self.count("chk_upload_helper.upload_requests") lp = self.log(format="helper: upload_chk query for SI %(si)s", si=si_b2a(storage_index)) From 8eb518a221c276c8c18f4e54da940a82047d61b9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 10:23:46 -0400 Subject: [PATCH 08/61] docstring for Helper._active_uploads --- src/allmydata/immutable/offloaded.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index ceb2e520b..784b7e090 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -485,6 +485,11 @@ class LocalCiphertextReader(AskUntilSuccessMixin): @implementer(interfaces.RIHelper, interfaces.IStatsProducer) class Helper(Referenceable): + """ + :ivar dict[bytes, CHKUploadHelper] _active_uploads: For any uploads which + have been started but not finished, a mapping from storage index to the + upload helper. + """ # this is the non-distributed version. When we need to have multiple # helpers, this object will become the HelperCoordinator, and will query # the farm of Helpers to see if anyone has the storage_index of interest, From ef11eeb4a2818c918d1bee599aa04f4518056ded Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 10:24:00 -0400 Subject: [PATCH 09/61] exercise concurrent upload --- src/allmydata/test/test_helper.py | 89 ++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 12 deletions(-) diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index c47da3277..016044bbf 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -14,6 +14,10 @@ from twisted.application import service from foolscap.api import Tub, fireEventually, flushEventualQueue +from eliot.twisted import ( + inline_callbacks, +) + from allmydata.crypto import aes from allmydata.storage.server import si_b2a from allmydata.storage_client import StorageFarmBroker @@ -126,6 +130,28 @@ def upload_data(uploader, data, convergence): u = upload.Data(data, convergence=convergence) return uploader.upload(u) + +def make_uploader(helper_furl, parent, override_name=None): + """ + Make an ``upload.Uploader`` service pointed at the given helper and with + the given service parent. + + :param bytes helper_furl: The Foolscap URL of the upload helper. + + :param IServiceCollection parent: A parent to assign to the new uploader. + + :param str override_name: If not ``None``, a new name for the uploader + service. Multiple services cannot coexist with the same name. + """ + if not isinstance(helper_furl, bytes): + raise TypeError("helper_furl must be bytes, got {!r} instead".format(helper_furl)) + u = upload.Uploader(helper_furl) + if override_name is not None: + u.name = override_name + u.setServiceParent(parent) + return u + + class AssistedUpload(unittest.TestCase): def setUp(self): self.tub = t = Tub() @@ -159,34 +185,75 @@ class AssistedUpload(unittest.TestCase): d.addBoth(flush_but_dont_ignore) return d - def test_one(self): + """ + Some data that has never been uploaded before can be uploaded in CHK + format using the ``RIHelper`` provider and ``Uploader.upload``. + """ self.basedir = "helper/AssistedUpload/test_one" self.setUpHelper(self.basedir) - u = upload.Uploader(self.helper_furl) - u.setServiceParent(self.s) + u = make_uploader(self.helper_furl, self.s) d = wait_a_few_turns() def _ready(res): - assert u._helper - + self.assertTrue( + u._helper, + "Expected uploader to have a helper reference, had {} instead.".format( + u._helper, + ), + ) return upload_data(u, DATA, convergence=b"some convergence string") d.addCallback(_ready) + def _uploaded(results): the_uri = results.get_uri() - assert b"CHK" in the_uri + self.assertIn(b"CHK", the_uri) d.addCallback(_uploaded) def _check_empty(res): + # Make sure the intermediate artifacts aren't left lying around. files = os.listdir(os.path.join(self.basedir, "CHK_encoding")) - self.failUnlessEqual(files, []) + self.assertEqual(files, []) files = os.listdir(os.path.join(self.basedir, "CHK_incoming")) - self.failUnlessEqual(files, []) + self.assertEqual(files, []) d.addCallback(_check_empty) return d + @inline_callbacks + def test_concurrent(self): + """ + The same data can be uploaded by more than one ``Uploader`` at a time. + """ + self.basedir = "helper/AssistedUpload/test_concurrent" + self.setUpHelper(self.basedir) + u1 = make_uploader(self.helper_furl, self.s, "u1") + u2 = make_uploader(self.helper_furl, self.s, "u2") + + yield wait_a_few_turns() + + for u in [u1, u2]: + self.assertTrue( + u._helper, + "Expected uploader to have a helper reference, had {} instead.".format( + u._helper, + ), + ) + + uploads = list( + upload_data(u, DATA, convergence=b"some convergence string") + for u + in [u1, u2] + ) + + result1, result2 = yield defer.gatherResults(uploads) + + self.assertEqual( + result1.get_uri(), + result2.get_uri(), + ) + def test_previous_upload_failed(self): self.basedir = "helper/AssistedUpload/test_previous_upload_failed" self.setUpHelper(self.basedir) @@ -214,8 +281,7 @@ class AssistedUpload(unittest.TestCase): f.write(aes.encrypt_data(encryptor, DATA)) f.close() - u = upload.Uploader(self.helper_furl) - u.setServiceParent(self.s) + u = make_uploader(self.helper_furl, self.s) d = wait_a_few_turns() @@ -240,8 +306,7 @@ class AssistedUpload(unittest.TestCase): def test_already_uploaded(self): self.basedir = "helper/AssistedUpload/test_already_uploaded" self.setUpHelper(self.basedir, helper_class=Helper_already_uploaded) - u = upload.Uploader(self.helper_furl) - u.setServiceParent(self.s) + u = make_uploader(self.helper_furl, self.s) d = wait_a_few_turns() From 4e46f9ae12d466535d38637551fa0a8b61d9677c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 10:57:59 -0400 Subject: [PATCH 10/61] expand the already-uploaded test for the helper to cover more shrink the amount of fake code being used and increase the amount of real code being used --- src/allmydata/immutable/offloaded.py | 15 +++- src/allmydata/test/test_helper.py | 115 ++++++++++++++++++--------- 2 files changed, 90 insertions(+), 40 deletions(-) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index 784b7e090..0802a12fb 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -25,10 +25,11 @@ class CHKCheckerAndUEBFetcher(object): less than 'N' shares present. If the file is completely healthy, I return a tuple of (sharemap, - UEB_data, UEB_hash). + UEB_data, UEB_hash). A sharemap is a dict with share numbers as keys and + sets of server ids (which hold that share) as values. """ - def __init__(self, peer_getter, storage_index, logparent=None): + def __init__(self, peer_getter, storage_index, logparent): self._peer_getter = peer_getter self._found_shares = set() self._storage_index = storage_index @@ -46,6 +47,12 @@ class CHKCheckerAndUEBFetcher(object): return log.msg(*args, **kwargs) def check(self): + """ + :return Deferred[bool|(DictOfSets, dict, bytes)]: If no share can be found + with a usable UEB block or fewer than N shares can be found then the + Deferred fires with ``False``. Otherwise, it fires with a tuple of + the sharemap, the UEB data, and the UEB hash. + """ d = self._get_all_shareholders(self._storage_index) d.addCallback(self._get_uri_extension) d.addCallback(self._done) @@ -594,12 +601,14 @@ class Helper(Referenceable): d.addErrback(_err) return d + chk_checker = CHKCheckerAndUEBFetcher + def _check_chk(self, storage_index, lp): # see if this file is already in the grid lp2 = self.log("doing a quick check+UEBfetch", parent=lp, level=log.NOISY) sb = self._storage_broker - c = CHKCheckerAndUEBFetcher(sb.get_servers_for_psi, storage_index, lp2) + c = self.chk_checker(sb.get_servers_for_psi, storage_index, lp2) d = c.check() def _checked(res): if res: diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 016044bbf..8d689664f 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -8,6 +8,11 @@ if PY2: from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import os +from functools import ( + partial, +) +import attr + from twisted.internet import defer from twisted.trial import unittest from twisted.application import service @@ -23,7 +28,7 @@ from allmydata.storage.server import si_b2a from allmydata.storage_client import StorageFarmBroker from allmydata.immutable import offloaded, upload from allmydata import uri, client -from allmydata.util import hashutil, fileutil, mathutil +from allmydata.util import hashutil, fileutil, mathutil, dictutil from .common import ( EMPTY_CLIENT_CONFIG, @@ -79,23 +84,30 @@ class Helper_fake_upload(offloaded.Helper): lp) return uh -class Helper_already_uploaded(Helper_fake_upload): - def _check_chk(self, storage_index, lp): - res = upload.HelperUploadResults() - res.uri_extension_hash = hashutil.uri_extension_hash(b"") +@attr.s +class FakeCHKCheckerAndUEBFetcher(object): + """ + A fake of ``CHKCheckerAndUEBFetcher`` which hard-codes some check result. + """ + peer_getter = attr.ib() + storage_index = attr.ib() + logparent = attr.ib() - # we're pretending that the file they're trying to upload was already - # present in the grid. We return some information about the file, so - # the client can decide if they like the way it looks. The parameters - # used here are chosen to match the defaults. - PARAMS = FakeClient.DEFAULT_ENCODING_PARAMETERS - ueb_data = {"needed_shares": PARAMS["k"], - "total_shares": PARAMS["n"], - "segment_size": min(PARAMS["max_segment_size"], len(DATA)), - "size": len(DATA), - } - res.uri_extension_data = ueb_data - return defer.succeed(res) + _sharemap = attr.ib() + _ueb_data = attr.ib() + + @property + def _ueb_hash(self): + return hashutil.uri_extension_hash( + uri.pack_extension(self._ueb_data), + ) + + def check(self): + return defer.succeed(( + self._sharemap, + self._ueb_data, + self._ueb_hash, + )) class FakeClient(service.MultiService): introducer_clients = [] @@ -171,13 +183,15 @@ class AssistedUpload(unittest.TestCase): # bogus host/port t.setLocation(b"bogus:1234") - def setUpHelper(self, basedir, helper_class=Helper_fake_upload): + def setUpHelper(self, basedir, helper_class=Helper_fake_upload, chk_checker=None): fileutil.make_dirs(basedir) - self.helper = h = helper_class(basedir, + self.helper = helper_class(basedir, self.s.storage_broker, self.s.secret_holder, None, None) - self.helper_furl = self.tub.registerReference(h) + if chk_checker is not None: + self.helper.chk_checker = chk_checker + self.helper_furl = self.tub.registerReference(self.helper) def tearDown(self): d = self.s.stopService() @@ -209,6 +223,10 @@ class AssistedUpload(unittest.TestCase): def _uploaded(results): the_uri = results.get_uri() self.assertIn(b"CHK", the_uri) + self.assertNotEqual( + results.get_pushed_shares(), + 0, + ) d.addCallback(_uploaded) def _check_empty(res): @@ -253,6 +271,11 @@ class AssistedUpload(unittest.TestCase): result1.get_uri(), result2.get_uri(), ) + # It would be really cool to assert that result1.get_pushed_shares() + + # result2.get_pushed_shares() == total_shares here. However, we're + # faking too much for that to be meaningful here. Also it doesn't + # hold because we don't actually push _anything_, we just lie about + # having pushed stuff. def test_previous_upload_failed(self): self.basedir = "helper/AssistedUpload/test_previous_upload_failed" @@ -303,28 +326,46 @@ class AssistedUpload(unittest.TestCase): return d + @inline_callbacks def test_already_uploaded(self): + """ + If enough shares to satisfy the needed parameter already exist, + """ + params = FakeClient.DEFAULT_ENCODING_PARAMETERS + chk_checker = partial( + FakeCHKCheckerAndUEBFetcher, + sharemap=dictutil.DictOfSets({ + 0: {b"server0"}, + 1: {b"server1"}, + }), + ueb_data={ + "size": len(DATA), + "segment_size": min(params["max_segment_size"], len(DATA)), + "needed_shares": params["k"], + "total_shares": params["n"], + }, + ) self.basedir = "helper/AssistedUpload/test_already_uploaded" - self.setUpHelper(self.basedir, helper_class=Helper_already_uploaded) + self.setUpHelper( + self.basedir, + chk_checker=chk_checker, + ) u = make_uploader(self.helper_furl, self.s) - d = wait_a_few_turns() + yield wait_a_few_turns() - def _ready(res): - assert u._helper + assert u._helper - return upload_data(u, DATA, convergence=b"some convergence string") - d.addCallback(_ready) - def _uploaded(results): - the_uri = results.get_uri() - assert b"CHK" in the_uri - d.addCallback(_uploaded) + results = yield upload_data(u, DATA, convergence=b"some convergence string") + the_uri = results.get_uri() + assert b"CHK" in the_uri - def _check_empty(res): - files = os.listdir(os.path.join(self.basedir, "CHK_encoding")) - self.failUnlessEqual(files, []) - files = os.listdir(os.path.join(self.basedir, "CHK_incoming")) - self.failUnlessEqual(files, []) - d.addCallback(_check_empty) + files = os.listdir(os.path.join(self.basedir, "CHK_encoding")) + self.failUnlessEqual(files, []) + files = os.listdir(os.path.join(self.basedir, "CHK_incoming")) + self.failUnlessEqual(files, []) - return d + self.assertEqual( + results.get_pushed_shares(), + 0, + ) From 555b751af0dda8e9eaf0464b46be67de85b6d757 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 11:05:00 -0400 Subject: [PATCH 11/61] Further expand existing test coverage by reducing fake quantities Helper_fake_upload is replaced by more narrowly focused CHKUploadHelper_fake --- src/allmydata/immutable/offloaded.py | 14 +++++++++----- src/allmydata/test/test_helper.py | 27 ++++++++++----------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index 0802a12fb..f99902217 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -653,11 +653,15 @@ class Helper(Referenceable): si_s = si_b2a(storage_index) incoming_file = os.path.join(self._chk_incoming, si_s) encoding_file = os.path.join(self._chk_encoding, si_s) - uh = CHKUploadHelper(storage_index, self, - self._storage_broker, - self._secret_holder, - incoming_file, encoding_file, - lp) + uh = self.chk_upload( + storage_index, + self, + self._storage_broker, + self._secret_holder, + incoming_file, + encoding_file, + lp, + ) return uh def _add_upload(self, uh): diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 8d689664f..4c5366fd9 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -72,18 +72,6 @@ class CHKUploadHelper_fake(offloaded.CHKUploadHelper): d.addCallback(_got_size) return d -class Helper_fake_upload(offloaded.Helper): - def _make_chk_upload_helper(self, storage_index, lp): - si_s = str(si_b2a(storage_index), "utf-8") - incoming_file = os.path.join(self._chk_incoming, si_s) - encoding_file = os.path.join(self._chk_encoding, si_s) - uh = CHKUploadHelper_fake(storage_index, self, - self._storage_broker, - self._secret_holder, - incoming_file, encoding_file, - lp) - return uh - @attr.s class FakeCHKCheckerAndUEBFetcher(object): """ @@ -183,12 +171,17 @@ class AssistedUpload(unittest.TestCase): # bogus host/port t.setLocation(b"bogus:1234") - def setUpHelper(self, basedir, helper_class=Helper_fake_upload, chk_checker=None): + def setUpHelper(self, basedir, chk_upload=CHKUploadHelper_fake, chk_checker=None): fileutil.make_dirs(basedir) - self.helper = helper_class(basedir, - self.s.storage_broker, - self.s.secret_holder, - None, None) + self.helper = offloaded.Helper( + basedir, + self.s.storage_broker, + self.s.secret_holder, + None, + None, + ) + if chk_upload is not None: + self.helper.chk_upload = chk_upload if chk_checker is not None: self.helper.chk_checker = chk_checker self.helper_furl = self.tub.registerReference(self.helper) From 9a18843bb176c8f70bc19b43762eeacdca03edf7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 11:09:31 -0400 Subject: [PATCH 12/61] finish the comment --- src/allmydata/test/test_helper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 4c5366fd9..4e3c92706 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -322,7 +322,8 @@ class AssistedUpload(unittest.TestCase): @inline_callbacks def test_already_uploaded(self): """ - If enough shares to satisfy the needed parameter already exist, + If enough shares to satisfy the needed parameter already exist, the upload + succeeds without pushing any shares. """ params = FakeClient.DEFAULT_ENCODING_PARAMETERS chk_checker = partial( From 5a51d98479306f916b2bdcee7f950aa84f12301b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 13 Oct 2020 15:19:12 -0400 Subject: [PATCH 13/61] Add missing chk_upload, document two new attrs, and move them up --- src/allmydata/immutable/offloaded.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index f99902217..2aba89f8f 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -496,6 +496,14 @@ class Helper(Referenceable): :ivar dict[bytes, CHKUploadHelper] _active_uploads: For any uploads which have been started but not finished, a mapping from storage index to the upload helper. + + :ivar chk_checker: A callable which returns an object like a + CHKCheckerAndUEBFetcher instance which can check CHK shares. + Primarily for the convenience of tests to override. + + :ivar chk_upload: A callable which returns an object like a + CHKUploadHelper instance which can upload CHK shares. Primarily for + the convenience of tests to override. """ # this is the non-distributed version. When we need to have multiple # helpers, this object will become the HelperCoordinator, and will query @@ -510,6 +518,9 @@ class Helper(Referenceable): } MAX_UPLOAD_STATUSES = 10 + chk_checker = CHKCheckerAndUEBFetcher + chk_upload = CHKUploadHelper + def __init__(self, basedir, storage_broker, secret_holder, stats_provider, history): self._basedir = basedir @@ -601,8 +612,6 @@ class Helper(Referenceable): d.addErrback(_err) return d - chk_checker = CHKCheckerAndUEBFetcher - def _check_chk(self, storage_index, lp): # see if this file is already in the grid lp2 = self.log("doing a quick check+UEBfetch", From 40e0ef0fad70a3c40e15cced94e71a43f715e9b6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Oct 2020 12:02:56 -0400 Subject: [PATCH 14/61] Directly test CHKCheckerAndUEBFetcher cases --- src/allmydata/test/test_helper.py | 226 +++++++++++++++++++++++++++++- 1 file changed, 225 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 4e3c92706..1f1ed2bea 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -8,6 +8,9 @@ if PY2: from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import os +from struct import ( + pack, +) from functools import ( partial, ) @@ -24,14 +27,35 @@ from eliot.twisted import ( ) from allmydata.crypto import aes -from allmydata.storage.server import si_b2a +from allmydata.storage.server import ( + si_b2a, + StorageServer, +) from allmydata.storage_client import StorageFarmBroker +from allmydata.immutable.layout import ( + make_write_bucket_proxy, +) from allmydata.immutable import offloaded, upload from allmydata import uri, client from allmydata.util import hashutil, fileutil, mathutil, dictutil +from .no_network import ( + NoNetworkServer, + LocalWrapper, + fireNow, +) from .common import ( EMPTY_CLIENT_CONFIG, + SyncTestCase, +) + +from testtools.matchers import ( + Equals, + MatchesListwise, + IsInstance, +) +from testtools.twistedsupport import ( + succeeded, ) MiB = 1024*1024 @@ -363,3 +387,203 @@ class AssistedUpload(unittest.TestCase): results.get_pushed_shares(), 0, ) + + +class CHKCheckerAndUEBFetcherTests(SyncTestCase): + """ + Tests for ``CHKCheckerAndUEBFetcher``. + """ + def test_check_no_peers(self): + """ + If the supplied "peer getter" returns no peers then + ``CHKCheckerAndUEBFetcher.check`` returns a ``Deferred`` that fires + with ``False``. + """ + storage_index = b"a" * 16 + peers = {storage_index: []} + caf = offloaded.CHKCheckerAndUEBFetcher( + peers.get, + storage_index, + None, + ) + self.assertThat( + caf.check(), + succeeded(Equals(False)), + ) + + @inline_callbacks + def test_check_ueb_unavailable(self): + """ + If the UEB cannot be read from any of the peers supplied by the "peer + getter" then ``CHKCheckerAndUEBFetcher.check`` returns a ``Deferred`` + that fires with ``False``. + """ + storage_index = b"a" * 16 + serverid = b"b" * 20 + storage = StorageServer(self.mktemp(), serverid) + rref_without_ueb = LocalWrapper(storage, fireNow) + yield write_bad_share(rref_without_ueb, storage_index) + server_without_ueb = NoNetworkServer(serverid, rref_without_ueb) + peers = {storage_index: [server_without_ueb]} + caf = offloaded.CHKCheckerAndUEBFetcher( + peers.get, + storage_index, + None, + ) + self.assertThat( + caf.check(), + succeeded(Equals(False)), + ) + + @inline_callbacks + def test_not_enough_shares(self): + """ + If fewer shares are found than are required to reassemble the data then + ``CHKCheckerAndUEBFetcher.check`` returns a ``Deferred`` that fires + with ``False``. + """ + storage_index = b"a" * 16 + serverid = b"b" * 20 + storage = StorageServer(self.mktemp(), serverid) + rref_with_ueb = LocalWrapper(storage, fireNow) + ueb = { + "needed_shares": 2, + "total_shares": 2, + "segment_size": 128 * 1024, + "size": 1024, + } + yield write_good_share(rref_with_ueb, storage_index, ueb, [0]) + + server_with_ueb = NoNetworkServer(serverid, rref_with_ueb) + peers = {storage_index: [server_with_ueb]} + caf = offloaded.CHKCheckerAndUEBFetcher( + peers.get, + storage_index, + None, + ) + self.assertThat( + caf.check(), + succeeded(Equals(False)), + ) + + @inline_callbacks + def test_enough_shares(self): + """ + If enough shares are found to reassemble the data then + ``CHKCheckerAndUEBFetcher.check`` returns a ``Deferred`` that fires + with share and share placement information. + """ + storage_index = b"a" * 16 + serverids = list( + ch * 20 + for ch + in [b"b", b"c"] + ) + storages = list( + StorageServer(self.mktemp(), serverid) + for serverid + in serverids + ) + rrefs_with_ueb = list( + LocalWrapper(storage, fireNow) + for storage + in storages + ) + ueb = { + "needed_shares": len(serverids), + "total_shares": len(serverids), + "segment_size": 128 * 1024, + "size": 1024, + } + for n, rref_with_ueb in enumerate(rrefs_with_ueb): + yield write_good_share(rref_with_ueb, storage_index, ueb, [n]) + + servers_with_ueb = list( + NoNetworkServer(serverid, rref_with_ueb) + for (serverid, rref_with_ueb) + in zip(serverids, rrefs_with_ueb) + ) + peers = {storage_index: servers_with_ueb} + caf = offloaded.CHKCheckerAndUEBFetcher( + peers.get, + storage_index, + None, + ) + self.assertThat( + caf.check(), + succeeded(MatchesListwise([ + Equals({ + n: {serverid} + for (n, serverid) + in enumerate(serverids) + }), + Equals(ueb), + IsInstance(bytes), + ])), + ) + + +def write_bad_share(storage_rref, storage_index): + """ + Write a share with a corrupt URI extension block. + """ + # Write some trash to the right bucket on this storage server. It won't + # have a recoverable UEB block. + return write_share(storage_rref, storage_index, [0], b"\0" * 1024) + + +def write_good_share(storage_rref, storage_index, ueb, sharenums): + """ + Write a valid share with the given URI extension block. + """ + write_proxy = make_write_bucket_proxy( + storage_rref, + None, + 1024, + ueb["segment_size"], + 1, + 1, + ueb["size"], + ) + # See allmydata/immutable/layout.py + offset = write_proxy._offsets["uri_extension"] + filler = b"\0" * (offset - len(write_proxy._offset_data)) + ueb_data = uri.pack_extension(ueb) + data = ( + write_proxy._offset_data + + filler + + pack(write_proxy.fieldstruct, len(ueb_data)) + + ueb_data + ) + return write_share(storage_rref, storage_index, sharenums, data) + + +@inline_callbacks +def write_share(storage_rref, storage_index, sharenums, sharedata): + """ + Write the given share data to the given storage index using the given + IStorageServer remote reference. + + :param foolscap.ipb.IRemoteReference storage_rref: A remote reference to + an IStorageServer. + + :param bytes storage_index: The storage index to which to write the share + data. + + :param [int] sharenums: The share numbers to which to write this sharedata. + + :param bytes sharedata: The ciphertext to write as the share. + """ + ignored, writers = yield storage_rref.callRemote( + "allocate_buckets", + storage_index, + b"x" * 16, + b"x" * 16, + sharenums, + len(sharedata), + LocalWrapper(None), + + ) + [writer] = writers.values() + yield writer.callRemote("write", 0, sharedata) + yield writer.callRemote("close") From bcd7cdf86ff6457555b93918016c0382b65c8d39 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 10:47:49 -0400 Subject: [PATCH 15/61] Some passing tests on Python 3. --- src/allmydata/client.py | 4 ++-- src/allmydata/introducer/client.py | 2 ++ src/allmydata/node.py | 2 +- src/allmydata/test/test_node.py | 6 +++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index af3a17d48..3614f081b 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -628,7 +628,7 @@ def storage_enabled(config): :return bool: ``True`` if storage is enabled, ``False`` otherwise. """ - return config.get_config(b"storage", b"enabled", True, boolean=True) + return config.get_config("storage", "enabled", True, boolean=True) def anonymous_storage_enabled(config): @@ -1020,7 +1020,7 @@ class _Client(node.Node, pollmixin.PollMixin): def init_control(self): c = ControlServer() c.setServiceParent(self) - control_url = self.control_tub.registerReference(c) + control_url = self.control_tub.registerReference(c).encode("utf-8") self.config.write_private_config("control.furl", control_url + b"\n") def init_helper(self): diff --git a/src/allmydata/introducer/client.py b/src/allmydata/introducer/client.py index 225ec1abc..243dd48bd 100644 --- a/src/allmydata/introducer/client.py +++ b/src/allmydata/introducer/client.py @@ -1,3 +1,5 @@ +from past.builtins import unicode + import time from zope.interface import implementer from twisted.application import service diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 26fc6fc97..d401e12ae 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -742,7 +742,7 @@ class Node(service.MultiService): if self.tub is not None: self.nodeid = b32decode(self.tub.tubID.upper()) # binary format self.short_nodeid = b32encode(self.nodeid).lower()[:8] # for printing - self.config.write_config_file("my_nodeid", b32encode(self.nodeid).lower() + "\n") + self.config.write_config_file("my_nodeid", b32encode(self.nodeid).lower() + b"\n", mode="wb") self.tub.setServiceParent(self) else: self.nodeid = self.short_nodeid = None diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index ecbf28d80..26f47fb82 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -194,8 +194,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): config = read_config(basedir, "portnum") self.assertEqual( config.items("node"), - [(b"nickname", b"foo"), - (b"timeout.disconnect", b"12"), + [("nickname", "foo"), + ("timeout.disconnect", "12"), ], ) @@ -545,7 +545,7 @@ enabled = true class FakeTub(object): def __init__(self): - self.tubID = base64.b32encode("foo") + self.tubID = base64.b32encode(b"foo") self.listening_ports = [] def setOption(self, name, value): pass def removeAllConnectionHintHandlers(self): pass From f689d59a406f35a3bb5e2958fbd84f90270d1b3b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 10:55:33 -0400 Subject: [PATCH 16/61] More passing tests on Python 3. --- src/allmydata/client.py | 6 +++--- src/allmydata/node.py | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 3614f081b..0770a3972 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -642,7 +642,7 @@ def anonymous_storage_enabled(config): """ return ( storage_enabled(config) and - config.get_config(b"storage", b"anonymous", True, boolean=True) + config.get_config("storage", "anonymous", True, boolean=True) ) @@ -781,7 +781,7 @@ class _Client(node.Node, pollmixin.PollMixin): vk_string = ed25519.string_from_verifying_key(self._node_public_key) vk_bytes = remove_prefix(vk_string, ed25519.PUBLIC_KEY_PREFIX) seed = base32.b2a(vk_bytes) - self.config.write_config_file("permutation-seed", seed+"\n") + self.config.write_config_file("permutation-seed", seed+b"\n", mode="wb") return seed.strip() def get_anonymous_storage_server(self): @@ -806,7 +806,7 @@ class _Client(node.Node, pollmixin.PollMixin): config_storedir = self.get_config( "storage", "storage_dir", self.STOREDIR, - ).decode('utf-8') + ) storedir = self.config.get_config_path(config_storedir) data = self.config.get_config("storage", "reserved_space", None) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index d401e12ae..25fa48b32 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -549,9 +549,12 @@ def _convert_tub_port(s): :returns: a proper Twisted endpoint string like (`tcp:X`) is `s` is a bare number, or returns `s` as-is """ - if re.search(r'^\d+$', s): - return "tcp:{}".format(int(s)) - return s + us = s + if isinstance(s, bytes): + us = s.decode("utf-8") + if re.search(r'^\d+$', us): + return "tcp:{}".format(int(us)) + return us def _tub_portlocation(config): @@ -844,7 +847,7 @@ class Node(service.MultiService): self.config.get_config_path("log_gatherer.furl")) incident_dir = self.config.get_config_path("logs", "incidents") - foolscap.logging.log.setLogDir(incident_dir.encode(get_filesystem_encoding())) + foolscap.logging.log.setLogDir(incident_dir) twlog.msg("Foolscap logging initialized") twlog.msg("Note to developers: twistd.log does not receive very much.") twlog.msg("Use 'flogtool tail -c NODEDIR/private/logport.furl' instead") From 51d472e22176294623ea67d5b27f808ef84a0110 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 11:13:11 -0400 Subject: [PATCH 17/61] More progress towards passing tests on Python 3. --- src/allmydata/client.py | 4 ++-- src/allmydata/node.py | 15 +++++++++++---- src/allmydata/test/test_node.py | 6 ++++-- src/allmydata/util/fileutil.py | 6 ++++-- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 0770a3972..7dcda92e3 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -1020,8 +1020,8 @@ class _Client(node.Node, pollmixin.PollMixin): def init_control(self): c = ControlServer() c.setServiceParent(self) - control_url = self.control_tub.registerReference(c).encode("utf-8") - self.config.write_private_config("control.furl", control_url + b"\n") + control_url = self.control_tub.registerReference(c) + self.config.write_private_config("control.furl", control_url + "\n") def init_helper(self): self.helper = Helper(self.config.get_config_path("helper"), diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 25fa48b32..a5da6370f 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -360,14 +360,16 @@ class _Config(object): """ privname = os.path.join(self._basedir, "private", name) try: - value = fileutil.read(privname) + value = fileutil.read(privname, mode="r") except EnvironmentError as e: if e.errno != errno.ENOENT: raise # we only care about "file doesn't exist" if default is _None: raise MissingConfigEntry("The required configuration file %s is missing." % (quote_output(privname),)) - if isinstance(default, (bytes, unicode)): + if isinstance(default, bytes): + default = unicode(default, "utf-8") + if isinstance(default, unicode): value = default else: value = default() @@ -379,19 +381,21 @@ class _Config(object): config file that resides within the subdirectory named 'private'), and return it. """ + if isinstance(value, unicode): + value = value.encode("utf-8") privname = os.path.join(self._basedir, "private", name) with open(privname, "wb") as f: f.write(value) def get_private_config(self, name, default=_None): - """Read the (string) contents of a private config file (which is a + """Read the (native string) contents of a private config file (a config file that resides within the subdirectory named 'private'), and return it. Return a default, or raise an error if one was not given. """ privname = os.path.join(self._basedir, "private", name) try: - return fileutil.read(privname).strip() + return fileutil.read(privname, mode="r").strip() except EnvironmentError as e: if e.errno != errno.ENOENT: raise # we only care about "file doesn't exist" @@ -689,6 +693,9 @@ def create_main_tub(config, tub_options, port_or_endpoint = tor_provider.get_listener() else: port_or_endpoint = port + if PY2 and isinstance(port_or_endpoint, unicode): + # Foolscap requires native string + port_or_endpoint = port_or_endpoint.encode("utf-8") tub.listenOn(port_or_endpoint) tub.setLocation(location) log.msg("Tub location set to %s" % (location,)) diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 26f47fb82..b771f1cc6 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -1,3 +1,5 @@ +from past.builtins import unicode + import base64 import os import stat @@ -145,7 +147,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): def test_tahoe_cfg_utf8(self): basedir = "test_node/test_tahoe_cfg_utf8" fileutil.make_dirs(basedir) - f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wb') f.write(u"\uFEFF[node]\n".encode('utf-8')) f.write(u"nickname = \u2621\n".encode('utf-8')) f.close() @@ -333,7 +335,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): ns.called = False def call_setLogDir(logdir): ns.called = True - self.failUnless(isinstance(logdir, str), logdir) + self.failUnless(isinstance(logdir, unicode), logdir) self.patch(foolscap.logging.log, 'setLogDir', call_setLogDir) create_node_dir(basedir, "nothing to see here") diff --git a/src/allmydata/util/fileutil.py b/src/allmydata/util/fileutil.py index 693cd1d63..ea16c0d6a 100644 --- a/src/allmydata/util/fileutil.py +++ b/src/allmydata/util/fileutil.py @@ -271,11 +271,13 @@ def write_atomically(target, contents, mode="b"): move_into_place(target+".tmp", target) def write(path, data, mode="wb"): + if "b" in mode and isinstance(data, str): + data = data.encode("utf-8") with open(path, mode) as f: f.write(data) -def read(path): - with open(path, "rb") as rf: +def read(path, mode="rb"): + with open(path, mode) as rf: return rf.read() def put_file(path, inf): From f7a89f76e7c65dbe8732d73f3ce10492f96df65e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 11:20:10 -0400 Subject: [PATCH 18/61] All tests pass on Python 3. --- src/allmydata/test/test_node.py | 9 +++++++-- src/allmydata/util/configutil.py | 9 +++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index b771f1cc6..73f7baca2 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -1,3 +1,4 @@ +from future.utils import PY2 from past.builtins import unicode import base64 @@ -153,8 +154,12 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): f.close() config = read_config(basedir, "") - self.failUnlessEqual(config.get_config("node", "nickname").decode('utf-8'), - u"\u2621") + # Config returns native strings: + expected_nick = u"\u2621" + if PY2: + expected_nick = expected_nick.encode("utf-8") + self.failUnlessEqual(config.get_config("node", "nickname"), + expected_nick) def test_tahoe_cfg_hash_in_name(self): basedir = "test_node/test_cfg_hash_in_name" diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 1a1a93f18..f85972d8d 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -10,7 +10,7 @@ from __future__ import division from __future__ import print_function from __future__ import unicode_literals -from future.utils import PY2 +from future.utils import PY2, PY3 if PY2: # We don't do open(), because we want files to read/write native strs when # we do "r" or "w". @@ -42,11 +42,12 @@ def get_config(tahoe_cfg): """ config = SafeConfigParser() with open(tahoe_cfg, "r") as f: - # On Python 2, where we read in bytes, skip any initial Byte Order - # Mark. Since this is an ordinary file, we don't need to handle - # incomplete reads, and can assume seekability. + # Skip any initial Byte Order Mark. Since this is an ordinary file, we + # don't need to handle incomplete reads, and can assume seekability. if PY2 and f.read(3) != b'\xEF\xBB\xBF': f.seek(0) + if PY3 and f.read(1) != u"\uFEFF": + f.seek(0) config.readfp(f) return config From a887ee5fc5bb77ccadd07e98e782bd0fc3080e8c Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 11:20:24 -0400 Subject: [PATCH 19/61] News file. --- 3479.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 3479.minor diff --git a/3479.minor b/3479.minor new file mode 100644 index 000000000..e69de29bb From 6aa96bbb8d17fac74e685146ee5c5b2971dd87ec Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 11:23:38 -0400 Subject: [PATCH 20/61] Port test_node.py to Python 3. --- src/allmydata/node.py | 2 -- src/allmydata/test/test_node.py | 13 +++++++++++-- src/allmydata/util/_python3.py | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index a5da6370f..9f5375885 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -16,8 +16,6 @@ from base64 import b32decode, b32encode # Python 2 compatibility from six.moves import configparser from future.utils import PY2 -if PY2: - from io import BytesIO as StringIO # noqa: F811 from twisted.python import log as twlog from twisted.application import service diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 73f7baca2..afde00edd 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -1,5 +1,14 @@ +""" +Ported to Python 3. +""" +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + from future.utils import PY2 -from past.builtins import unicode +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 base64 import os @@ -340,7 +349,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): ns.called = False def call_setLogDir(logdir): ns.called = True - self.failUnless(isinstance(logdir, unicode), logdir) + self.failUnless(isinstance(logdir, str), logdir) self.patch(foolscap.logging.log, 'setLogDir', call_setLogDir) create_node_dir(basedir, "nothing to see here") diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index da4b86eef..2c51b469b 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -120,6 +120,7 @@ PORTED_TEST_MODULES = [ "allmydata.test.test_monitor", "allmydata.test.test_netstring", "allmydata.test.test_no_network", + "allmydata.test.test_node", "allmydata.test.test_observer", "allmydata.test.test_pipeline", "allmydata.test.test_python3", From 8e618b93838eba8e51eeac15016c98eb0e77a01f Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 16 Oct 2020 11:25:29 -0400 Subject: [PATCH 21/61] Fix Python 2 issue. --- src/allmydata/node.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 9f5375885..852c8889d 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -9,7 +9,7 @@ import os.path import re import types import errno -from io import StringIO +from io import StringIO, BytesIO import tempfile from base64 import b32decode, b32encode @@ -209,7 +209,10 @@ def config_from_string(basedir, portnumfile, config_str, _valid_config=None): # load configuration from in-memory string parser = configparser.SafeConfigParser() - parser.readfp(StringIO(config_str)) + if PY2: + parser.readfp(BytesIO(config_str.encode("utf-8"))) + else: + parser.readfp(StringIO(config_str)) fname = "" configutil.validate_config(fname, parser, _valid_config) From fc1f43c7a3a4d4d69158a1d818c9cdc19bfde4d0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 16 Oct 2020 19:07:28 -0400 Subject: [PATCH 22/61] How does it go without the type check This is for py36: > TypeError: helper_furl must be bytes, got 'pb://kl5iekm6itcyjejirxva2upthepsasnn@bogus:1234/frekgeq7gsongibyeuvzmvqoyf4h5pcx' instead --- src/allmydata/test/test_helper.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 1f1ed2bea..2f2d0c9d5 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -167,8 +167,6 @@ def make_uploader(helper_furl, parent, override_name=None): :param str override_name: If not ``None``, a new name for the uploader service. Multiple services cannot coexist with the same name. """ - if not isinstance(helper_furl, bytes): - raise TypeError("helper_furl must be bytes, got {!r} instead".format(helper_furl)) u = upload.Uploader(helper_furl) if override_name is not None: u.name = override_name From 8239d92892f5448de380e900942e95bed33390a9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:04:48 -0400 Subject: [PATCH 23/61] news fragment --- newsfragments/3483.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3483.minor diff --git a/newsfragments/3483.minor b/newsfragments/3483.minor new file mode 100644 index 000000000..e69de29bb From bc8c2c46896b6a03e7b82436ef23e7f544534c01 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:04:59 -0400 Subject: [PATCH 24/61] Put all CircleCI jobs into the "dockerhub-auth" context --- .circleci/config.yml | 88 ++++++++++++++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 24 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1327a524b..9d9d0b6d3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -14,44 +14,73 @@ version: 2.1 workflows: ci: jobs: - # Platforms - - "debian-9" + # Start with jobs testing various platforms. + + # Every job that pulls a Docker image from Docker Hub needs to provide + # credentials for that pull operation to avoid being subjected to + # unauthenticated pull limits shared across all of CircleCI. Use this + # first job to define a yaml anchor that can be used to supply a + # CircleCI job context which makes Docker Hub credentials available in + # the environment. + # + # Contexts are managed in the CircleCI web interface: + # + # https://app.circleci.com/settings/organization/github/tahoe-lafs/contexts + - "debian-9": &DOCKERHUB_CONTEXT + context: "dockerhub-auth" + - "debian-8": + <<: *DOCKERHUB_CONTEXT requires: - "debian-9" - - "ubuntu-20-04" + - "ubuntu-20-04": + <<: *DOCKERHUB_CONTEXT - "ubuntu-18-04": + <<: *DOCKERHUB_CONTEXT requires: - "ubuntu-20-04" - "ubuntu-16-04": + <<: *DOCKERHUB_CONTEXT requires: - "ubuntu-20-04" - - "fedora-29" + - "fedora-29": + <<: *DOCKERHUB_CONTEXT - "fedora-28": + <<: *DOCKERHUB_CONTEXT requires: - "fedora-29" - - "centos-8" + - "centos-8": + <<: *DOCKERHUB_CONTEXT - - "nixos-19-09" + - "nixos-19-09": + <<: *DOCKERHUB_CONTEXT # Test against PyPy 2.7 - - "pypy27-buster" + - "pypy27-buster": + <<: *DOCKERHUB_CONTEXT # Just one Python 3.6 configuration while the port is in-progress. - - "python36" + - "python36": + <<: *DOCKERHUB_CONTEXT # Other assorted tasks and configurations - - "lint" - - "pyinstaller" - - "deprecations" - - "c-locale" + - "lint": + <<: *DOCKERHUB_CONTEXT + - "pyinstaller": + <<: *DOCKERHUB_CONTEXT + - "deprecations": + <<: *DOCKERHUB_CONTEXT + - "c-locale": + <<: *DOCKERHUB_CONTEXT # Any locale other than C or UTF-8. - - "another-locale" + - "another-locale": + <<: *DOCKERHUB_CONTEXT - "integration": + <<: *DOCKERHUB_CONTEXT requires: # If the unit test suite doesn't pass, don't bother running the # integration tests. @@ -59,7 +88,8 @@ workflows: # Generate the underlying data for a visualization to aid with Python 3 # porting. - - "build-porting-depgraph" + - "build-porting-depgraph": + <<: *DOCKERHUB_CONTEXT images: # Build the Docker images used by the ci jobs. This makes the ci jobs @@ -74,16 +104,26 @@ workflows: - "master" jobs: - - "build-image-debian-8" - - "build-image-debian-9" - - "build-image-ubuntu-16-04" - - "build-image-ubuntu-18-04" - - "build-image-ubuntu-20-04" - - "build-image-fedora-28" - - "build-image-fedora-29" - - "build-image-centos-8" - - "build-image-pypy27-buster" - - "build-image-python36-ubuntu" + - "build-image-debian-8": + <<: *DOCKERHUB_CONTEXT + - "build-image-debian-9": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-16-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-18-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-ubuntu-20-04": + <<: *DOCKERHUB_CONTEXT + - "build-image-fedora-28": + <<: *DOCKERHUB_CONTEXT + - "build-image-fedora-29": + <<: *DOCKERHUB_CONTEXT + - "build-image-centos-8": + <<: *DOCKERHUB_CONTEXT + - "build-image-pypy27-buster": + <<: *DOCKERHUB_CONTEXT + - "build-image-python36-ubuntu": + <<: *DOCKERHUB_CONTEXT jobs: From 22921e2b1dcfb2c41edaababc0071cf4268bc2a3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:08:30 -0400 Subject: [PATCH 25/61] Use secrets from the context to authenticate with Docker Hub --- .circleci/config.yml | 101 ++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d9d0b6d3..12cdb843d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -127,9 +127,28 @@ workflows: jobs: - lint: + dockerhub-auth-template: + # This isn't a real job. It doesn't get scheduled as part of any + # workhlow. Instead, it's just a place we can hang a yaml anchor to + # finish the Docker Hub authentication configuration. Workflow jobs using + # the DOCKERHUB_CONTEXT anchor will have access to the environment + # variables used here. These variables will allow the Docker Hub image + # pull to be authenticated and hopefully avoid hitting and rate limits. docker: - - image: "circleci/python:2" + - image: "null" + auth: &DOCKERHUB_AUTH + username: $DOCKERHUB_USERNAME + password: $DOCKERHUB_PASSWORD + + steps: + - run: + name: "Schema conformity" + command: | + + lint: + docker: + - <<: *DOCKERHUB_AUTH + image: "circleci/python:2" steps: - "checkout" @@ -146,7 +165,8 @@ jobs: pyinstaller: docker: - - image: "circleci/python:2" + - <<: *DOCKERHUB_AUTH + image: "circleci/python:2" steps: - "checkout" @@ -171,7 +191,8 @@ jobs: debian-9: &DEBIAN docker: - - image: "tahoelafsci/debian:9-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/debian:9-py2.7" user: "nobody" environment: &UTF_8_ENVIRONMENT @@ -252,14 +273,16 @@ jobs: debian-8: <<: *DEBIAN docker: - - image: "tahoelafsci/debian:8-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/debian:8-py2.7" user: "nobody" pypy27-buster: <<: *DEBIAN docker: - - image: "tahoelafsci/pypy:buster-py2" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/pypy:buster-py2" user: "nobody" environment: @@ -320,21 +343,24 @@ jobs: ubuntu-16-04: <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:16.04-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:16.04-py2.7" user: "nobody" ubuntu-18-04: &UBUNTU_18_04 <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:18.04-py2.7" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:18.04-py2.7" user: "nobody" python36: <<: *UBUNTU_18_04 docker: - - image: "tahoelafsci/ubuntu:18.04-py3" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:18.04-py3" user: "nobody" environment: @@ -349,13 +375,15 @@ jobs: ubuntu-20-04: <<: *DEBIAN docker: - - image: "tahoelafsci/ubuntu:20.04" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/ubuntu:20.04" user: "nobody" centos-8: &RHEL_DERIV docker: - - image: "tahoelafsci/centos:8-py2" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/centos:8-py2" user: "nobody" environment: *UTF_8_ENVIRONMENT @@ -377,21 +405,24 @@ jobs: fedora-28: <<: *RHEL_DERIV docker: - - image: "tahoelafsci/fedora:28-py" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/fedora:28-py" user: "nobody" fedora-29: <<: *RHEL_DERIV docker: - - image: "tahoelafsci/fedora:29-py" + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/fedora:29-py" user: "nobody" nixos-19-09: docker: # Run in a highly Nix-capable environment. - - image: "nixorg/nix:circleci" + - <<: *DOCKERHUB_AUTH + image: "nixorg/nix:circleci" environment: NIX_PATH: "nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-19.09-small.tar.gz" @@ -448,7 +479,8 @@ jobs: # # https://circleci.com/blog/how-to-build-a-docker-image-on-circleci-2-0/ docker: - - image: "docker:17.05.0-ce-git" + - <<: *DOCKERHUB_AUTH + image: "docker:17.05.0-ce-git" environment: DISTRO: "tahoelafsci/:foo-py2" @@ -458,47 +490,10 @@ jobs: steps: - "checkout" - "setup_remote_docker" - - run: - name: "Get openssl" - command: | - apk add --no-cache openssl - - run: - name: "Get Dockerhub secrets" - command: | - # If you create an encryption key like this: - # - # openssl enc -aes-256-cbc -k secret -P -md sha256 - - # From the output that looks like: - # - # salt=... - # key=... - # iv =... - # - # extract just the value for ``key``. - - # then you can re-generate ``secret-env-cipher`` locally using the - # command: - # - # openssl aes-256-cbc -e -md sha256 -in secret-env-plain -out .circleci/secret-env-cipher -pass env:KEY - # - # Make sure the key is set as the KEY environment variable in the - # CircleCI web interface. You can do this by visiting - # - # after logging in to CircleCI with an account in the tahoe-lafs - # CircleCI team. - # - # Then you can recover the environment plaintext (for example, to - # change and re-encrypt it) like just like CircleCI recovers it - # here: - # - openssl aes-256-cbc -d -md sha256 -in .circleci/secret-env-cipher -pass env:KEY >> ~/.env - run: name: "Log in to Dockerhub" command: | - . ~/.env - # TAHOELAFSCI_PASSWORD come from the secret env. - docker login -u tahoelafsci -p ${TAHOELAFSCI_PASSWORD} + docker login -u ${DOCKERHUB_USERNAME} -p ${DOCKERHUB_PASSWORD} - run: name: "Build image" command: | From e778c8ab8462be01118448b5244e62fb346cd372 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:09:09 -0400 Subject: [PATCH 26/61] This is no longer used by anything --- .circleci/secret-env-cipher | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .circleci/secret-env-cipher diff --git a/.circleci/secret-env-cipher b/.circleci/secret-env-cipher deleted file mode 100644 index 2facc470c..000000000 --- a/.circleci/secret-env-cipher +++ /dev/null @@ -1 +0,0 @@ -Salted__ •GPÁøÊ)|!÷[©U[‡ûvSÚ,F¿–m:ö š~ÓY[Uú_¸Fx×’¤Ÿ%“4l×Ö»Š8¼œ¹„1öø‰/lƒÌ`nÆ^·Z]óqš¬æ¢&ø°÷£Ý‚‚ß%T¡n \ No newline at end of file From 5e1d3db72e87f21c40170bb4fa049ddc67c7f401 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:12:58 -0400 Subject: [PATCH 27/61] Correct whitespace --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 12cdb843d..316cad4c8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -145,7 +145,7 @@ jobs: name: "Schema conformity" command: | - lint: + lint: docker: - <<: *DOCKERHUB_AUTH image: "circleci/python:2" From 1303a852858b201c04146cdcafb04b0be9024c26 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:21:11 -0400 Subject: [PATCH 28/61] Attempt to get the Docker Hub auth into the right place --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 316cad4c8..a5def0f2a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -134,9 +134,9 @@ jobs: # the DOCKERHUB_CONTEXT anchor will have access to the environment # variables used here. These variables will allow the Docker Hub image # pull to be authenticated and hopefully avoid hitting and rate limits. - docker: + docker: &DOCKERHUB_AUTH - image: "null" - auth: &DOCKERHUB_AUTH + auth: username: $DOCKERHUB_USERNAME password: $DOCKERHUB_PASSWORD From e2f03e00ba409f440e4a78afd07c6f2d6d9351d9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 12:27:22 -0400 Subject: [PATCH 29/61] typo --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a5def0f2a..b097812cb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -129,7 +129,7 @@ workflows: jobs: dockerhub-auth-template: # This isn't a real job. It doesn't get scheduled as part of any - # workhlow. Instead, it's just a place we can hang a yaml anchor to + # workflow. Instead, it's just a place we can hang a yaml anchor to # finish the Docker Hub authentication configuration. Workflow jobs using # the DOCKERHUB_CONTEXT anchor will have access to the environment # variables used here. These variables will allow the Docker Hub image From 81428d083940f46503387a03b44d5c443d529851 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 14:46:26 -0400 Subject: [PATCH 30/61] explain "Schema conformity" a bit more --- .circleci/config.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b097812cb..afa3fafa1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -142,8 +142,12 @@ jobs: steps: - run: - name: "Schema conformity" + name: "CircleCI YAML schema conformity" command: | + # This isn't a real command. We have to have something in this + # space, though, or the CircleCI yaml schema validator gets angry. + # Since this job is never scheduled this step is never run so the + # actual value here is irrelevant. lint: docker: From da75fa40694511641a5be4bbef602a0be57f3b5e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 14:47:18 -0400 Subject: [PATCH 31/61] make all the image builders run too, to see if they will --- .circleci/config.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index afa3fafa1..6bb02ce95 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -91,19 +91,19 @@ workflows: - "build-porting-depgraph": <<: *DOCKERHUB_CONTEXT - images: - # Build the Docker images used by the ci jobs. This makes the ci jobs - # faster and takes various spurious failures out of the critical path. - triggers: - # Build once a day - - schedule: - cron: "0 0 * * *" - filters: - branches: - only: - - "master" + # images: + # # Build the Docker images used by the ci jobs. This makes the ci jobs + # # faster and takes various spurious failures out of the critical path. + # triggers: + # # Build once a day + # - schedule: + # cron: "0 0 * * *" + # filters: + # branches: + # only: + # - "master" - jobs: + # jobs: - "build-image-debian-8": <<: *DOCKERHUB_CONTEXT - "build-image-debian-9": From dddf49ff71cdea5977d2d1784e30650967a7a5d8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 22 Oct 2020 17:00:02 -0400 Subject: [PATCH 32/61] Restore original image configuration --- .circleci/config.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6bb02ce95..afa3fafa1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -91,19 +91,19 @@ workflows: - "build-porting-depgraph": <<: *DOCKERHUB_CONTEXT - # images: - # # Build the Docker images used by the ci jobs. This makes the ci jobs - # # faster and takes various spurious failures out of the critical path. - # triggers: - # # Build once a day - # - schedule: - # cron: "0 0 * * *" - # filters: - # branches: - # only: - # - "master" + images: + # Build the Docker images used by the ci jobs. This makes the ci jobs + # faster and takes various spurious failures out of the critical path. + triggers: + # Build once a day + - schedule: + cron: "0 0 * * *" + filters: + branches: + only: + - "master" - # jobs: + jobs: - "build-image-debian-8": <<: *DOCKERHUB_CONTEXT - "build-image-debian-9": From 931bdef2a2b6fe6ca5d124911c3bf5cc73dd65a9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:20:54 -0400 Subject: [PATCH 33/61] Get rid of the old implementation and related unused code Also put in the new implementation, though now it needs tests because *there were no direct tests for the old one*. --- setup.py | 3 + src/allmydata/test/test_iputil.py | 167 +------------------------ src/allmydata/util/iputil.py | 197 +++--------------------------- 3 files changed, 22 insertions(+), 345 deletions(-) diff --git a/setup.py b/setup.py index 874cc1258..212e6c369 100644 --- a/setup.py +++ b/setup.py @@ -126,6 +126,9 @@ install_requires = [ # Support for Python 3 transition "future >= 0.18.2", + # Discover local network configuration + "netifaces", + # Utility code: "pyutil >= 3.3.0", diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index f403de35b..6f6100410 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -13,7 +13,7 @@ from future.utils import PY2, native_str if PY2: from 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 re, errno, subprocess, os, socket +import re, os, socket import gc from twisted.trial import unittest @@ -23,171 +23,6 @@ from tenacity import retry, stop_after_attempt from foolscap.api import Tub from allmydata.util import iputil, gcutil -import allmydata.test.common_util as testutil -from allmydata.util.namespace import Namespace - - -DOTTED_QUAD_RE=re.compile(r"^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$") - -# Mock output from subprocesses should be bytes, that's what happens on both -# Python 2 and Python 3: -MOCK_IPADDR_OUTPUT = b"""\ -1: lo: mtu 16436 qdisc noqueue state UNKNOWN \n\ - link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 - inet 127.0.0.1/8 scope host lo - inet6 ::1/128 scope host \n\ - valid_lft forever preferred_lft forever -2: eth1: mtu 1500 qdisc pfifo_fast state UP qlen 1000 - link/ether d4:3d:7e:01:b4:3e brd ff:ff:ff:ff:ff:ff - inet 192.168.0.6/24 brd 192.168.0.255 scope global eth1 - inet6 fe80::d63d:7eff:fe01:b43e/64 scope link \n\ - valid_lft forever preferred_lft forever -3: wlan0: mtu 1500 qdisc mq state UP qlen 1000 - link/ether 90:f6:52:27:15:0a brd ff:ff:ff:ff:ff:ff - inet 192.168.0.2/24 brd 192.168.0.255 scope global wlan0 - inet6 fe80::92f6:52ff:fe27:150a/64 scope link \n\ - valid_lft forever preferred_lft forever -""" - -MOCK_IFCONFIG_OUTPUT = b"""\ -eth1 Link encap:Ethernet HWaddr d4:3d:7e:01:b4:3e \n\ - inet addr:192.168.0.6 Bcast:192.168.0.255 Mask:255.255.255.0 - inet6 addr: fe80::d63d:7eff:fe01:b43e/64 Scope:Link - UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 - RX packets:154242234 errors:0 dropped:0 overruns:0 frame:0 - TX packets:155461891 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:1000 \n\ - RX bytes:84367213640 (78.5 GiB) TX bytes:73401695329 (68.3 GiB) - Interrupt:20 Memory:f4f00000-f4f20000 \n\ - -lo Link encap:Local Loopback \n\ - inet addr:127.0.0.1 Mask:255.0.0.0 - inet6 addr: ::1/128 Scope:Host - UP LOOPBACK RUNNING MTU:16436 Metric:1 - RX packets:27449267 errors:0 dropped:0 overruns:0 frame:0 - TX packets:27449267 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:0 \n\ - RX bytes:192643017823 (179.4 GiB) TX bytes:192643017823 (179.4 GiB) - -wlan0 Link encap:Ethernet HWaddr 90:f6:52:27:15:0a \n\ - inet addr:192.168.0.2 Bcast:192.168.0.255 Mask:255.255.255.0 - inet6 addr: fe80::92f6:52ff:fe27:150a/64 Scope:Link - UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 - RX packets:12352750 errors:0 dropped:0 overruns:0 frame:0 - TX packets:4501451 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:1000 \n\ - RX bytes:3916475942 (3.6 GiB) TX bytes:458353654 (437.1 MiB) -""" - -# This is actually from a VirtualBox VM running XP. -MOCK_ROUTE_OUTPUT = b"""\ -=========================================================================== -Interface List -0x1 ........................... MS TCP Loopback interface -0x2 ...08 00 27 c3 80 ad ...... AMD PCNET Family PCI Ethernet Adapter - Packet Scheduler Miniport -=========================================================================== -=========================================================================== -Active Routes: -Network Destination Netmask Gateway Interface Metric - 0.0.0.0 0.0.0.0 10.0.2.2 10.0.2.15 20 - 10.0.2.0 255.255.255.0 10.0.2.15 10.0.2.15 20 - 10.0.2.15 255.255.255.255 127.0.0.1 127.0.0.1 20 - 10.255.255.255 255.255.255.255 10.0.2.15 10.0.2.15 20 - 127.0.0.0 255.0.0.0 127.0.0.1 127.0.0.1 1 - 224.0.0.0 240.0.0.0 10.0.2.15 10.0.2.15 20 - 255.255.255.255 255.255.255.255 10.0.2.15 10.0.2.15 1 -Default Gateway: 10.0.2.2 -=========================================================================== -Persistent Routes: - None -""" - -UNIX_TEST_ADDRESSES = set(["127.0.0.1", "192.168.0.6", "192.168.0.2", "192.168.0.10"]) -WINDOWS_TEST_ADDRESSES = set(["127.0.0.1", "10.0.2.15", "192.168.0.10"]) -CYGWIN_TEST_ADDRESSES = set(["127.0.0.1", "192.168.0.10"]) - - -class FakeProcess(object): - def __init__(self, output, err): - self.output = output - self.err = err - def communicate(self): - return (self.output, self.err) - - -class ListAddresses(testutil.SignalMixin, unittest.TestCase): - def test_get_local_ip_for(self): - addr = iputil.get_local_ip_for('127.0.0.1') - self.failUnless(DOTTED_QUAD_RE.match(addr)) - # Bytes can be taken as input: - bytes_addr = iputil.get_local_ip_for(b'127.0.0.1') - self.assertEqual(addr, bytes_addr) - # The output is a native string: - self.assertIsInstance(addr, native_str) - - def test_list_async(self): - d = iputil.get_local_addresses_async() - def _check(addresses): - self.failUnlessIn("127.0.0.1", addresses) - self.failIfIn("0.0.0.0", addresses) - d.addCallbacks(_check) - return d - # David A.'s OpenSolaris box timed out on this test one time when it was at 2s. - test_list_async.timeout=4 - - def _test_list_async_mock(self, command, output, expected): - ns = Namespace() - ns.first = True - - def call_Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, - preexec_fn=None, close_fds=False, shell=False, cwd=None, env=None, - universal_newlines=False, startupinfo=None, creationflags=0): - if ns.first: - ns.first = False - e = OSError("EINTR") - e.errno = errno.EINTR - raise e - elif os.path.basename(args[0]) == command: - return FakeProcess(output, "") - else: - e = OSError("[Errno 2] No such file or directory") - e.errno = errno.ENOENT - raise e - self.patch(subprocess, 'Popen', call_Popen) - self.patch(os.path, 'isfile', lambda x: True) - - def call_get_local_ip_for(target): - if target in ("localhost", "127.0.0.1"): - return "127.0.0.1" - else: - return "192.168.0.10" - self.patch(iputil, 'get_local_ip_for', call_get_local_ip_for) - - def call_which(name): - return [name] - self.patch(iputil, 'which', call_which) - - d = iputil.get_local_addresses_async() - def _check(addresses): - self.failUnlessEquals(set(addresses), set(expected)) - d.addCallbacks(_check) - return d - - def test_list_async_mock_ip_addr(self): - self.patch(iputil, 'platform', "linux2") - return self._test_list_async_mock("ip", MOCK_IPADDR_OUTPUT, UNIX_TEST_ADDRESSES) - - def test_list_async_mock_ifconfig(self): - self.patch(iputil, 'platform', "linux2") - return self._test_list_async_mock("ifconfig", MOCK_IFCONFIG_OUTPUT, UNIX_TEST_ADDRESSES) - - def test_list_async_mock_route(self): - self.patch(iputil, 'platform', "win32") - return self._test_list_async_mock("route.exe", MOCK_ROUTE_OUTPUT, WINDOWS_TEST_ADDRESSES) - - def test_list_async_mock_cygwin(self): - self.patch(iputil, 'platform', "cygwin") - return self._test_list_async_mock(None, None, CYGWIN_TEST_ADDRESSES) class ListenOnUsed(unittest.TestCase): diff --git a/src/allmydata/util/iputil.py b/src/allmydata/util/iputil.py index bd5ea7e78..cbb4922e4 100644 --- a/src/allmydata/util/iputil.py +++ b/src/allmydata/util/iputil.py @@ -13,19 +13,19 @@ from future.utils import PY2, native_str if PY2: from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -import os, re, socket, subprocess, errno -from sys import platform +import os, socket from zope.interface import implementer import attr +from netifaces import ( + interfaces, + ifaddresses, +) + # from Twisted from twisted.python.reflect import requireModule -from twisted.internet import defer, threads, reactor -from twisted.internet.protocol import DatagramProtocol -from twisted.internet.error import CannotListenError -from twisted.python.procutils import which from twisted.python import log from twisted.internet.endpoints import AdoptedStreamServerEndpoint from twisted.internet.interfaces import ( @@ -101,180 +101,21 @@ except ImportError: # since one might be shadowing the other. This hack appeases pyflakes. increase_rlimits = _increase_rlimits + def get_local_addresses_sync(): """ - Return a list of IPv4 addresses (as dotted-quad native strings) that are - currently configured on this host, sorted in descending order of how likely - we think they are to work. + Get locally assigned addresses as dotted-quad native strings. + + :return [str]: A list of IPv4 addresses which are assigned to interfaces + on the local system. """ - return [native_str(a) for a in _synchronously_find_addresses_via_config()] - -def get_local_addresses_async(target="198.41.0.4"): # A.ROOT-SERVERS.NET - """ - Return a Deferred that fires with a list of IPv4 addresses (as dotted-quad - native strings) that are currently configured on this host, sorted in - descending order of how likely we think they are to work. - - @param target: we want to learn an IP address they could try using to - connect to us; The default value is fine, but it might help if you - pass the address of a host that you are actually trying to be - reachable to. - """ - addresses = [] - local_ip = get_local_ip_for(target) - if local_ip is not None: - addresses.append(local_ip) - - if platform == "cygwin": - d = _cygwin_hack_find_addresses() - else: - d = _find_addresses_via_config() - - def _collect(res): - for addr in res: - if addr != "0.0.0.0" and not addr in addresses: - addresses.append(addr) - return addresses - d.addCallback(_collect) - d.addCallback(lambda addresses: [native_str(s) for s in addresses]) - return d - -def get_local_ip_for(target): - """Find out what our IP address is for use by a given target. - - @return: the IP address as a dotted-quad native string which could be used - to connect to us. It might work for them, it might not. If - there is no suitable address (perhaps we don't currently have an - externally-visible interface), this will return None. - """ - - try: - target_ipaddr = socket.gethostbyname(target) - except socket.gaierror: - # DNS isn't running, or somehow we encountered an error - - # note: if an interface is configured and up, but nothing is - # connected to it, gethostbyname("A.ROOT-SERVERS.NET") will take 20 - # seconds to raise socket.gaierror . This is synchronous and occurs - # for each node being started, so users of - # test.common.SystemTestMixin (like test_system) will see something - # like 120s of delay, which may be enough to hit the default trial - # timeouts. For that reason, get_local_addresses_async() was changed - # to default to the numerical ip address for A.ROOT-SERVERS.NET, to - # avoid this DNS lookup. This also makes node startup fractionally - # faster. - return None - - try: - udpprot = DatagramProtocol() - port = reactor.listenUDP(0, udpprot) - try: - # connect() will fail if we're offline (e.g. running tests on a - # disconnected laptop), which is fine (localip=None), but we must - # still do port.stopListening() or we'll get a DirtyReactorError - udpprot.transport.connect(target_ipaddr, 7) - localip = udpprot.transport.getHost().host - return localip - finally: - d = port.stopListening() - d.addErrback(log.err) - except (socket.error, CannotListenError): - # no route to that host - localip = None - return native_str(localip) - - -# Wow, I'm really amazed at home much mileage we've gotten out of calling -# the external route.exe program on windows... It appears to work on all -# versions so far. -# ... thus wrote Greg Smith in time immemorial... -# Also, the Win32 APIs for this are really klunky and error-prone. --Daira - -_win32_re = re.compile(br'^\s*\d+\.\d+\.\d+\.\d+\s.+\s(?P
\d+\.\d+\.\d+\.\d+)\s+(?P\d+)\s*$', flags=re.M|re.I|re.S) -_win32_commands = (('route.exe', ('print',), _win32_re),) - -# These work in most Unices. -_addr_re = re.compile(br'^\s*inet [a-zA-Z]*:?(?P
\d+\.\d+\.\d+\.\d+)[\s/].+$', flags=re.M|re.I|re.S) -_unix_commands = (('/bin/ip', ('addr',), _addr_re), - ('/sbin/ip', ('addr',), _addr_re), - ('/sbin/ifconfig', ('-a',), _addr_re), - ('/usr/sbin/ifconfig', ('-a',), _addr_re), - ('/usr/etc/ifconfig', ('-a',), _addr_re), - ('ifconfig', ('-a',), _addr_re), - ('/sbin/ifconfig', (), _addr_re), - ) - - -def _find_addresses_via_config(): - return threads.deferToThread(_synchronously_find_addresses_via_config) - -def _synchronously_find_addresses_via_config(): - # originally by Greg Smith, hacked by Zooko and then Daira - - # We don't reach here for cygwin. - if platform == 'win32': - commands = _win32_commands - else: - commands = _unix_commands - - for (pathtotool, args, regex) in commands: - # If pathtotool is a fully qualified path then we just try that. - # If it is merely an executable name then we use Twisted's - # "which()" utility and try each executable in turn until one - # gives us something that resembles a dotted-quad IPv4 address. - - if os.path.isabs(pathtotool): - exes_to_try = [pathtotool] - else: - exes_to_try = which(pathtotool) - - subprocess_error = getattr( - subprocess, "SubprocessError", subprocess.CalledProcessError - ) - for exe in exes_to_try: - try: - addresses = _query(exe, args, regex) - except (IOError, OSError, ValueError, subprocess_error): - addresses = [] - if addresses: - return addresses - - return [] - -def _query(path, args, regex): - if not os.path.isfile(path): - return [] - env = {native_str('LANG'): native_str('en_US.UTF-8')} - TRIES = 5 - for trial in range(TRIES): - try: - p = subprocess.Popen([path] + list(args), stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) - (output, err) = p.communicate() - break - except OSError as e: - if e.errno == errno.EINTR and trial < TRIES-1: - continue - raise - - addresses = [] - outputsplit = output.split(b'\n') - for outline in outputsplit: - m = regex.match(outline) - if m: - addr = m.group('address') - if addr not in addresses: - addresses.append(addr.decode("utf-8")) - - return addresses - -def _cygwin_hack_find_addresses(): - addresses = [] - for h in ["localhost", "127.0.0.1",]: - addr = get_local_ip_for(h) - if addr is not None and addr not in addresses: - addresses.append(addr) - - return defer.succeed(addresses) + return list( + native_str(address[native_str("addr")]) + for iface_name + in interfaces() + for address + in ifaddresses(iface_name).get(socket.AF_INET) + ) def _foolscapEndpointForPortNumber(portnum): @@ -382,7 +223,5 @@ def listenOnUnused(tub, portnum=None): __all__ = ["allocate_tcp_port", "increase_rlimits", "get_local_addresses_sync", - "get_local_addresses_async", - "get_local_ip_for", "listenOnUnused", ] From c60d62d85832e137b6df07c63a76ff349e296eb4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:32:13 -0400 Subject: [PATCH 34/61] Direct test for the new implementation --- src/allmydata/test/test_iputil.py | 40 +++++++++++++++++++++++++++++++ src/allmydata/util/iputil.py | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index 6f6100410..54f414bfe 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -16,6 +16,13 @@ if PY2: import re, os, socket import gc +from testtools.matchers import ( + MatchesAll, + IsInstance, + AllMatch, + MatchesPredicate, +) + from twisted.trial import unittest from tenacity import retry, stop_after_attempt @@ -24,6 +31,13 @@ from foolscap.api import Tub from allmydata.util import iputil, gcutil +from ..util.iputil import ( + get_local_addresses_sync, +) + +from .common import ( + SyncTestCase, +) class ListenOnUsed(unittest.TestCase): """Tests for listenOnUnused.""" @@ -96,3 +110,29 @@ class GcUtil(unittest.TestCase): self.assertEqual(len(collections), 0) tracker.allocate() self.assertEqual(len(collections), 1) + + +class GetLocalAddressesSyncTests(SyncTestCase): + """ + Tests for ``get_local_addresses_sync``. + """ + def test_some_ipv4_addresses(self): + """ + ``get_local_addresses_sync`` returns a list of IPv4 addresses as native + strings. + """ + self.assertThat( + get_local_addresses_sync(), + MatchesAll( + IsInstance(list), + AllMatch( + MatchesAll( + IsInstance(native_str), + MatchesPredicate( + lambda addr: socket.inet_pton(socket.AF_INET, addr), + "%r is not an IPv4 address.", + ), + ), + ), + ), + ) diff --git a/src/allmydata/util/iputil.py b/src/allmydata/util/iputil.py index cbb4922e4..fd3e88c7f 100644 --- a/src/allmydata/util/iputil.py +++ b/src/allmydata/util/iputil.py @@ -114,7 +114,7 @@ def get_local_addresses_sync(): for iface_name in interfaces() for address - in ifaddresses(iface_name).get(socket.AF_INET) + in ifaddresses(iface_name).get(socket.AF_INET, []) ) From e170d8b881676b546e5e7f08cecdbd4580447018 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:33:20 -0400 Subject: [PATCH 35/61] Remove nettools (iproute2, ifconfig) from the NixOS packaging --- nix/tahoe-lafs.nix | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index a7f8fcbf7..302e1ff84 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -1,5 +1,5 @@ { fetchFromGitHub, lib -, nettools, python +, python , twisted, foolscap, zfec , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs @@ -41,10 +41,6 @@ python.pkgs.buildPythonPackage rec { ''; - propagatedNativeBuildInputs = [ - nettools - ]; - propagatedBuildInputs = with python.pkgs; [ twisted foolscap zfec appdirs setuptoolsTrial pyasn1 zope_interface From 682e91382fda7c9416c9c374b46bd81934d84439 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:36:26 -0400 Subject: [PATCH 36/61] *Add* the new dep to the NixOS packaging too --- nix/tahoe-lafs.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index 302e1ff84..a862781cf 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -3,7 +3,7 @@ , twisted, foolscap, zfec , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs -, beautifulsoup4, eliot, autobahn, cryptography +, beautifulsoup4, eliot, autobahn, cryptography, netifaces , html5lib, pyutil, distro }: python.pkgs.buildPythonPackage rec { @@ -45,7 +45,7 @@ python.pkgs.buildPythonPackage rec { twisted foolscap zfec appdirs setuptoolsTrial pyasn1 zope_interface service-identity pyyaml magic-wormhole treq - eliot autobahn cryptography setuptools + eliot autobahn cryptography netifaces setuptools future pyutil distro ]; From 96c848b2ad7413ef0a86bdb85f9675edbbbbc4a5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:50:31 -0400 Subject: [PATCH 37/61] flakes --- src/allmydata/test/test_iputil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index 54f414bfe..081c80ee3 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -13,7 +13,7 @@ from future.utils import PY2, native_str if PY2: from 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 re, os, socket +import os, socket import gc from testtools.matchers import ( From ed5d472209f63d83cfe5fc0bf8bb1e9ff4dbbacf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:52:07 -0400 Subject: [PATCH 38/61] news fragment --- newsfragments/3486.installation | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3486.installation diff --git a/newsfragments/3486.installation b/newsfragments/3486.installation new file mode 100644 index 000000000..7b24956b2 --- /dev/null +++ b/newsfragments/3486.installation @@ -0,0 +1 @@ +Tahoe-LAFS now requires the `netifaces` Python package and no longer requires the external `ip`, `ifconfig`, or `route.exe` executables. From 338ee89e63199f920c797a5c622763da0ea68570 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 23 Oct 2020 12:52:32 -0400 Subject: [PATCH 39/61] Stick with unicode (new str) for file path parts This seems to be the pattern, e.g.: https://github.com/tahoe-lafs/tahoe-lafs/blob/master/src/allmydata/client.py#L229 --- src/allmydata/immutable/offloaded.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index d04bf2894..d574b980d 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -670,7 +670,7 @@ class Helper(Referenceable): return (None, uh) def _make_chk_upload_helper(self, storage_index, lp): - si_s = si_b2a(storage_index) + si_s = si_b2a(storage_index).decode('ascii') incoming_file = os.path.join(self._chk_incoming, si_s) encoding_file = os.path.join(self._chk_encoding, si_s) uh = self.chk_upload( From 3e7968a480b9fa1ce1d1dde5fe454595b5513eae Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 14:52:45 -0400 Subject: [PATCH 40/61] Just do codechecks pre-push --- .pre-commit-config.yaml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 604614eb0..76162535a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,13 +3,7 @@ repos: hooks: - id: codechecks name: codechecks - stages: ["commit"] + stages: ["push"] entry: "tox -e codechecks" language: system pass_filenames: false - - id: test - name: test - stages: ["push"] - entry: "make test" - language: system - pass_filenames: false From 8dec56d7143aa763ec51d1b17fc289e6fe1b89ee Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 14:53:00 -0400 Subject: [PATCH 41/61] news fragment --- newsfragments/3488.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3488.minor diff --git a/newsfragments/3488.minor b/newsfragments/3488.minor new file mode 100644 index 000000000..e69de29bb From f50fd8e47406272f491349776df84b0d77960d67 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 11:30:12 -0400 Subject: [PATCH 42/61] Switch to new configparser backport. --- setup.py | 3 +++ src/allmydata/client.py | 7 +++---- src/allmydata/node.py | 5 +++-- src/allmydata/scripts/common.py | 4 +++- src/allmydata/storage_client.py | 10 ++++------ src/allmydata/util/configutil.py | 10 +++------- 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/setup.py b/setup.py index 874cc1258..14d249c51 100644 --- a/setup.py +++ b/setup.py @@ -131,6 +131,9 @@ install_requires = [ # Linux distribution detection: "distro >= 1.4.0", + + # Backported configparser for Python 2: + "configparser >= 5.0.1 ; python_version < '3.0'", ] setup_requires = [ diff --git a/src/allmydata/client.py b/src/allmydata/client.py index af3a17d48..a2ba80961 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -2,10 +2,9 @@ import os, stat, time, weakref from base64 import urlsafe_b64encode from functools import partial from errno import ENOENT, EPERM -try: - from ConfigParser import NoSectionError -except ImportError: - from configparser import NoSectionError + +# On Python 2 this will be the backported package: +from configparser import NoSectionError from foolscap.furl import ( decode_furl, diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 26fc6fc97..d3e84f213 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -13,8 +13,9 @@ from io import StringIO import tempfile from base64 import b32decode, b32encode -# Python 2 compatibility -from six.moves import configparser +# On Python 2 this will be the backported package. +import configparser + from future.utils import PY2 if PY2: from io import BytesIO as StringIO # noqa: F811 diff --git a/src/allmydata/scripts/common.py b/src/allmydata/scripts/common.py index b2bb8a1e6..34266ee72 100644 --- a/src/allmydata/scripts/common.py +++ b/src/allmydata/scripts/common.py @@ -8,7 +8,9 @@ from os.path import join from future.utils import PY2 if PY2: from future.builtins import str # noqa: F401 -from six.moves.configparser import NoSectionError + +# On Python 2 this will be the backported package: +from configparser import NoSectionError from twisted.python import usage diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 193f6d0f9..1e4497d68 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -31,12 +31,10 @@ the foolscap-based server implemented in src/allmydata/storage/*.py . from past.builtins import unicode import re, time, hashlib -try: - from ConfigParser import ( - NoSectionError, - ) -except ImportError: - from configparser import NoSectionError + +# On Python 2 this will be the backport. +from configparser import NoSectionError + import attr from zope.interface import ( Attribute, diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 1a1a93f18..97516d279 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -16,13 +16,9 @@ if PY2: # we do "r" or "w". from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -if PY2: - # In theory on Python 2 configparser also works, but then code gets the - # wrong exceptions and they don't get handled. So just use native parser - # for now. - from ConfigParser import SafeConfigParser -else: - from configparser import SafeConfigParser +# On Python 2 we use the backport package; that means we always get unicode +# out. +from configparser import SafeConfigParser import attr From 375ed5096c6931c9661946777384a5a909b4f019 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 12:12:49 -0400 Subject: [PATCH 43/61] Config parsing now always returns Unicode. --- src/allmydata/client.py | 17 +++++++---------- src/allmydata/frontends/ftpd.py | 3 +++ src/allmydata/util/configutil.py | 14 +++++++------- src/allmydata/webish.py | 4 ++++ 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index a2ba80961..3b63d8489 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -34,8 +34,7 @@ from allmydata.util import ( hashutil, base32, pollmixin, log, idlib, yamlutil, configutil, ) -from allmydata.util.encodingutil import (get_filesystem_encoding, - from_utf8_or_none) +from allmydata.util.encodingutil import get_filesystem_encoding from allmydata.util.abbreviate import parse_abbreviated_size from allmydata.util.time_format import parse_duration, parse_date from allmydata.util.i2p_provider import create as create_i2p_provider @@ -805,7 +804,7 @@ class _Client(node.Node, pollmixin.PollMixin): config_storedir = self.get_config( "storage", "storage_dir", self.STOREDIR, - ).decode('utf-8') + ) storedir = self.config.get_config_path(config_storedir) data = self.config.get_config("storage", "reserved_space", None) @@ -1042,15 +1041,14 @@ class _Client(node.Node, pollmixin.PollMixin): from allmydata.webish import WebishServer nodeurl_path = self.config.get_config_path("node.url") - staticdir_config = self.config.get_config("node", "web.static", "public_html").decode("utf-8") + 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.setServiceParent(self) def init_ftp_server(self): if self.config.get_config("ftpd", "enabled", False, boolean=True): - accountfile = from_utf8_or_none( - self.config.get_config("ftpd", "accounts.file", None)) + accountfile = self.config.get_config("ftpd", "accounts.file", None) if accountfile: accountfile = self.config.get_config_path(accountfile) accounturl = self.config.get_config("ftpd", "accounts.url", None) @@ -1062,14 +1060,13 @@ class _Client(node.Node, pollmixin.PollMixin): def init_sftp_server(self): if self.config.get_config("sftpd", "enabled", False, boolean=True): - accountfile = from_utf8_or_none( - self.config.get_config("sftpd", "accounts.file", None)) + accountfile = self.config.get_config("sftpd", "accounts.file", None) if accountfile: accountfile = self.config.get_config_path(accountfile) accounturl = self.config.get_config("sftpd", "accounts.url", None) sftp_portstr = self.config.get_config("sftpd", "port", "8022") - pubkey_file = from_utf8_or_none(self.config.get_config("sftpd", "host_pubkey_file")) - privkey_file = from_utf8_or_none(self.config.get_config("sftpd", "host_privkey_file")) + pubkey_file = self.config.get_config("sftpd", "host_pubkey_file") + privkey_file = self.config.get_config("sftpd", "host_privkey_file") from allmydata.frontends import sftpd s = sftpd.SFTPServer(self, accountfile, accounturl, diff --git a/src/allmydata/frontends/ftpd.py b/src/allmydata/frontends/ftpd.py index 3c1e91b90..0b18df85b 100644 --- a/src/allmydata/frontends/ftpd.py +++ b/src/allmydata/frontends/ftpd.py @@ -1,3 +1,4 @@ +from six import ensure_str from types import NoneType @@ -333,5 +334,7 @@ class FTPServer(service.MultiService): raise NeedRootcapLookupScheme("must provide some translation") f = ftp.FTPFactory(p) + # strports requires a native string. + ftp_portstr = ensure_str(ftp_portstr) s = strports.service(ftp_portstr, f) s.setServiceParent(self) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 97516d279..a41197b86 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -12,9 +12,7 @@ from __future__ import unicode_literals from future.utils import PY2 if PY2: - # We don't do open(), because we want files to read/write native strs when - # we do "r" or "w". - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + from 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 # On Python 2 we use the backport package; that means we always get unicode # out. @@ -38,10 +36,12 @@ def get_config(tahoe_cfg): """ config = SafeConfigParser() with open(tahoe_cfg, "r") as f: - # On Python 2, where we read in bytes, skip any initial Byte Order - # Mark. Since this is an ordinary file, we don't need to handle - # incomplete reads, and can assume seekability. - if PY2 and f.read(3) != b'\xEF\xBB\xBF': + # Who put the BOM in the BOM SHOO BOP SHOO BOP. + # + # Byte Order Mark is an optional garbage byte you sometimes get at the + # start of UTF-8 encoded files. Especially on Windows. Skip it. + # https://en.wikipedia.org/wiki/Byte_order_mark + if f.read(1) != u'\uFEFF': f.seek(0) config.readfp(f) return config diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index b07d06be1..12a5c197b 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,3 +1,5 @@ +from six import ensure_str + import re, time from functools import ( @@ -186,6 +188,8 @@ class WebishServer(service.MultiService): self.root.putChild("static", static.File(staticdir)) if re.search(r'^\d', webport): webport = "tcp:"+webport # twisted warns about bare "0" or "3456" + # strports must be native strings. + webport = ensure_str(webport) s = strports.service(webport, self.site) s.setServiceParent(self) From 76697c8c1bc673c7228f268d451afeda481e5854 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 14:33:59 -0400 Subject: [PATCH 44/61] It's already unicode. --- src/allmydata/test/test_node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index ecbf28d80..b9e9f9122 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -151,7 +151,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): f.close() config = read_config(basedir, "") - self.failUnlessEqual(config.get_config("node", "nickname").decode('utf-8'), + self.failUnlessEqual(config.get_config("node", "nickname"), u"\u2621") def test_tahoe_cfg_hash_in_name(self): From 7ed34168b2e98f77093fe212f38d382fd06141fc Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 16:08:38 -0400 Subject: [PATCH 45/61] News file. --- newsfragments/3485.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3485.minor diff --git a/newsfragments/3485.minor b/newsfragments/3485.minor new file mode 100644 index 000000000..e69de29bb From 957afa356cbe7c5ed203d0002bee27042ad4796b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 16:09:01 -0400 Subject: [PATCH 46/61] Add new dependency. --- nix/tahoe-lafs.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index a7f8fcbf7..9c6fed027 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -4,7 +4,7 @@ , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs , beautifulsoup4, eliot, autobahn, cryptography -, html5lib, pyutil, distro +, html5lib, pyutil, distro, configparser }: python.pkgs.buildPythonPackage rec { version = "1.14.0.dev"; @@ -50,7 +50,7 @@ python.pkgs.buildPythonPackage rec { setuptoolsTrial pyasn1 zope_interface service-identity pyyaml magic-wormhole treq eliot autobahn cryptography setuptools - future pyutil distro + future pyutil distro configparser ]; checkInputs = with python.pkgs; [ From 4dc1adc8179a16bdda712d6dd925bdcb14e88b69 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 16:37:00 -0400 Subject: [PATCH 47/61] Some progress towards passing Python 2 tests. --- src/allmydata/client.py | 3 +++ src/allmydata/node.py | 8 ++++++++ src/allmydata/test/test_configutil.py | 9 +++++++++ src/allmydata/util/configutil.py | 2 +- 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 3b63d8489..0cd8d6c39 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -717,6 +717,9 @@ class _Client(node.Node, pollmixin.PollMixin): def init_stats_provider(self): gatherer_furl = self.config.get_config("client", "stats_gatherer.furl", None) + if gatherer_furl: + # FURLs should be bytes: + gatherer_furl = gatherer_furl.encode("utf-8") self.stats_provider = StatsProvider(self, gatherer_furl) self.stats_provider.setServiceParent(self) self.stats_provider.register_producer(self) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index d3e84f213..1ffed6f45 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -3,6 +3,7 @@ This module contains classes and functions to implement and manage a node for Tahoe-LAFS. """ from past.builtins import unicode +from six import ensure_str import datetime import os.path @@ -640,6 +641,10 @@ def _tub_portlocation(config): new_locations.append(loc) location = ",".join(new_locations) + # Lacking this, Python 2 blows up in PB when it is confused by a Unicode + # FURL. + location = location.encode("utf-8") + return tubport, location @@ -687,6 +692,9 @@ def create_main_tub(config, tub_options, port_or_endpoint = tor_provider.get_listener() else: port_or_endpoint = port + # Foolscap requires native strings: + if isinstance(port_or_endpoint, (bytes, unicode)): + port_or_endpoint = ensure_str(port_or_endpoint) tub.listenOn(port_or_endpoint) tub.setLocation(location) log.msg("Tub location set to %s" % (location,)) diff --git a/src/allmydata/test/test_configutil.py b/src/allmydata/test/test_configutil.py index c57381289..9e792dc87 100644 --- a/src/allmydata/test/test_configutil.py +++ b/src/allmydata/test/test_configutil.py @@ -154,3 +154,12 @@ enabled = false self.dynamic_valid_config, ) self.assertIn("section [node] contains unknown option 'invalid'", str(e)) + + def test_duplicate_sections(self): + """ + Duplicate section names are merged. + """ + fname = self.create_tahoe_cfg('[node]\na = foo\n[node]\n b = bar\n') + config = configutil.get_config(fname) + self.assertEqual(config.get("node", "a"), "foo") + self.assertEqual(config.get("node", "b"), "bar") diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index a41197b86..41fe76b0d 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -34,7 +34,7 @@ def get_config(tahoe_cfg): Configuration is returned as native strings. """ - config = SafeConfigParser() + config = SafeConfigParser(strict=False) with open(tahoe_cfg, "r") as f: # Who put the BOM in the BOM SHOO BOP SHOO BOP. # From 4a54377208ffe4025a452209ec92321be199095a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 16:48:18 -0400 Subject: [PATCH 48/61] Some more fixes. --- src/allmydata/client.py | 4 ++++ src/allmydata/node.py | 1 + 2 files changed, 5 insertions(+) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 0cd8d6c39..9aba95426 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -936,6 +936,10 @@ class _Client(node.Node, pollmixin.PollMixin): if helper_furl in ("None", ""): helper_furl = None + # FURLs need to be bytes: + if helper_furl is not None: + helper_furl = helper_furl.encode("utf-8") + DEP = self.encoding_params DEP["k"] = int(self.config.get_config("client", "shares.needed", DEP["k"])) DEP["n"] = int(self.config.get_config("client", "shares.total", DEP["n"])) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 1ffed6f45..d42f3b59a 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -848,6 +848,7 @@ class Node(service.MultiService): lgfurl = self.config.get_config("node", "log_gatherer.furl", "") if lgfurl: # this is in addition to the contents of log-gatherer-furlfile + lgfurl = lgfurl.encode("utf-8") self.log_tub.setOption("log-gatherer-furl", lgfurl) self.log_tub.setOption("log-gatherer-furlfile", self.config.get_config_path("log_gatherer.furl")) From 4b7ab2bfd8c44943f58a63ccc69f328feb3df0a1 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 26 Oct 2020 16:54:19 -0400 Subject: [PATCH 49/61] Version that works with Python 2. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 0e4384f83..1247ad368 100644 --- a/setup.py +++ b/setup.py @@ -136,7 +136,7 @@ install_requires = [ "distro >= 1.4.0", # Backported configparser for Python 2: - "configparser >= 5.0.1 ; python_version < '3.0'", + "configparser >= 4.0.0 ; python_version < '3.0'", ] setup_requires = [ From dce8d3598aece5f9b98e7fa569aa297a331ee95f Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 08:54:16 -0400 Subject: [PATCH 50/61] Be even more lenient, in the hopes of working on Nix. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 1247ad368..c27681ea8 100644 --- a/setup.py +++ b/setup.py @@ -136,7 +136,7 @@ install_requires = [ "distro >= 1.4.0", # Backported configparser for Python 2: - "configparser >= 4.0.0 ; python_version < '3.0'", + "configparser ; python_version < '3.0'", ] setup_requires = [ From c76afc9ece4c5dcb573f9bafde17505034fee342 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 08:54:28 -0400 Subject: [PATCH 51/61] Try to fix some failing unit tests in ASCII locale. --- src/allmydata/node.py | 9 +++------ src/allmydata/test/test_connections.py | 6 +++++- src/allmydata/util/configutil.py | 14 +++++--------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index d42f3b59a..24addd3cd 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -10,17 +10,12 @@ import os.path import re import types import errno -from io import StringIO import tempfile from base64 import b32decode, b32encode # On Python 2 this will be the backported package. import configparser -from future.utils import PY2 -if PY2: - from io import BytesIO as StringIO # noqa: F811 - from twisted.python import log as twlog from twisted.application import service from twisted.python.failure import Failure @@ -213,7 +208,9 @@ def config_from_string(basedir, portnumfile, config_str, _valid_config=None): # load configuration from in-memory string parser = configparser.SafeConfigParser() - parser.readfp(StringIO(config_str)) + if isinstance(config_str, bytes): + config_str = config_str.decode("utf-8") + parser.read_string(config_str) fname = "" configutil.validate_config(fname, parser, _valid_config) diff --git a/src/allmydata/test/test_connections.py b/src/allmydata/test/test_connections.py index 9b5bd7f30..220815203 100644 --- a/src/allmydata/test/test_connections.py +++ b/src/allmydata/test/test_connections.py @@ -168,7 +168,11 @@ class Tor(unittest.TestCase): tor_provider = create_tor_provider(reactor, config) tor_provider.get_tor_handler() self.assertIn( - "invalid literal for int() with base 10: 'kumquat'", + "invalid literal for int()", + str(ctx.exception) + ) + self.assertIn( + "kumquat", str(ctx.exception) ) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 41fe76b0d..287620c2f 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -35,15 +35,11 @@ def get_config(tahoe_cfg): Configuration is returned as native strings. """ config = SafeConfigParser(strict=False) - with open(tahoe_cfg, "r") as f: - # Who put the BOM in the BOM SHOO BOP SHOO BOP. - # - # Byte Order Mark is an optional garbage byte you sometimes get at the - # start of UTF-8 encoded files. Especially on Windows. Skip it. - # https://en.wikipedia.org/wiki/Byte_order_mark - if f.read(1) != u'\uFEFF': - f.seek(0) - config.readfp(f) + # Byte Order Mark is an optional garbage byte you sometimes get at the + # start of UTF-8 encoded files. Especially on Windows. Skip it by using + # utf-8-sig. https://en.wikipedia.org/wiki/Byte_order_mark + with open(tahoe_cfg, "r", encoding="utf-8-sig") as f: + config.read_file(f) return config def set_config(config, section, option, value): From 900c25c78b0fadd23918fe5d71d5b7dda8912354 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Oct 2020 09:29:13 -0400 Subject: [PATCH 52/61] news fragment --- newsfragments/3490.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3490.minor diff --git a/newsfragments/3490.minor b/newsfragments/3490.minor new file mode 100644 index 000000000..e69de29bb From 03a6546f004128e27c5789b4fb239e4ecc199c98 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Oct 2020 09:29:24 -0400 Subject: [PATCH 53/61] flake8 integration --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 97ad3883a..a94740e9d 100644 --- a/tox.ini +++ b/tox.ini @@ -96,7 +96,7 @@ setenv = # `TypeError: decode() argument 1 must be string, not None` PYTHONIOENCODING=utf_8 commands = - flake8 src static misc setup.py + flake8 src integration static misc setup.py python misc/coding_tools/check-umids.py src python misc/coding_tools/check-debugging.py python misc/coding_tools/find-trailing-spaces.py -r src static misc setup.py From 1a7bb065871a3b240886a345894e22f0edc59e75 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 09:35:09 -0400 Subject: [PATCH 54/61] SafeConfigParser has been replaced by ConfigParser. --- src/allmydata/node.py | 4 ++-- src/allmydata/util/configutil.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 24addd3cd..1aa28a378 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -184,7 +184,7 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config=None): # (try to) read the main config file config_fname = os.path.join(basedir, "tahoe.cfg") - parser = configparser.SafeConfigParser() + parser = configparser.ConfigParser() try: parser = configutil.get_config(config_fname) except EnvironmentError as e: @@ -207,7 +207,7 @@ def config_from_string(basedir, portnumfile, config_str, _valid_config=None): _valid_config = _common_valid_config() # load configuration from in-memory string - parser = configparser.SafeConfigParser() + parser = configparser.ConfigParser() if isinstance(config_str, bytes): config_str = config_str.decode("utf-8") parser.read_string(config_str) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 287620c2f..a80921972 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -16,7 +16,7 @@ if PY2: # On Python 2 we use the backport package; that means we always get unicode # out. -from configparser import SafeConfigParser +from configparser import ConfigParser import attr @@ -30,11 +30,11 @@ class UnknownConfigError(Exception): def get_config(tahoe_cfg): - """Load the config, returning a SafeConfigParser. + """Load the config, returning a ConfigParser. Configuration is returned as native strings. """ - config = SafeConfigParser(strict=False) + config = ConfigParser(strict=False) # Byte Order Mark is an optional garbage byte you sometimes get at the # start of UTF-8 encoded files. Especially on Windows. Skip it by using # utf-8-sig. https://en.wikipedia.org/wiki/Byte_order_mark From b9f7bcab4e8f85b4e58b457969b81cc78e0c35cc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 27 Oct 2020 09:49:58 -0400 Subject: [PATCH 55/61] flakes --- integration/conftest.py | 2 -- integration/test_servers_of_happiness.py | 1 - integration/test_tor.py | 11 ++--------- integration/test_web.py | 8 ++------ 4 files changed, 4 insertions(+), 18 deletions(-) diff --git a/integration/conftest.py b/integration/conftest.py index 802deb79d..ca18230cd 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -15,7 +15,6 @@ from foolscap.furl import ( from eliot import ( to_file, log_call, - start_action, ) from twisted.python.procutils import which @@ -34,7 +33,6 @@ from util import ( _DumpOutputProtocol, _ProcessExitedProtocol, _create_node, - _run_node, _cleanup_tahoe_process, _tahoe_runner_optional_coverage, await_client_ready, diff --git a/integration/test_servers_of_happiness.py b/integration/test_servers_of_happiness.py index e5e4eb565..97392bf00 100644 --- a/integration/test_servers_of_happiness.py +++ b/integration/test_servers_of_happiness.py @@ -1,7 +1,6 @@ import sys from os.path import join -from twisted.internet import task from twisted.internet.error import ProcessTerminated import util diff --git a/integration/test_tor.py b/integration/test_tor.py index 28360207a..3d169a88f 100644 --- a/integration/test_tor.py +++ b/integration/test_tor.py @@ -1,15 +1,8 @@ from __future__ import print_function import sys -import time -import shutil -from os import mkdir, unlink, listdir -from os.path import join, exists -from six.moves import StringIO - -from twisted.internet.protocol import ProcessProtocol -from twisted.internet.error import ProcessExitedAlready, ProcessDone -from twisted.internet.defer import inlineCallbacks, Deferred +from os import mkdir +from os.path import join import pytest import pytest_twisted diff --git a/integration/test_web.py b/integration/test_web.py index 575e4fc1a..fe2137ff3 100644 --- a/integration/test_web.py +++ b/integration/test_web.py @@ -9,20 +9,15 @@ WebAPI *should* do in every situation. It's not clear the latter exists anywhere, however. """ -import sys import time -import shutil import json import urllib2 -from os import mkdir, unlink, utime -from os.path import join, exists, getmtime import allmydata.uri import util import requests -import pytest_twisted import html5lib from bs4 import BeautifulSoup @@ -265,7 +260,8 @@ def test_directory_deep_check(alice): dircap_url, params={u"t": u"json"}, ) - dir_meta = json.loads(resp.content) + # Just verify it is valid JSON. + json.loads(resp.content) # upload a file of pangrams into the directory FILE_CONTENTS = u"Sphinx of black quartz, judge my vow.\n" * (2048*10) From 207111fb9c54a7c5f4cf0646b90711a6fa0d84d1 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 11:43:27 -0400 Subject: [PATCH 56/61] Documentation fixes. --- src/allmydata/node.py | 4 ++-- src/allmydata/util/configutil.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 1aa28a378..ad76a64e1 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -638,8 +638,8 @@ def _tub_portlocation(config): new_locations.append(loc) location = ",".join(new_locations) - # Lacking this, Python 2 blows up in PB when it is confused by a Unicode - # FURL. + # Lacking this, Python 2 blows up in Foolscap when it is confused by a + # Unicode FURL. location = location.encode("utf-8") return tubport, location diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index a80921972..55769487f 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -1,7 +1,7 @@ """ Read/write config files. -Configuration is returned as native strings. +Configuration is returned as Unicode strings. Ported to Python 3. """ @@ -32,11 +32,11 @@ class UnknownConfigError(Exception): def get_config(tahoe_cfg): """Load the config, returning a ConfigParser. - Configuration is returned as native strings. + Configuration is returned as Unicode strings. """ config = ConfigParser(strict=False) - # Byte Order Mark is an optional garbage byte you sometimes get at the - # start of UTF-8 encoded files. Especially on Windows. Skip it by using + # Byte Order Mark is an optional garbage code point you sometimes get at + # the start of UTF-8 encoded files. Especially on Windows. Skip it by using # utf-8-sig. https://en.wikipedia.org/wiki/Byte_order_mark with open(tahoe_cfg, "r", encoding="utf-8-sig") as f: config.read_file(f) From 0d270e12907e5fedd7fa37bdcd13b15f1a67661e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 11:48:25 -0400 Subject: [PATCH 57/61] Create ConfigParsers in a consistent manner. --- src/allmydata/node.py | 5 +++-- src/allmydata/util/configutil.py | 10 +++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index ad76a64e1..2d58ea4fa 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -184,12 +184,13 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config=None): # (try to) read the main config file config_fname = os.path.join(basedir, "tahoe.cfg") - parser = configparser.ConfigParser() try: parser = configutil.get_config(config_fname) except EnvironmentError as e: if e.errno != errno.ENOENT: raise + # The file is missing, just create empty ConfigParser. + parser = configutil.create_parser() configutil.validate_config(config_fname, parser, _valid_config) @@ -207,7 +208,7 @@ def config_from_string(basedir, portnumfile, config_str, _valid_config=None): _valid_config = _common_valid_config() # load configuration from in-memory string - parser = configparser.ConfigParser() + parser = configutil.create_parser() if isinstance(config_str, bytes): config_str = config_str.decode("utf-8") parser.read_string(config_str) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 55769487f..898f05862 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -29,12 +29,20 @@ class UnknownConfigError(Exception): """ +def create_parser(): + """ + Create a ConfigParser pre-configured the way we want it, for consistency + across different code paths. + """ + return ConfigParser(strict=False) + + def get_config(tahoe_cfg): """Load the config, returning a ConfigParser. Configuration is returned as Unicode strings. """ - config = ConfigParser(strict=False) + config = create_parser() # Byte Order Mark is an optional garbage code point you sometimes get at # the start of UTF-8 encoded files. Especially on Windows. Skip it by using # utf-8-sig. https://en.wikipedia.org/wiki/Byte_order_mark From b79504a43b2c1057a0c0d566f3e25d088cd8e168 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 13:59:45 -0400 Subject: [PATCH 58/61] Refactor to unify different code paths. --- src/allmydata/node.py | 8 ++++---- src/allmydata/util/configutil.py | 24 +++++++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 2d58ea4fa..1574577d9 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -190,7 +190,7 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config=None): if e.errno != errno.ENOENT: raise # The file is missing, just create empty ConfigParser. - parser = configutil.create_parser() + parser = configutil.get_config_from_string(u"") configutil.validate_config(config_fname, parser, _valid_config) @@ -207,11 +207,11 @@ def config_from_string(basedir, portnumfile, config_str, _valid_config=None): if _valid_config is None: _valid_config = _common_valid_config() - # load configuration from in-memory string - parser = configutil.create_parser() if isinstance(config_str, bytes): config_str = config_str.decode("utf-8") - parser.read_string(config_str) + + # load configuration from in-memory string + parser = configutil.get_config_from_string(config_str) fname = "" configutil.validate_config(fname, parser, _valid_config) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index 898f05862..4bee7d043 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -29,26 +29,28 @@ class UnknownConfigError(Exception): """ -def create_parser(): - """ - Create a ConfigParser pre-configured the way we want it, for consistency - across different code paths. - """ - return ConfigParser(strict=False) - - def get_config(tahoe_cfg): """Load the config, returning a ConfigParser. Configuration is returned as Unicode strings. """ - config = create_parser() # Byte Order Mark is an optional garbage code point you sometimes get at # the start of UTF-8 encoded files. Especially on Windows. Skip it by using # utf-8-sig. https://en.wikipedia.org/wiki/Byte_order_mark with open(tahoe_cfg, "r", encoding="utf-8-sig") as f: - config.read_file(f) - return config + cfg_string = f.read() + return get_config_from_string(cfg_string) + + +def get_config_from_string(tahoe_cfg_string): + """Load the config from a string, return the ConfigParser. + + Configuration is returned as Unicode strings. + """ + parser = ConfigParser(strict=False) + parser.read_string(tahoe_cfg_string) + return parser + def set_config(config, section, option, value): if not config.has_section(section): From f1b281123d5d63ff38d2bdbbab430a4c21a72bed Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 14:24:20 -0400 Subject: [PATCH 59/61] Fix tests. --- src/allmydata/test/test_node.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 081106def..0d6c915b6 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -384,7 +384,7 @@ class TestMissingPorts(unittest.TestCase): with get_addr, alloc_port: tubport, tublocation = _tub_portlocation(config) self.assertEqual(tubport, "tcp:777") - self.assertEqual(tublocation, "tcp:LOCAL:777") + self.assertEqual(tublocation, b"tcp:LOCAL:777") def test_parsing_defaults(self): """ @@ -406,7 +406,7 @@ class TestMissingPorts(unittest.TestCase): with get_addr, alloc_port: tubport, tublocation = _tub_portlocation(config) self.assertEqual(tubport, "tcp:999") - self.assertEqual(tublocation, "tcp:LOCAL:999") + self.assertEqual(tublocation, b"tcp:LOCAL:999") def test_parsing_location_complex(self): """ @@ -429,7 +429,7 @@ class TestMissingPorts(unittest.TestCase): with get_addr, alloc_port: tubport, tublocation = _tub_portlocation(config) self.assertEqual(tubport, "tcp:999") - self.assertEqual(tublocation, "tcp:HOST:888,tcp:LOCAL:999") + self.assertEqual(tublocation, b"tcp:HOST:888,tcp:LOCAL:999") def test_parsing_all_disabled(self): """ From 20a64d57674686edcc9cfea78a9b166c3e1882c4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 14:24:23 -0400 Subject: [PATCH 60/61] Test new fileutil behavior. --- src/allmydata/test/test_util.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py index 6c64174bc..c671caa31 100644 --- a/src/allmydata/test/test_util.py +++ b/src/allmydata/test/test_util.py @@ -413,6 +413,16 @@ class FileUtil(ReallyEqualMixin, unittest.TestCase): f.write(b"foobar") f.close() + def test_write(self): + """fileutil.write() can write both unicode and bytes.""" + path = self.mktemp() + fileutil.write(path, b"abc") + with open(path, "rb") as f: + self.assertEqual(f.read(), b"abc") + fileutil.write(path, u"def \u1234") + with open(path, "rb") as f: + self.assertEqual(f.read(), u"def \u1234".encode("utf-8")) + class PollMixinTests(unittest.TestCase): def setUp(self): From 6f2027e824a5bbbafcb8a5821a7917783a8cc592 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 27 Oct 2020 16:59:43 -0400 Subject: [PATCH 61/61] Fix lint. --- src/allmydata/util/configutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/configutil.py b/src/allmydata/util/configutil.py index ae3052723..4bee7d043 100644 --- a/src/allmydata/util/configutil.py +++ b/src/allmydata/util/configutil.py @@ -10,7 +10,7 @@ from __future__ import division from __future__ import print_function from __future__ import unicode_literals -from future.utils import PY2, PY3 +from future.utils import PY2 if PY2: from 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