From: Brian Warner Date: Thu, 14 Jan 2010 21:02:46 +0000 (-0800) Subject: tahoe add-alias/create-alias: don't corrupt non-newline-terminated alias X-Git-Url: https://git.rkrishnan.org/components/%22news.html/...?a=commitdiff_plain;h=874a979a8ec2b167df3f14cd9ff55523faa76274;p=tahoe-lafs%2Ftahoe-lafs.git tahoe add-alias/create-alias: don't corrupt non-newline-terminated alias file. Closes #741. --- diff --git a/src/allmydata/scripts/tahoe_add_alias.py b/src/allmydata/scripts/tahoe_add_alias.py index e74a13ee..b7ebcddd 100644 --- a/src/allmydata/scripts/tahoe_add_alias.py +++ b/src/allmydata/scripts/tahoe_add_alias.py @@ -3,6 +3,7 @@ import os.path from allmydata import uri from allmydata.scripts.common_http import do_http, check_http_error from allmydata.scripts.common import get_aliases +from allmydata.util.fileutil import move_into_place def add_alias(options): nodedir = options['node-directory'] @@ -19,9 +20,24 @@ def add_alias(options): return 1 aliasfile = os.path.join(nodedir, "private", "aliases") cap = uri.from_string_dirnode(cap).to_string() - f = open(aliasfile, "a") - f.write("%s: %s\n" % (alias, cap)) + + # we use os.path.exists, rather than catching EnvironmentError, to avoid + # clobbering the valuable alias file in case of spurious or transient + # filesystem errors. + if os.path.exists(aliasfile): + f = open(aliasfile, "r") + aliases = f.read() + f.close() + if not aliases.endswith("\n"): + aliases += "\n" + else: + aliases = "" + aliases += "%s: %s\n" % (alias, cap) + f = open(aliasfile+".tmp", "w") + f.write(aliases) f.close() + move_into_place(aliasfile+".tmp", aliasfile) + print >>stdout, "Alias '%s' added" % (alias,) return 0 @@ -52,16 +68,28 @@ def create_alias(options): new_uri = resp.read().strip() # probably check for others.. - f = open(aliasfile, "a") - f.write("%s: %s\n" % (alias, new_uri)) + + # see above about EnvironmentError + if os.path.exists(aliasfile): + f = open(aliasfile, "r") + aliases = f.read() + f.close() + if not aliases.endswith("\n"): + aliases += "\n" + else: + aliases = "" + aliases += "%s: %s\n" % (alias, new_uri) + f = open(aliasfile+".tmp", "w") + f.write(aliases) f.close() + move_into_place(aliasfile+".tmp", aliasfile) + print >>stdout, "Alias '%s' created" % (alias,) return 0 def list_aliases(options): nodedir = options['node-directory'] stdout = options.stdout - stderr = options.stderr aliases = get_aliases(nodedir) alias_names = sorted(aliases.keys()) max_width = max([len(name) for name in alias_names] + [0]) diff --git a/src/allmydata/test/test_cli.py b/src/allmydata/test/test_cli.py index 3b80e47a..30037910 100644 --- a/src/allmydata/test/test_cli.py +++ b/src/allmydata/test/test_cli.py @@ -98,7 +98,6 @@ class CLI(unittest.TestCase): def test_dump_cap_chk(self): key = "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" - storage_index = hashutil.storage_index_hash(key) uri_extension_hash = hashutil.uri_extension_hash("stuff") needed_shares = 25 total_shares = 100 @@ -460,6 +459,7 @@ class CreateAlias(GridTestMixin, CLITestMixin, unittest.TestCase): def test_create(self): self.basedir = "cli/CreateAlias/create" self.set_up_grid() + aliasfile = os.path.join(self.get_clientdir(), "private", "aliases") d = self.do_cli("create-alias", "tahoe") def _done((rc,stdout,stderr)): @@ -522,6 +522,56 @@ class CreateAlias(GridTestMixin, CLITestMixin, unittest.TestCase): self._test_webopen(["two:"], self.two_url) d.addCallback(_test_urls) + def _remove_trailing_newline_and_create_alias(ign): + f = open(aliasfile, "r") + old = f.read() + f.close() + # ticket #741 is about a manually-edited alias file (which + # doesn't end in a newline) being corrupted by a subsequent + # "tahoe create-alias" + f = open(aliasfile, "w") + f.write(old.rstrip()) + f.close() + return self.do_cli("create-alias", "un-corrupted1") + d.addCallback(_remove_trailing_newline_and_create_alias) + def _check_not_corrupted1((rc,stdout,stderr)): + self.failUnless("Alias 'un-corrupted1' created" in stdout, stdout) + self.failIf(stderr) + # the old behavior was to simply append the new record, causing a + # line that looked like "NAME1: CAP1NAME2: CAP2". This won't look + # like a valid dircap, so get_aliases() will raise an exception. + aliases = get_aliases(self.get_clientdir()) + self.failUnless("added" in aliases) + self.failUnless(aliases["added"].startswith("URI:DIR2:")) + # to be safe, let's confirm that we don't see "NAME2:" in CAP1. + # No chance of a false-negative, because the hyphen in + # "un-corrupted1" is not a valid base32 character. + self.failIfIn("un-corrupted1:", aliases["added"]) + self.failUnless("un-corrupted1" in aliases) + self.failUnless(aliases["un-corrupted1"].startswith("URI:DIR2:")) + d.addCallback(_check_not_corrupted1) + + def _remove_trailing_newline_and_add_alias(ign): + # same thing, but for "tahoe add-alias" + f = open(aliasfile, "r") + old = f.read() + f.close() + f = open(aliasfile, "w") + f.write(old.rstrip()) + f.close() + return self.do_cli("add-alias", "un-corrupted2", self.two_uri) + d.addCallback(_remove_trailing_newline_and_add_alias) + def _check_not_corrupted((rc,stdout,stderr)): + self.failUnless("Alias 'un-corrupted2' added" in stdout, stdout) + self.failIf(stderr) + aliases = get_aliases(self.get_clientdir()) + self.failUnless("un-corrupted1" in aliases) + self.failUnless(aliases["un-corrupted1"].startswith("URI:DIR2:")) + self.failIfIn("un-corrupted2:", aliases["un-corrupted1"]) + self.failUnless("un-corrupted2" in aliases) + self.failUnless(aliases["un-corrupted2"].startswith("URI:DIR2:")) + d.addCallback(_check_not_corrupted) + return d class Put(GridTestMixin, CLITestMixin, unittest.TestCase): @@ -592,7 +642,6 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): self.set_up_grid() rel_fn = os.path.join(self.basedir, "DATAFILE") - abs_fn = os.path.abspath(rel_fn) # we make the file small enough to fit in a LIT file, for speed DATA = "short file" DATA2 = "short file two" @@ -676,7 +725,6 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): DATA = "data" * 100 DATA2 = "two" * 100 rel_fn = os.path.join(self.basedir, "DATAFILE") - abs_fn = os.path.abspath(rel_fn) DATA3 = "three" * 100 f = open(rel_fn, "w") f.write(DATA3)