Quote local paths correctly. refs #2235

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

src/allmydata/scripts/cli.py

@@ -2,7 +2,7 @@ import os.path, re, fnmatch
 from twisted.python import usage
 from allmydata.scripts.common import get_aliases, get_default_nodedir, \
      DEFAULT_ALIAS, BaseOptions
-from allmydata.util.encodingutil import argv_to_unicode, argv_to_abspath, quote_output
+from allmydata.util.encodingutil import argv_to_unicode, argv_to_abspath, quote_local_unicode_path
 
 NODEURL_RE=re.compile("http(s?)://([^:]*)(:([1-9][0-9]*))?")
 
@@ -368,7 +368,7 @@ class BackupOptions(FilesystemOptions):
         try:
             exclude_file = file(abs_filepath)
         except:
-            raise BackupConfigurationError('Error opening exclude file %s.' % quote_output(abs_filepath))
+            raise BackupConfigurationError('Error opening exclude file %s.' % quote_local_unicode_path(abs_filepath))
         try:
             for line in exclude_file:
                 self.opt_exclude(line)

src/allmydata/scripts/debug.py

@@ -645,7 +645,7 @@ def find_shares(options):
     /home/warner/testnet/node-2/storage/shares/44k/44kai1tui348689nrw8fjegc8c/2
     """
     from allmydata.storage.server import si_a2b, storage_index_to_dir
-    from allmydata.util.encodingutil import listdir_unicode
+    from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path
 
     out = options.stdout
     sharedir = storage_index_to_dir(si_a2b(options.si_s))
@@ -653,7 +653,7 @@ def find_shares(options):
         d = os.path.join(d, "storage", "shares", sharedir)
         if os.path.exists(d):
             for shnum in listdir_unicode(d):
-                print >>out, os.path.join(d, shnum)
+                print >>out, quote_local_unicode_path(os.path.join(d, shnum), quotemarks=False)
 
     return 0
 

src/allmydata/scripts/startstop_node.py

@@ -4,7 +4,7 @@ from allmydata.scripts.common import BasedirOptions
 from twisted.scripts import twistd
 from twisted.python import usage
 from allmydata.util import fileutil
-from allmydata.util.encodingutil import listdir_unicode, quote_output
+from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path
 
 
 class StartOptions(BasedirOptions):
@@ -92,13 +92,14 @@ def identify_node_type(basedir):
 
 def start(config, out=sys.stdout, err=sys.stderr):
     basedir = config['basedir']
-    print >>out, "STARTING", quote_output(basedir)
+    quoted_basedir = quote_local_unicode_path(basedir)
+    print >>out, "STARTING", quoted_basedir
     if not os.path.isdir(basedir):
-        print >>err, "%s does not look like a directory at all" % quote_output(basedir)
+        print >>err, "%s does not look like a directory at all" % quoted_basedir
         return 1
     nodetype = identify_node_type(basedir)
     if not nodetype:
-        print >>err, "%s is not a recognizable node directory" % quote_output(basedir)
+        print >>err, "%s is not a recognizable node directory" % quoted_basedir
         return 1
     # Now prepare to turn into a twistd process. This os.chdir is the point
     # of no return.
@@ -108,7 +109,7 @@ def start(config, out=sys.stdout, err=sys.stderr):
         and "--nodaemon" not in config.twistd_args
         and "--syslog" not in config.twistd_args
         and "--logfile" not in config.twistd_args):
-        fileutil.make_dirs(os.path.join(basedir, "logs"))
+        fileutil.make_dirs(os.path.join(basedir, u"logs"))
         twistd_args.extend(["--logfile", os.path.join("logs", "twistd.log")])
     twistd_args.extend(config.twistd_args)
     twistd_args.append("StartTahoeNode") # point at our StartTahoeNodePlugin
@@ -154,17 +155,18 @@ def start(config, out=sys.stdout, err=sys.stderr):
     else:
         verb = "starting"
 
-    print >>out, "%s node in %s" % (verb, basedir)
+    print >>out, "%s node in %s" % (verb, quoted_basedir)
     twistd.runApp(twistd_config)
     # we should only reach here if --nodaemon or equivalent was used
     return 0
 
 def stop(config, out=sys.stdout, err=sys.stderr):
     basedir = config['basedir']
-    print >>out, "STOPPING", quote_output(basedir)
+    quoted_basedir = quote_local_unicode_path(basedir)
-    pidfile = os.path.join(basedir, "twistd.pid")
+    print >>out, "STOPPING", quoted_basedir
+    pidfile = os.path.join(basedir, u"twistd.pid")
     if not os.path.exists(pidfile):
-        print >>err, "%s does not look like a running node directory (no twistd.pid)" % quote_output(basedir)
+        print >>err, "%s does not look like a running node directory (no twistd.pid)" % quoted_basedir
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
         return 2

src/allmydata/scripts/tahoe_backup.py

@@ -10,7 +10,7 @@ from allmydata.scripts.common_http import do_http, HTTPError, format_http_error
 from allmydata.util import time_format
 from allmydata.scripts import backupdb
 from allmydata.util.encodingutil import listdir_unicode, quote_output, \
-     to_str, FilenameEncodingError, unicode_to_url
+     quote_local_unicode_path, to_str, FilenameEncodingError, unicode_to_url
 from allmydata.util.assertutil import precondition
 from allmydata.util.fileutil import abspath_expanduser_unicode
 
@@ -163,7 +163,8 @@ class BackerUpper:
         precondition(isinstance(localpath, unicode), localpath)
         # returns newdircap
 
-        self.verboseprint("processing %s" % quote_output(localpath))
+        quoted_path = quote_local_unicode_path(localpath)
+        self.verboseprint("processing %s" % (quoted_path,))
         create_contents = {} # childname -> (type, rocap, metadata)
         compare_contents = {} # childname -> rocap
 
@@ -171,11 +172,11 @@ class BackerUpper:
             children = listdir_unicode(localpath)
         except EnvironmentError:
             self.directories_skipped += 1
-            self.warn("WARNING: permission denied on directory %s" % quote_output(localpath))
+            self.warn("WARNING: permission denied on directory %s" % (quoted_path,))
             children = []
         except FilenameEncodingError:
             self.directories_skipped += 1
-            self.warn("WARNING: could not list directory %s due to a filename encoding error" % quote_output(localpath))
+            self.warn("WARNING: could not list directory %s due to a filename encoding error" % (quoted_path,))
             children = []
 
         for child in self.options.filter_listdir(children):
@@ -197,17 +198,17 @@ class BackerUpper:
                     compare_contents[child] = childcap
                 except EnvironmentError:
                     self.files_skipped += 1
-                    self.warn("WARNING: permission denied on file %s" % quote_output(childpath))
+                    self.warn("WARNING: permission denied on file %s" % quote_local_unicode_path(childpath))
             else:
                 self.files_skipped += 1
                 if os.path.islink(childpath):
-                    self.warn("WARNING: cannot backup symlink %s" % quote_output(childpath))
+                    self.warn("WARNING: cannot backup symlink %s" % quote_local_unicode_path(childpath))
                 else:
-                    self.warn("WARNING: cannot backup special file %s" % quote_output(childpath))
+                    self.warn("WARNING: cannot backup special file %s" % quote_local_unicode_path(childpath))
 
         must_create, r = self.check_backupdb_directory(compare_contents)
         if must_create:
-            self.verboseprint(" creating directory for %s" % quote_output(localpath))
+            self.verboseprint(" creating directory for %s" % quote_local_unicode_path(localpath))
             newdircap = mkdir(create_contents, self.options)
             assert isinstance(newdircap, str)
             if r:
@@ -215,7 +216,7 @@ class BackerUpper:
             self.directories_created += 1
             return newdircap
         else:
-            self.verboseprint(" re-using old directory for %s" % quote_output(localpath))
+            self.verboseprint(" re-using old directory for %s" % quote_local_unicode_path(localpath))
             self.directories_reused += 1
             return r.was_created()
 
@@ -290,14 +291,14 @@ class BackerUpper:
     def upload(self, childpath):
         precondition(isinstance(childpath, unicode), childpath)
 
-        #self.verboseprint("uploading %s.." % quote_output(childpath))
+        #self.verboseprint("uploading %s.." % quote_local_unicode_path(childpath))
         metadata = get_local_metadata(childpath)
 
         # we can use the backupdb here
         must_upload, bdb_results = self.check_backupdb_file(childpath)
 
         if must_upload:
-            self.verboseprint("uploading %s.." % quote_output(childpath))
+            self.verboseprint("uploading %s.." % quote_local_unicode_path(childpath))
             infileobj = open(childpath, "rb")
             url = self.options['node-url'] + "uri"
             resp = do_http("PUT", url, infileobj)
@@ -305,7 +306,7 @@ class BackerUpper:
                 raise HTTPError("Error during file PUT", resp)
 
             filecap = resp.read().strip()
-            self.verboseprint(" %s -> %s" % (quote_output(childpath, quotemarks=False),
+            self.verboseprint(" %s -> %s" % (quote_local_unicode_path(childpath, quotemarks=False),
                                              quote_output(filecap, quotemarks=False)))
             #self.verboseprint(" metadata: %s" % (quote_output(metadata, quotemarks=False),))
 
@@ -316,7 +317,7 @@ class BackerUpper:
             return filecap, metadata
 
         else:
-            self.verboseprint("skipping %s.." % quote_output(childpath))
+            self.verboseprint("skipping %s.." % quote_local_unicode_path(childpath))
             self.files_reused += 1
             return bdb_results.was_uploaded(), metadata
 

src/allmydata/scripts/tahoe_cp.py

@@ -10,13 +10,14 @@ from allmydata.scripts.common_http import do_http, HTTPError
 from allmydata import uri
 from allmydata.util import fileutil
 from allmydata.util.fileutil import abspath_expanduser_unicode
-from allmydata.util.encodingutil import unicode_to_url, listdir_unicode, quote_output, to_str
+from allmydata.util.encodingutil import unicode_to_url, listdir_unicode, quote_output, \
+    quote_local_unicode_path, to_str
 from allmydata.util.assertutil import precondition
 
 
 class MissingSourceError(TahoeError):
-    def __init__(self, name):
+    def __init__(self, name, quotefn=quote_output):
-        TahoeError.__init__(self, "No such file or directory %s" % quote_output(name))
+        TahoeError.__init__(self, "No such file or directory %s" % quotefn(name))
 
 
 def GET_to_file(url):
@@ -565,7 +566,7 @@ class Copier:
             pathname = abspath_expanduser_unicode(path.decode('utf-8'))
             name = os.path.basename(pathname)
             if not os.path.exists(pathname):
-                raise MissingSourceError(source_spec)
+                raise MissingSourceError(source_spec, quotefn=quote_local_unicode_path)
             if os.path.isdir(pathname):
                 t = LocalDirectorySource(self.progress, pathname)
             else:

src/allmydata/test/test_cli.py

@@ -39,7 +39,7 @@ from twisted.python import usage
 
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, unicode_platform, \
-    quote_output, get_io_encoding, get_filesystem_encoding, \
+    quote_output, quote_local_unicode_path, get_io_encoding, get_filesystem_encoding, \
     unicode_to_output, unicode_to_argv, to_str
 from allmydata.util.fileutil import abspath_expanduser_unicode
 
@@ -2854,7 +2854,8 @@ class Backup(GridTestMixin, CLITestMixin, StallMixin, unittest.TestCase):
         def _check((rc, out, err)):
             self.failUnlessReallyEqual(rc, 2)
             foo2 = os.path.join(source, "foo2.txt")
-            self.failUnlessReallyEqual(err, "WARNING: cannot backup symlink '%s'\n" % foo2)
+            self.failUnlessIn("WARNING: cannot backup symlink ", err)
+            self.failUnlessIn(foo2, err)
 
             fu, fr, fs, dc, dr, ds = self.count_output(out)
             # foo.txt

src/allmydata/test/test_encodingutil.py

@@ -63,8 +63,9 @@ import os, sys, locale
 from allmydata.test.common_util import ReallyEqualMixin
 from allmydata.util import encodingutil
 from allmydata.util.encodingutil import argv_to_unicode, unicode_to_url, \
-    unicode_to_output, quote_output, unicode_platform, listdir_unicode, \
+    unicode_to_output, quote_output, quote_path, quote_local_unicode_path, \
-    FilenameEncodingError, get_io_encoding, get_filesystem_encoding, _reload
+    unicode_platform, listdir_unicode, FilenameEncodingError, get_io_encoding, \
+    get_filesystem_encoding, _reload
 from allmydata.dirnode import normalize
 
 from twisted.python import usage
@@ -395,6 +396,19 @@ class QuoteOutput(ReallyEqualMixin, unittest.TestCase):
         self.test_quote_output_utf8(None)
 
 
+class QuotePaths(ReallyEqualMixin, unittest.TestCase):
+    def test_quote_path(self):
+        self.failUnlessReallyEqual(quote_path([u'foo', u'bar']), "'foo/bar'")
+        self.failUnlessReallyEqual(quote_path([u'foo', u'bar'], quotemarks=True), "'foo/bar'")
+        self.failUnlessReallyEqual(quote_path([u'foo', u'\nbar']), '"foo/\\x0abar"')
+        self.failUnlessReallyEqual(quote_path([u'foo', u'\nbar'], quotemarks=True), '"foo/\\x0abar"')
+
+        self.failUnlessReallyEqual(quote_local_unicode_path(u"\\\\?\\C:\\foo"),
+                                   "'C:\\foo'" if sys.platform == "win32" else "'\\\\?\\C:\\foo'")
+        self.failUnlessReallyEqual(quote_local_unicode_path(u"\\\\?\\UNC\\foo\\bar"),
+                                   "'\\\\foo\\bar'" if sys.platform == "win32" else "'\\\\?\\UNC\\foo\\bar'")
+
+
 class UbuntuKarmicUTF8(EncodingUtil, unittest.TestCase):
     uname = 'Linux korn 2.6.31-14-generic #48-Ubuntu SMP Fri Oct 16 14:05:01 UTC 2009 x86_64'
     argv = 'lumi\xc3\xa8re'

src/allmydata/util/encodingutil.py

@@ -230,6 +230,16 @@ def quote_output(s, quotemarks=True, quote_newlines=None, encoding=None):
 def quote_path(path, quotemarks=True):
     return quote_output("/".join(map(to_str, path)), quotemarks=quotemarks, quote_newlines=True)
 
+def quote_local_unicode_path(path, quotemarks=True):
+    precondition(isinstance(path, unicode), path)
+
+    if sys.platform == "win32" and path.startswith(u"\\\\?\\"):
+        path = path[4 :]
+        if path.startswith(u"UNC\\"):
+            path = u"\\\\" + path[4 :]
+
+    return quote_output(path, quotemarks=quotemarks, quote_newlines=True)
+
 
 def unicode_platform():
     """