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 e74a13eeaedfdc826a1e0b2be521c38e273cfcc8..b7ebcddd51ab96b024a51b4cec1bfb4207813529 100644 (file)
--- 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 3b80e47a1bb5be73277dc5ce8c558d6256c4e3ab..30037910f51bc2a7427cfcb28310d833b19c1c40 100644 (file)
--- 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)