From 1197b1510642d8e7fa329fb850b8ca9f92ccfe5c Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 11 May 2016 12:54:11 -0700 Subject: [PATCH 1/6] introducer_client: split out _deliver_announcements --- src/allmydata/introducer/client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/introducer/client.py b/src/allmydata/introducer/client.py index b0128fe63..42ebc58c5 100644 --- a/src/allmydata/introducer/client.py +++ b/src/allmydata/introducer/client.py @@ -359,6 +359,10 @@ class IntroducerClient(service.Service, Referenceable): self._save_announcements() # note: we never forget an index, but we might update its value + self._deliver_announcements(key_s, ann) + + def _deliver_announcements(self, key_s, ann): + service_name = str(ann["service-name"]) for (service_name2,cb,args,kwargs) in self._local_subscribers: if service_name2 == service_name: eventually(cb, key_s, ann, *args, **kwargs) From ecec58b339cf54dbd25cd85213da66e4e9a24e44 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 11 May 2016 13:20:22 -0700 Subject: [PATCH 2/6] test_introducer: factor out _load_cache --- src/allmydata/test/test_introducer.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py index 22f5e9e30..8a774aafe 100644 --- a/src/allmydata/test/test_introducer.py +++ b/src/allmydata/test/test_introducer.py @@ -1008,6 +1008,13 @@ class Announcements(unittest.TestCase): self.failUnlessEqual(a[0].version, "my_version") self.failUnlessEqual(a[0].announcement["anonymous-storage-FURL"], furl1) + def _load_cache(self, cache_filepath): + with cache_filepath.open() as f: + def constructor(loader, node): + return node.value + yaml.SafeLoader.add_constructor("tag:yaml.org,2002:python/unicode", constructor) + return yaml.safe_load(f) + def test_client_cache_1(self): basedir = "introducer/ClientSeqnums/test_client_cache_1" fileutil.make_dirs(basedir) @@ -1036,13 +1043,7 @@ class Announcements(unittest.TestCase): ic.got_announcements([ann_t]) # check the cache for the announcement - with cache_filepath.open() as f: - def constructor(loader, node): - return node.value - yaml.SafeLoader.add_constructor("tag:yaml.org,2002:python/unicode", constructor) - announcements = yaml.safe_load(f) - f.close() - + announcements = self._load_cache(cache_filepath) self.failUnlessEqual(len(announcements), 1) self.failUnlessEqual("pub-" + announcements[0]['key_s'], vk_s) From cfb939aa99fd1a1bf424fa8988ea169b98623aaf Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 11 May 2016 13:30:39 -0700 Subject: [PATCH 3/6] improve test_client_cache --- src/allmydata/test/test_introducer.py | 38 ++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py index 8a774aafe..a361fd8af 100644 --- a/src/allmydata/test/test_introducer.py +++ b/src/allmydata/test/test_introducer.py @@ -1015,7 +1015,7 @@ class Announcements(unittest.TestCase): yaml.SafeLoader.add_constructor("tag:yaml.org,2002:python/unicode", constructor) return yaml.safe_load(f) - def test_client_cache_1(self): + def test_client_cache(self): basedir = "introducer/ClientSeqnums/test_client_cache_1" fileutil.make_dirs(basedir) cache_filepath = FilePath(os.path.join(basedir, "private", @@ -1036,7 +1036,7 @@ class Announcements(unittest.TestCase): ic = c.introducer_client sk_s, vk_s = keyutil.make_keypair() sk, _ignored = keyutil.parse_privkey(sk_s) - keyutil.remove_prefix(vk_s, "pub-v0-") + pub1 = keyutil.remove_prefix(vk_s, "pub-") furl1 = "pb://onug64tu@127.0.0.1:123/short" # base32("short") ann_t = make_ann_t(ic, furl1, sk, 1) @@ -1045,7 +1045,39 @@ class Announcements(unittest.TestCase): # check the cache for the announcement announcements = self._load_cache(cache_filepath) self.failUnlessEqual(len(announcements), 1) - self.failUnlessEqual("pub-" + announcements[0]['key_s'], vk_s) + self.failUnlessEqual(announcements[0]['key_s'], pub1) + ann = announcements[0]["ann"] + self.failUnlessEqual(ann["anonymous-storage-FURL"], furl1) + self.failUnlessEqual(ann["seqnum"], 1) + + # a new announcement that replaces the first should replace the + # cached entry, not duplicate it + furl2 = furl1 + "er" + ann_t2 = make_ann_t(ic, furl2, sk, 2) + ic.got_announcements([ann_t2]) + announcements = self._load_cache(cache_filepath) + self.failUnlessEqual(len(announcements), 1) + self.failUnlessEqual(announcements[0]['key_s'], pub1) + ann = announcements[0]["ann"] + self.failUnlessEqual(ann["anonymous-storage-FURL"], furl2) + self.failUnlessEqual(ann["seqnum"], 2) + + # but a third announcement with a different key should add to the + # cache + sk_s2, vk_s2 = keyutil.make_keypair() + sk2, _ignored = keyutil.parse_privkey(sk_s2) + pub2 = keyutil.remove_prefix(vk_s2, "pub-") + furl3 = "pb://onug64tu@127.0.0.1:456/short" + ann_t3 = make_ann_t(ic, furl3, sk2, 1) + ic.got_announcements([ann_t3]) + + announcements = self._load_cache(cache_filepath) + self.failUnlessEqual(len(announcements), 2) + self.failUnlessEqual(set([pub1, pub2]), + set([a["key_s"] for a in announcements])) + self.failUnlessEqual(set([furl2, furl3]), + set([a["ann"]["anonymous-storage-FURL"] + for a in announcements])) class ClientSeqnums(unittest.TestCase): From 5508f751b60d83fd6d71ec4eef86a3566ea41d09 Mon Sep 17 00:00:00 2001 From: David Stainton Date: Wed, 11 May 2016 09:22:47 +0000 Subject: [PATCH 4/6] Load announcement cache if failure to connect to introducer --- src/allmydata/introducer/client.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/allmydata/introducer/client.py b/src/allmydata/introducer/client.py index 42ebc58c5..a2998e17d 100644 --- a/src/allmydata/introducer/client.py +++ b/src/allmydata/introducer/client.py @@ -111,9 +111,29 @@ class IntroducerClient(service.Service, Referenceable): def connect_failed(failure): self.log("Initial Introducer connection failed: perhaps it's down", level=log.WEIRD, failure=failure, umid="c5MqUQ") + self._load_announcements() d = self._tub.getReference(self.introducer_furl) d.addErrback(connect_failed) + def _load_announcements(self): + if self._cache_filepath.exists(): + with self._cache_filepath.open() as f: + servers = yaml.load(f) + f.close() + if not isinstance(servers, list): + msg = "Invalid cached storage server announcements. No list encountered." + self.log(msg, + level=log.WEIRD) + raise storage_client.UnknownServerTypeError(msg) + for server_params in servers: + if not isinstance(server_params, dict): + msg = "Invalid cached storage server announcement encountered. No key/values found in %s" % server_params + self.log(msg, + level=log.WEIRD) + raise storage_client.UnknownServerTypeError(msg) + for _, cb, _, _ in self._local_subscribers: + eventually(cb, server_params['key_s'], server_params['ann']) + def _save_announcements(self): announcements = [] for _, value in self._inbound_announcements.items(): From 5bedca43e35d4752db517f8ef5facf7d2e2f9172 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 11 May 2016 13:15:41 -0700 Subject: [PATCH 5/6] load-yaml-cache cleanups * use yaml.safe_load and yaml.safe_dump * configure SafeLoader to return unicode consistently, not str * log+ignore bad cache, instead of throwing error, since we're already in the log+ignore chain from connect_failed() * use a local exception type, instead of one from storage_client.py * delegate delivery to self._deliver_announcements Using yaml.safe_dump gives us: - ann: my-version: tahoe-lafs/1.11.0.post96.dev0 nickname: node-4 instead of: - ann: !!python/unicode 'my-version': !!python/unicode 'tahoe-lafs/1.11.0.post96.dev0' !!python/unicode 'nickname': !!python/unicode 'node-4' We want SafeLoader to consistently return unicode instead of sometimes plain strings (for ASCII-safe values) and sometimes unicode (for everything else). The data we write into the cache was all unicode to start with (it came from a JSON parser), so it seems better to get back unicode too. --- src/allmydata/introducer/client.py | 42 ++++++++++++++++----------- src/allmydata/test/test_introducer.py | 19 ++++++++++-- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/allmydata/introducer/client.py b/src/allmydata/introducer/client.py index a2998e17d..b042ca2d5 100644 --- a/src/allmydata/introducer/client.py +++ b/src/allmydata/introducer/client.py @@ -13,6 +13,9 @@ from allmydata.util import log from allmydata.util.rrefutil import add_version_to_remote_reference from allmydata.util.keyutil import BadSignatureError +class InvalidCacheError(Exception): + pass + class WrapV2ClientInV1Interface(Referenceable): # for_v1 """I wrap a v2 IntroducerClient to make it look like a v1 client, so it can be attached to an old server.""" @@ -116,23 +119,28 @@ class IntroducerClient(service.Service, Referenceable): d.addErrback(connect_failed) def _load_announcements(self): - if self._cache_filepath.exists(): + # Announcements contain unicode, because they come from JSON. We tell + # PyYAML to give us unicode instead of str/bytes. + def construct_unicode(loader, node): + return node.value + yaml.SafeLoader.add_constructor("tag:yaml.org,2002:str", + construct_unicode) + try: with self._cache_filepath.open() as f: - servers = yaml.load(f) - f.close() - if not isinstance(servers, list): - msg = "Invalid cached storage server announcements. No list encountered." - self.log(msg, - level=log.WEIRD) - raise storage_client.UnknownServerTypeError(msg) - for server_params in servers: - if not isinstance(server_params, dict): - msg = "Invalid cached storage server announcement encountered. No key/values found in %s" % server_params - self.log(msg, - level=log.WEIRD) - raise storage_client.UnknownServerTypeError(msg) - for _, cb, _, _ in self._local_subscribers: - eventually(cb, server_params['key_s'], server_params['ann']) + servers = yaml.safe_load(f) + except EnvironmentError: + return # no cache file + if not isinstance(servers, list): + log.err(InvalidCacheError("not a list"), level=log.WEIRD) + return + self.log("Using server data from cache", level=log.UNUSUAL) + for server_params in servers: + if not isinstance(server_params, dict): + log.err(InvalidCacheError("not a dict: %r" % (server_params,)), + level=log.WEIRD) + continue + self._deliver_announcements(server_params['key_s'], + server_params['ann']) def _save_announcements(self): announcements = [] @@ -143,7 +151,7 @@ class IntroducerClient(service.Service, Referenceable): "key_s" : key_s, } announcements.append(server_params) - announcement_cache_yaml = yaml.dump(announcements) + announcement_cache_yaml = yaml.safe_dump(announcements) self._cache_filepath.setContent(announcement_cache_yaml) def _got_introducer(self, publisher): diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py index a361fd8af..af04d1577 100644 --- a/src/allmydata/test/test_introducer.py +++ b/src/allmydata/test/test_introducer.py @@ -1009,10 +1009,11 @@ class Announcements(unittest.TestCase): self.failUnlessEqual(a[0].announcement["anonymous-storage-FURL"], furl1) def _load_cache(self, cache_filepath): + def construct_unicode(loader, node): + return node.value + yaml.SafeLoader.add_constructor("tag:yaml.org,2002:str", + construct_unicode) with cache_filepath.open() as f: - def constructor(loader, node): - return node.value - yaml.SafeLoader.add_constructor("tag:yaml.org,2002:python/unicode", constructor) return yaml.safe_load(f) def test_client_cache(self): @@ -1080,6 +1081,18 @@ class Announcements(unittest.TestCase): for a in announcements])) +class YAMLUnicode(unittest.TestCase): + def test_convert(self): + data = yaml.safe_dump(["str", u"unicode", u"\u1234nicode"]) + def construct_unicode(loader, node): + return node.value + yaml.SafeLoader.add_constructor("tag:yaml.org,2002:str", + construct_unicode) + back = yaml.safe_load(data) + self.failUnlessEqual(type(back[0]), unicode) + self.failUnlessEqual(type(back[1]), unicode) + self.failUnlessEqual(type(back[2]), unicode) + class ClientSeqnums(unittest.TestCase): def test_client(self): basedir = "introducer/ClientSeqnums/test_client" From 0ff00dff6f7cc44cf54759071f975693493f0507 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 11 May 2016 16:41:52 -0700 Subject: [PATCH 6/6] test loading the cache --- src/allmydata/test/test_introducer.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py index af04d1577..4feb2aa0c 100644 --- a/src/allmydata/test/test_introducer.py +++ b/src/allmydata/test/test_introducer.py @@ -1016,6 +1016,7 @@ class Announcements(unittest.TestCase): with cache_filepath.open() as f: return yaml.safe_load(f) + @defer.inlineCallbacks def test_client_cache(self): basedir = "introducer/ClientSeqnums/test_client_cache_1" fileutil.make_dirs(basedir) @@ -1080,6 +1081,22 @@ class Announcements(unittest.TestCase): set([a["ann"]["anonymous-storage-FURL"] for a in announcements])) + # test loading + ic2 = IntroducerClient(None, "introducer.furl", u"my_nickname", + "my_version", "oldest_version", {}, fakeseq, + ic._cache_filepath) + announcements = {} + def got(key_s, ann): + announcements[key_s] = ann + ic2.subscribe_to("storage", got) + ic2._load_announcements() # normally happens when connection fails + yield flushEventualQueue() + + self.failUnless(pub1 in announcements) + self.failUnlessEqual(announcements[pub1]["anonymous-storage-FURL"], + furl2) + self.failUnlessEqual(announcements[pub2]["anonymous-storage-FURL"], + furl3) class YAMLUnicode(unittest.TestCase): def test_convert(self):