From 95f98e1aaef880b7d53308815a88255bf6dfe51e Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Fri, 30 Jan 2015 00:04:11 +0000 Subject: [PATCH] Quote local paths correctly. refs #2235 Signed-off-by: Daira Hopwood --- src/allmydata/scripts/cli.py | 4 ++-- src/allmydata/scripts/debug.py | 4 ++-- src/allmydata/scripts/startstop_node.py | 20 +++++++++--------- src/allmydata/scripts/tahoe_backup.py | 27 +++++++++++++------------ src/allmydata/scripts/tahoe_cp.py | 9 +++++---- src/allmydata/test/test_cli.py | 5 +++-- src/allmydata/test/test_encodingutil.py | 18 +++++++++++++++-- src/allmydata/util/encodingutil.py | 10 +++++++++ 8 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py index b0e4e6de..4fb9b66c 100644 --- a/src/allmydata/scripts/cli.py +++ b/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) diff --git a/src/allmydata/scripts/debug.py b/src/allmydata/scripts/debug.py index 1bba9d5f..fedd6985 100644 --- a/src/allmydata/scripts/debug.py +++ b/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 diff --git a/src/allmydata/scripts/startstop_node.py b/src/allmydata/scripts/startstop_node.py index 09cdc937..45b7fd11 100644 --- a/src/allmydata/scripts/startstop_node.py +++ b/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) - pidfile = os.path.join(basedir, "twistd.pid") + quoted_basedir = quote_local_unicode_path(basedir) + 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 diff --git a/src/allmydata/scripts/tahoe_backup.py b/src/allmydata/scripts/tahoe_backup.py index ef3da34b..e52407a6 100644 --- a/src/allmydata/scripts/tahoe_backup.py +++ b/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 diff --git a/src/allmydata/scripts/tahoe_cp.py b/src/allmydata/scripts/tahoe_cp.py index 965aa26f..dc62145d 100644 --- a/src/allmydata/scripts/tahoe_cp.py +++ b/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): - TahoeError.__init__(self, "No such file or directory %s" % quote_output(name)) + def __init__(self, name, quotefn=quote_output): + 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: diff --git a/src/allmydata/test/test_cli.py b/src/allmydata/test/test_cli.py index a277f0b6..e415707d 100644 --- a/src/allmydata/test/test_cli.py +++ b/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 diff --git a/src/allmydata/test/test_encodingutil.py b/src/allmydata/test/test_encodingutil.py index abd3d8cb..b37e2f72 100644 --- a/src/allmydata/test/test_encodingutil.py +++ b/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, \ - FilenameEncodingError, get_io_encoding, get_filesystem_encoding, _reload + unicode_to_output, quote_output, quote_path, quote_local_unicode_path, \ + 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' diff --git a/src/allmydata/util/encodingutil.py b/src/allmydata/util/encodingutil.py index 3ceb1a91..feafd8f5 100644 --- a/src/allmydata/util/encodingutil.py +++ b/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(): """ -- 2.37.2