Changeset 874a979 in trunk

Timestamp:

2010-01-14T21:02:46Z (16 years ago)

Author:

Brian Warner <warner@…>

Branches:

master

Children:

731d15e

Parents:

d3d1293

Message:

tahoe add-alias/create-alias: don't corrupt non-newline-terminated alias
file. Closes #741.

Location:

src/allmydata

Files:

• scripts/tahoe_add_alias.py (modified) (4 diffs)
• test/test_cli.py (modified) (5 diffs)

src/allmydata/scripts/tahoe_add_alias.py ¶

@@ -4 +4 @@
 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):
@@ -20 +21 @@
     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
@@ -53 +69 @@
 
     # 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
@@ -62 +91 @@
     nodedir = options['node-directory']
     stdout = options.stdout
-    stderr = options.stderr
     aliases = get_aliases(nodedir)
     alias_names = sorted(aliases.keys())

src/allmydata/test/test_cli.py ¶

@@ -99 +99 @@
     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
@@ -461 +460 @@
         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")
@@ -522 +522 @@
             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
@@ -593 +643 @@
 
         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"
@@ -677 +726 @@
         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")