From 947cb1c11bcb1c57ec3372eeefc0b59b88955f03 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 14 Sep 2020 14:40:02 -0400 Subject: [PATCH 01/12] Tiny bit more test coverage for server.py. --- src/allmydata/test/test_storage.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 4bbbcda30..080ddc0ef 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -702,6 +702,12 @@ class Server(unittest.TestCase): leases = list(ss.get_leases(b"si3")) self.failUnlessEqual(len(leases), 2) + def test_have_shares(self): + """By default the StorageServer has no shares.""" + workdir = self.workdir("test_have_shares") + ss = StorageServer(workdir, b"\x00" * 20, readonly_storage=True) + self.assertFalse(ss.have_shares()) + def test_readonly(self): workdir = self.workdir("test_readonly") ss = StorageServer(workdir, b"\x00" * 20, readonly_storage=True) From d84a7a61f30e181d83a00665ea32c9f498525b1a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 14 Sep 2020 14:46:08 -0400 Subject: [PATCH 02/12] Port to Python 3. --- src/allmydata/storage/server.py | 17 ++++++++++++++--- src/allmydata/util/_python3.py | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index c044a9fb0..2ba02aa9a 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -1,4 +1,15 @@ -from future.utils import bytes_to_native_str +from __future__ import division +from __future__ import absolute_import +from __future__ import print_function +from __future__ import unicode_literals + +from past.utils import old_div +from future.utils import bytes_to_native_str, PY2 +if PY2: + # Omit open() to get native behavior where open("w") always accepts native strings. + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + + import os, re, struct, time import weakref import six @@ -146,7 +157,7 @@ class StorageServer(service.MultiService, Referenceable): stats["samplesize"] = count samples.sort() if count > 1: - stats["mean"] = sum(samples) / count + stats["mean"] = old_div(sum(samples), count) else: stats["mean"] = None @@ -671,7 +682,7 @@ class StorageServer(service.MultiService, Referenceable): filename = os.path.join(bucketdir, sharenum_s) msf = MutableShareFile(filename, self) datavs[sharenum] = msf.readv(readv) - log.msg("returning shares %s" % (datavs.keys(),), + log.msg("returning shares %s" % (list(datavs.keys()),), facility="tahoe.storage", level=log.NOISY, parent=lp) self.add_latency("readv", time.time() - start) return datavs diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index afdbea1f0..c707356a1 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -37,6 +37,7 @@ PORTED_MODULES = [ "allmydata.monitor", "allmydata.storage.crawler", "allmydata.storage.expirer", + "allmydata.storage.server", "allmydata.test.common_py3", "allmydata.uri", "allmydata.util._python3", From 31aa594290cc3794de11e033a8db970b5e5d225a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 14 Sep 2020 14:47:26 -0400 Subject: [PATCH 03/12] Looks like float is fine for mean. --- src/allmydata/storage/server.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 2ba02aa9a..e36e4a2f1 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -1,9 +1,11 @@ +""" +Ported to Python 3. +""" from __future__ import division from __future__ import absolute_import from __future__ import print_function from __future__ import unicode_literals -from past.utils import old_div from future.utils import bytes_to_native_str, PY2 if PY2: # Omit open() to get native behavior where open("w") always accepts native strings. @@ -157,7 +159,7 @@ class StorageServer(service.MultiService, Referenceable): stats["samplesize"] = count samples.sort() if count > 1: - stats["mean"] = old_div(sum(samples), count) + stats["mean"] = sum(samples) / count else: stats["mean"] = None From 72f72491d0adf05d6f0208d254fb750116d5a357 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 14 Sep 2020 14:48:39 -0400 Subject: [PATCH 04/12] News fragment. --- newsfragments/3415.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3415.minor diff --git a/newsfragments/3415.minor b/newsfragments/3415.minor new file mode 100644 index 000000000..e69de29bb From bea1d657f32a8a4191e902e05a072980f076392b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 16 Sep 2020 14:37:11 -0400 Subject: [PATCH 05/12] Better debug output. --- src/allmydata/test/common_py3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/common_py3.py b/src/allmydata/test/common_py3.py index 50fb02ff7..b49078f93 100644 --- a/src/allmydata/test/common_py3.py +++ b/src/allmydata/test/common_py3.py @@ -123,7 +123,7 @@ class ShouldFailMixin(object): class ReallyEqualMixin(object): def failUnlessReallyEqual(self, a, b, msg=None): self.assertEqual(a, b, msg) - self.assertEqual(type(a), type(b), "a :: %r, b :: %r, %r" % (a, b, msg)) + self.assertEqual(type(a), type(b), "a :: %r (%s), b :: %r (%s), %r" % (a, b, type(a), type(b), msg)) def skip_if_cannot_represent_filename(u): From e8743a607fb978d59c2e2a9cb45fc1beb62dec86 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 16 Sep 2020 14:37:16 -0400 Subject: [PATCH 06/12] Fix failing tests. --- src/allmydata/storage/server.py | 27 +++++++++++++++------------ src/allmydata/test/test_deepcheck.py | 3 ++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index e36e4a2f1..845d3ac1a 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -6,10 +6,11 @@ from __future__ import absolute_import from __future__ import print_function from __future__ import unicode_literals -from future.utils import bytes_to_native_str, PY2 +from future.utils import bytes_to_native_str, PY2, native_str_to_bytes if PY2: - # Omit open() to get native behavior where open("w") always accepts native strings. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + # Omit open() to get native behavior where open("w") always accepts native + # strings. Omit bytes so we don't leak future's custom bytes. + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, dict, list, object, range, str, max, min # noqa: F401 import os, re, struct, time @@ -241,16 +242,18 @@ class StorageServer(service.MultiService, Referenceable): # We're on a platform that has no API to get disk stats. remaining_space = 2**64 - version = { "http://allmydata.org/tahoe/protocols/storage/v1" : - { "maximum-immutable-share-size": remaining_space, - "maximum-mutable-share-size": MAX_MUTABLE_SHARE_SIZE, - "available-space": remaining_space, - "tolerates-immutable-read-overrun": True, - "delete-mutable-shares-with-zero-length-writev": True, - "fills-holes-with-zero-bytes": True, - "prevents-read-past-end-of-share-data": True, + # Unicode strings might be nicer, but for now sticking to bytes since + # this is what the wire protocol has always been. + version = { b"http://allmydata.org/tahoe/protocols/storage/v1" : + { b"maximum-immutable-share-size": remaining_space, + b"maximum-mutable-share-size": MAX_MUTABLE_SHARE_SIZE, + b"available-space": remaining_space, + b"tolerates-immutable-read-overrun": True, + b"delete-mutable-shares-with-zero-length-writev": True, + b"fills-holes-with-zero-bytes": True, + b"prevents-read-past-end-of-share-data": True, }, - "application-version": str(allmydata.__full_version__), + b"application-version": allmydata.__full_version__.encode("utf-8"), } return version diff --git a/src/allmydata/test/test_deepcheck.py b/src/allmydata/test/test_deepcheck.py index 90a27b424..ea3ba6338 100644 --- a/src/allmydata/test/test_deepcheck.py +++ b/src/allmydata/test/test_deepcheck.py @@ -1,3 +1,4 @@ +from future.utils import native_str import os, json, urllib from twisted.trial import unittest @@ -945,7 +946,7 @@ class DeepCheckWebBad(DeepCheckBase, unittest.TestCase): def _corrupt_some_shares(self, node): for (shnum, serverid, sharefile) in self.find_uri_shares(node.get_uri()): if shnum in (0,1): - yield run_cli("debug", "corrupt-share", sharefile) + yield run_cli("debug", "corrupt-share", native_str(sharefile)) def _delete_most_shares(self, node): self.delete_shares_numbered(node.get_uri(), range(1,10)) From cecbc260fa11eedbdb62959f09bf85c3a533db22 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 11:43:35 -0400 Subject: [PATCH 07/12] Fix order. --- src/allmydata/test/common_py3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/common_py3.py b/src/allmydata/test/common_py3.py index b49078f93..5b791fd0a 100644 --- a/src/allmydata/test/common_py3.py +++ b/src/allmydata/test/common_py3.py @@ -123,7 +123,7 @@ class ShouldFailMixin(object): class ReallyEqualMixin(object): def failUnlessReallyEqual(self, a, b, msg=None): self.assertEqual(a, b, msg) - self.assertEqual(type(a), type(b), "a :: %r (%s), b :: %r (%s), %r" % (a, b, type(a), type(b), msg)) + self.assertEqual(type(a), type(b), "a :: %r (%s), b :: %r (%s), %r" % (a, type(a), b, type(b), msg)) def skip_if_cannot_represent_filename(u): From 03fd566e2c1f9d25332643d8d71de8f85b2533d1 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 12:37:10 -0400 Subject: [PATCH 08/12] Fix flake error. --- src/allmydata/storage/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 845d3ac1a..14eb08f84 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -6,7 +6,7 @@ from __future__ import absolute_import from __future__ import print_function from __future__ import unicode_literals -from future.utils import bytes_to_native_str, PY2, native_str_to_bytes +from future.utils import bytes_to_native_str, PY2 if PY2: # Omit open() to get native behavior where open("w") always accepts native # strings. Omit bytes so we don't leak future's custom bytes. From 98185128024d7d480d41327f97139591a6228431 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 13:10:52 -0400 Subject: [PATCH 09/12] Fix newbytes leak. --- src/allmydata/util/hashutil.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py index 96d52c862..46f361287 100644 --- a/src/allmydata/util/hashutil.py +++ b/src/allmydata/util/hashutil.py @@ -10,7 +10,8 @@ from __future__ import unicode_literals 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 + # Don't import bytes to prevent leaking future's bytes. + from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, str, max, min # noqa: F401 from past.builtins import chr as byteschr From 6c85f392dd1472ff48e10a4df3c9179f2ea1d9fb Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 13:39:47 -0400 Subject: [PATCH 10/12] Fix another future newbytes leak that was breaking Foolscap. --- src/allmydata/uri.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/allmydata/uri.py b/src/allmydata/uri.py index 253cb109c..bad0dd9df 100644 --- a/src/allmydata/uri.py +++ b/src/allmydata/uri.py @@ -13,8 +13,9 @@ from __future__ import unicode_literals from future.utils import PY2 if PY2: - # Don't import bytes, to prevent leaks. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, str, max, min # noqa: F401 + # Don't import bytes or str, to prevent leaks. + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, max, min # noqa: F401 + str = unicode from past.builtins import unicode, long From 3d79793ee855b8e385797fcc72ed5e38f0789d03 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 15:38:08 -0400 Subject: [PATCH 11/12] Try to fix hashutil. --- src/allmydata/util/hashutil.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py index 46f361287..ebb2f12af 100644 --- a/src/allmydata/util/hashutil.py +++ b/src/allmydata/util/hashutil.py @@ -11,7 +11,9 @@ from __future__ import unicode_literals from future.utils import PY2 if PY2: # Don't import bytes to prevent leaking future's bytes. - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, 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, dict, list, object, range, str, max, min, bytes as future_bytes # noqa: F401 +else: + future_bytes = bytes from past.builtins import chr as byteschr @@ -214,7 +216,7 @@ def bucket_cancel_secret_hash(file_cancel_secret, peerid): def _xor(a, b): - return b"".join([byteschr(c ^ b) for c in a]) + return b"".join([byteschr(c ^ b) for c in future_bytes(a)]) def hmac(tag, data): From 02cb451a6b97a992f11b8edaed1103eb2749646d Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 17 Sep 2020 16:06:26 -0400 Subject: [PATCH 12/12] Fix failing tests. --- src/allmydata/test/test_storage.py | 18 +++++++++--------- src/allmydata/util/hashutil.py | 6 ++++-- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 1f5f91844..b9e3d00c6 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -366,21 +366,21 @@ class Server(unittest.TestCase): def test_declares_fixed_1528(self): ss = self.create("test_declares_fixed_1528") ver = ss.remote_get_version() - sv1 = ver['http://allmydata.org/tahoe/protocols/storage/v1'] - self.failUnless(sv1.get('prevents-read-past-end-of-share-data'), sv1) + sv1 = ver[b'http://allmydata.org/tahoe/protocols/storage/v1'] + self.failUnless(sv1.get(b'prevents-read-past-end-of-share-data'), sv1) def test_declares_maximum_share_sizes(self): ss = self.create("test_declares_maximum_share_sizes") ver = ss.remote_get_version() - sv1 = ver['http://allmydata.org/tahoe/protocols/storage/v1'] - self.failUnlessIn('maximum-immutable-share-size', sv1) - self.failUnlessIn('maximum-mutable-share-size', sv1) + sv1 = ver[b'http://allmydata.org/tahoe/protocols/storage/v1'] + self.failUnlessIn(b'maximum-immutable-share-size', sv1) + self.failUnlessIn(b'maximum-mutable-share-size', sv1) def test_declares_available_space(self): ss = self.create("test_declares_available_space") ver = ss.remote_get_version() - sv1 = ver['http://allmydata.org/tahoe/protocols/storage/v1'] - self.failUnlessIn('available-space', sv1) + sv1 = ver[b'http://allmydata.org/tahoe/protocols/storage/v1'] + self.failUnlessIn(b'available-space', sv1) def allocate(self, ss, storage_index, sharenums, size, canary=None): renew_secret = hashutil.tagged_hash(b"blah", b"%d" % next(self._lease_secret)) @@ -980,8 +980,8 @@ class MutableServer(unittest.TestCase): # Also see if the server explicitly declares that it supports this # feature. ver = ss.remote_get_version() - storage_v1_ver = ver["http://allmydata.org/tahoe/protocols/storage/v1"] - self.failUnless(storage_v1_ver.get("fills-holes-with-zero-bytes")) + storage_v1_ver = ver[b"http://allmydata.org/tahoe/protocols/storage/v1"] + self.failUnless(storage_v1_ver.get(b"fills-holes-with-zero-bytes")) # If the size is dropped to zero the share is deleted. answer = rstaraw(b"si1", secrets, diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py index 46f361287..ebb2f12af 100644 --- a/src/allmydata/util/hashutil.py +++ b/src/allmydata/util/hashutil.py @@ -11,7 +11,9 @@ from __future__ import unicode_literals from future.utils import PY2 if PY2: # Don't import bytes to prevent leaking future's bytes. - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, 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, dict, list, object, range, str, max, min, bytes as future_bytes # noqa: F401 +else: + future_bytes = bytes from past.builtins import chr as byteschr @@ -214,7 +216,7 @@ def bucket_cancel_secret_hash(file_cancel_secret, peerid): def _xor(a, b): - return b"".join([byteschr(c ^ b) for c in a]) + return b"".join([byteschr(c ^ b) for c in future_bytes(a)]) def hmac(tag, data):