Merge pull request #220 from tahoe-lafs/2669.magic-folder-misc-patches.1

This PR is a bunch of miscellaneous patches that are on the Magic Folder branches, but independent of Magic Folder itself.

src/allmydata/node.py

@@ -12,6 +12,7 @@ from allmydata.util import fileutil, iputil, observer
 from allmydata.util.assertutil import precondition, _assert
 from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.util.encodingutil import get_filesystem_encoding, quote_output
+from allmydata.util import configutil
 
 # Add our application versions to the data that Foolscap's LogPublisher
 # reports.
@@ -133,27 +134,13 @@ class Node(service.MultiService):
                                          % (quote_output(fn), section, option))
             return default
 
-    def set_config(self, section, option, value):
-        if not self.config.has_section(section):
-            self.config.add_section(section)
-        self.config.set(section, option, value)
-        assert self.config.get(section, option) == value
-
     def read_config(self):
         self.error_about_old_config_files()
         self.config = ConfigParser.SafeConfigParser()
 
         tahoe_cfg = os.path.join(self.basedir, "tahoe.cfg")
         try:
-            f = open(tahoe_cfg, "rb")
-            try:
-                # Skip any initial Byte Order Mark. Since this is an ordinary file, we
-                # don't need to handle incomplete reads, and can assume seekability.
-                if f.read(3) != '\xEF\xBB\xBF':
-                    f.seek(0)
-                self.config.readfp(f)
-            finally:
-                f.close()
+            self.config = configutil.get_config(tahoe_cfg)
         except EnvironmentError:
             if os.path.exists(tahoe_cfg):
                 raise
@@ -165,7 +152,7 @@ class Node(service.MultiService):
             # provide a value.
             try:
                 file_tubport = fileutil.read(self._portnumfile).strip()
-                self.set_config("node", "tub.port", file_tubport)
+                configutil.set_config(self.config, "node", "tub.port", file_tubport)
             except EnvironmentError:
                 if os.path.exists(self._portnumfile):
                     raise

src/allmydata/scripts/common.py

@@ -57,9 +57,14 @@ class BasedirOptions(BaseOptions):
     ]
 
     def parseArgs(self, basedir=None):
-        if self.parent['node-directory'] and self['basedir']:
+        # This finds the node-directory option correctly even if we are in a subcommand.
+        root = self.parent
+        while root.parent is not None:
+            root = root.parent
+
+        if root['node-directory'] and self['basedir']:
             raise usage.UsageError("The --node-directory (or -d) and --basedir (or -C) options cannot both be used.")
-        if self.parent['node-directory'] and basedir:
+        if root['node-directory'] and basedir:
             raise usage.UsageError("The --node-directory (or -d) option and a basedir argument cannot both be used.")
         if self['basedir'] and basedir:
             raise usage.UsageError("The --basedir (or -C) option and a basedir argument cannot both be used.")
@@ -68,13 +73,14 @@ class BasedirOptions(BaseOptions):
             b = argv_to_abspath(basedir)
         elif self['basedir']:
             b = argv_to_abspath(self['basedir'])
-        elif self.parent['node-directory']:
-            b = argv_to_abspath(self.parent['node-directory'])
+        elif root['node-directory']:
+            b = argv_to_abspath(root['node-directory'])
         elif self.default_nodedir:
             b = self.default_nodedir
         else:
             raise usage.UsageError("No default basedir available, you must provide one with --node-directory, --basedir, or a basedir argument")
         self['basedir'] = b
+        self['node-directory'] = b
 
     def postOptions(self):
         if not self['basedir']:

src/allmydata/scripts/tahoe_add_alias.py

@@ -1,6 +1,9 @@
 
 import os.path
 import codecs
+
+from allmydata.util.assertutil import precondition
+
 from allmydata import uri
 from allmydata.scripts.common_http import do_http, check_http_error
 from allmydata.scripts.common import get_aliases
@@ -29,6 +32,7 @@ def add_line_to_aliasfile(aliasfile, alias, cap):
 def add_alias(options):
     nodedir = options['node-directory']
     alias = options.alias
+    precondition(isinstance(alias, unicode), alias=alias)
     cap = options.cap
     stdout = options.stdout
     stderr = options.stderr
@@ -56,6 +60,7 @@ def create_alias(options):
     # mkdir+add_alias
     nodedir = options['node-directory']
     alias = options.alias
+    precondition(isinstance(alias, unicode), alias=alias)
     stdout = options.stdout
     stderr = options.stderr
     if u":" in alias:

src/allmydata/test/test_cli.py

@@ -36,7 +36,7 @@ from twisted.python import usage
 
 from allmydata.util.assertutil import precondition
 from allmydata.util.encodingutil import listdir_unicode, unicode_platform, \
-    get_io_encoding, get_filesystem_encoding
+    get_io_encoding, get_filesystem_encoding, unicode_to_argv
 
 timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s
 
@@ -49,8 +49,14 @@ def parse_options(basedir, command, args):
 
 class CLITestMixin(ReallyEqualMixin):
     def do_cli(self, verb, *args, **kwargs):
+        precondition(not [True for arg in args if not isinstance(arg, str)],
+                     "arguments to do_cli must be strs -- convert using unicode_to_argv", args=args)
+
+        # client_num is used to execute client CLI commands on a specific client.
+        client_num = kwargs.get("client_num", 0)
+
         nodeargs = [
-            "--node-directory", self.get_clientdir(),
+            "--node-directory", unicode_to_argv(self.get_clientdir(i=client_num)),
             ]
         argv = nodeargs + [verb] + list(args)
         stdin = kwargs.get("stdin", "")

src/allmydata/test/test_configutil.py

@@ -0,0 +1,34 @@
+import os.path
+
+from twisted.trial import unittest
+
+from allmydata.util import configutil
+from allmydata.test.no_network import GridTestMixin
+from .test_cli import CLITestMixin
+
+
+class ConfigUtilTests(CLITestMixin, GridTestMixin, unittest.TestCase):
+
+    def test_config_utils(self):
+        self.basedir = "cli/ConfigUtilTests/test-config-utils"
+        self.set_up_grid()
+        tahoe_cfg = os.path.join(self.get_clientdir(i=0), "tahoe.cfg")
+
+        # test that at least one option was read correctly
+        config = configutil.get_config(tahoe_cfg)
+        self.failUnlessEqual(config.get("node", "nickname"), "client-0")
+
+        # test that set_config can mutate an existing option
+        configutil.set_config(config, "node", "nickname", "Alice!")
+        configutil.write_config(tahoe_cfg, config)
+
+        config = configutil.get_config(tahoe_cfg)
+        self.failUnlessEqual(config.get("node", "nickname"), "Alice!")
+
+        # test that set_config can set a new option
+        descriptor = "Twas brillig, and the slithy toves Did gyre and gimble in the wabe"
+        configutil.set_config(config, "node", "descriptor", descriptor)
+        configutil.write_config(tahoe_cfg, config)
+
+        config = configutil.get_config(tahoe_cfg)
+        self.failUnlessEqual(config.get("node", "descriptor"), descriptor)

src/allmydata/test/test_encodingutil.py

@@ -400,13 +400,13 @@ class QuoteOutput(ReallyEqualMixin, unittest.TestCase):
         check(u"\n",       u"\"\\x0a\"", quote_newlines=True)
 
     def test_quote_output_default(self):
-        encodingutil.io_encoding = 'ascii'
+        self.patch(encodingutil, 'io_encoding', 'ascii')
         self.test_quote_output_ascii(None)
 
-        encodingutil.io_encoding = 'latin1'
+        self.patch(encodingutil, 'io_encoding', 'latin1')
         self.test_quote_output_latin1(None)
 
-        encodingutil.io_encoding = 'utf-8'
+        self.patch(encodingutil, 'io_encoding', 'utf-8')
         self.test_quote_output_utf8(None)
 
 

src/allmydata/uri.py

@@ -730,7 +730,7 @@ ALLEGED_IMMUTABLE_PREFIX = 'imm.'
 
 def from_string(u, deep_immutable=False, name=u"<unknown name>"):
     if not isinstance(u, str):
-        raise TypeError("unknown URI type: %s.." % str(u)[:100])
+        raise TypeError("URI must be str: %r" % (u,))
 
     # We allow and check ALLEGED_READONLY_PREFIX or ALLEGED_IMMUTABLE_PREFIX
     # on all URIs, even though we would only strictly need to do so for caps of

src/allmydata/util/configutil.py

@@ -0,0 +1,29 @@
+
+from ConfigParser import SafeConfigParser
+
+
+def get_config(tahoe_cfg):
+    config = SafeConfigParser()
+    f = open(tahoe_cfg, "rb")
+    try:
+        # Skip any initial Byte Order Mark. Since this is an ordinary file, we
+        # don't need to handle incomplete reads, and can assume seekability.
+        if f.read(3) != '\xEF\xBB\xBF':
+            f.seek(0)
+        config.readfp(f)
+    finally:
+        f.close()
+    return config
+
+def set_config(config, section, option, value):
+    if not config.has_section(section):
+        config.add_section(section)
+    config.set(section, option, value)
+    assert config.get(section, option) == value
+
+def write_config(tahoe_cfg, config):
+    f = open(tahoe_cfg, "wb")
+    try:
+        config.write(f)
+    finally:
+        f.close()

src/allmydata/util/encodingutil.py

@@ -88,12 +88,16 @@ def argv_to_unicode(s):
         raise usage.UsageError("Argument %s cannot be decoded as %s." %
                                (quote_output(s), io_encoding))
 
-def argv_to_abspath(s):
+def argv_to_abspath(s, **kwargs):
     """
     Convenience function to decode an argv element to an absolute path, with ~ expanded.
     If this fails, raise a UsageError.
     """
-    return abspath_expanduser_unicode(argv_to_unicode(s))
+    decoded = argv_to_unicode(s)
+    if decoded.startswith(u'-'):
+        raise usage.UsageError("Path argument %s cannot start with '-'.\nUse %s if you intended to refer to a file."
+                               % (quote_output(s), quote_output(os.path.join('.', s))))
+    return abspath_expanduser_unicode(decoded, **kwargs)
 
 def unicode_to_argv(s, mangle=False):
     """

src/allmydata/util/fileutil.py

@@ -4,6 +4,11 @@ Futz with files like a pro.
 
 import sys, exceptions, os, stat, tempfile, time, binascii
 
+if sys.platform == "win32":
+    from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, c_ulonglong, \
+        create_unicode_buffer, get_last_error
+    from ctypes.wintypes import BOOL, DWORD, LPCWSTR, LPWSTR
+
 from twisted.python import log
 
 from pycryptopp.cipher.aes import AES
@@ -335,16 +340,11 @@ def to_windows_long_path(path):
 
 have_GetDiskFreeSpaceExW = False
 if sys.platform == "win32":
-    from ctypes import WINFUNCTYPE, windll, POINTER, byref, c_ulonglong, create_unicode_buffer, \
-        get_last_error
-    from ctypes.wintypes import BOOL, DWORD, LPCWSTR, LPWSTR
-
     # <http://msdn.microsoft.com/en-us/library/windows/desktop/ms683188%28v=vs.85%29.aspx>
     GetEnvironmentVariableW = WINFUNCTYPE(
-        DWORD,
-          LPCWSTR, LPWSTR, DWORD,
+        DWORD,  LPCWSTR, LPWSTR, DWORD,
         use_last_error=True
-      )(("GetEnvironmentVariableW", windll.kernel32))
+    )(("GetEnvironmentVariableW", windll.kernel32))
 
     try:
         # <http://msdn.microsoft.com/en-us/library/aa383742%28v=VS.85%29.aspx>
@@ -352,10 +352,9 @@ if sys.platform == "win32":
 
         # <http://msdn.microsoft.com/en-us/library/aa364937%28VS.85%29.aspx>
         GetDiskFreeSpaceExW = WINFUNCTYPE(
-            BOOL,
-              LPCWSTR, PULARGE_INTEGER, PULARGE_INTEGER, PULARGE_INTEGER,
+            BOOL,  LPCWSTR, PULARGE_INTEGER, PULARGE_INTEGER, PULARGE_INTEGER,
             use_last_error=True
-          )(("GetDiskFreeSpaceExW", windll.kernel32))
+        )(("GetDiskFreeSpaceExW", windll.kernel32))
 
         have_GetDiskFreeSpaceExW = True
     except Exception:
@@ -403,8 +402,8 @@ def windows_getenv(name):
         err = get_last_error()
         if err == ERROR_ENVVAR_NOT_FOUND:
             return None
-        raise OSError("Windows error %d attempting to read size of environment variable %r"
-                      % (err, name))
+        raise OSError("WinError: %s\n attempting to read size of environment variable %r"
+                      % (WinError(err), name))
     if n == 1:
         # Avoid an ambiguity between a zero-length string and an error in the return value of the
         # call to GetEnvironmentVariableW below.
@@ -416,8 +415,8 @@ def windows_getenv(name):
         err = get_last_error()
         if err == ERROR_ENVVAR_NOT_FOUND:
             return None
-        raise OSError("Windows error %d attempting to read environment variable %r"
-                      % (err, name))
+        raise OSError("WinError: %s\n attempting to read environment variable %r"
+                      % (WinError(err), name))
     if retval >= n:
         raise OSError("Unexpected result %d (expected less than %d) from GetEnvironmentVariableW attempting to read environment variable %r"
                       % (retval, n, name))
@@ -459,8 +458,8 @@ def get_disk_stats(whichdir, reserved_space=0):
                                                byref(n_total),
                                                byref(n_free_for_root))
         if retval == 0:
-            raise OSError("Windows error %d attempting to get disk statistics for %r"
-                          % (get_last_error(), whichdir))
+            raise OSError("WinError: %s\n attempting to get disk statistics for %r"
+                          % (WinError(get_last_error()), whichdir))
         free_for_nonroot = n_free_for_nonroot.value
         total            = n_total.value
         free_for_root    = n_free_for_root.value

src/allmydata/windows/fixups.py

@@ -9,13 +9,18 @@ def initialize():
     done = True
 
     import codecs, re
-    from ctypes import WINFUNCTYPE, windll, POINTER, byref, c_int
+    from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, c_int, get_last_error
     from ctypes.wintypes import BOOL, HANDLE, DWORD, UINT, LPWSTR, LPCWSTR, LPVOID
+
     from allmydata.util import log
     from allmydata.util.encodingutil import canonical_encoding
 
     # <https://msdn.microsoft.com/en-us/library/ms680621%28VS.85%29.aspx>
-    SetErrorMode = WINFUNCTYPE(UINT, UINT)(("SetErrorMode", windll.kernel32))
+    SetErrorMode = WINFUNCTYPE(
+        UINT,  UINT,
+        use_last_error=True
+    )(("SetErrorMode", windll.kernel32))
+
     SEM_FAILCRITICALERRORS = 0x0001
     SEM_NOOPENFILEERRORBOX = 0x8000
 
@@ -50,13 +55,27 @@ def initialize():
         # <https://msdn.microsoft.com/en-us/library/ms683167(VS.85).aspx>
         # BOOL WINAPI GetConsoleMode(HANDLE hConsole, LPDWORD lpMode);
 
-        GetStdHandle = WINFUNCTYPE(HANDLE, DWORD)(("GetStdHandle", windll.kernel32))
+        GetStdHandle = WINFUNCTYPE(
+            HANDLE,  DWORD,
+            use_last_error=True
+        )(("GetStdHandle", windll.kernel32))
+
         STD_OUTPUT_HANDLE = DWORD(-11)
         STD_ERROR_HANDLE  = DWORD(-12)
-        GetFileType = WINFUNCTYPE(DWORD, DWORD)(("GetFileType", windll.kernel32))
+
+        GetFileType = WINFUNCTYPE(
+            DWORD,  DWORD,
+            use_last_error=True
+        )(("GetFileType", windll.kernel32))
+
         FILE_TYPE_CHAR   = 0x0002
         FILE_TYPE_REMOTE = 0x8000
-        GetConsoleMode = WINFUNCTYPE(BOOL, HANDLE, POINTER(DWORD))(("GetConsoleMode", windll.kernel32))
+
+        GetConsoleMode = WINFUNCTYPE(
+            BOOL,  HANDLE, POINTER(DWORD),
+            use_last_error=True
+        )(("GetConsoleMode", windll.kernel32))
+
         INVALID_HANDLE_VALUE = DWORD(-1).value
 
         def not_a_console(handle):
@@ -88,11 +107,14 @@ def initialize():
                 real_stderr = False
 
         if real_stdout or real_stderr:
+            # <https://msdn.microsoft.com/en-us/library/windows/desktop/ms687401%28v=vs.85%29.aspx>
             # BOOL WINAPI WriteConsoleW(HANDLE hOutput, LPWSTR lpBuffer, DWORD nChars,
             #                           LPDWORD lpCharsWritten, LPVOID lpReserved);
 
-            WriteConsoleW = WINFUNCTYPE(BOOL, HANDLE, LPWSTR, DWORD, POINTER(DWORD), LPVOID) \
-                                (("WriteConsoleW", windll.kernel32))
+            WriteConsoleW = WINFUNCTYPE(
+                BOOL,  HANDLE, LPWSTR, DWORD, POINTER(DWORD), LPVOID,
+                use_last_error=True
+            )(("WriteConsoleW", windll.kernel32))
 
             class UnicodeOutput:
                 def __init__(self, hConsole, stream, fileno, name):
@@ -139,8 +161,10 @@ def initialize():
                                 # There is a shorter-than-documented limitation on the length of the string
                                 # passed to WriteConsoleW (see #1232).
                                 retval = WriteConsoleW(self._hConsole, text, min(remaining, 10000), byref(n), None)
-                                if retval == 0 or n.value == 0:
-                                    raise IOError("WriteConsoleW returned %r, n.value = %r" % (retval, n.value))
+                                if retval == 0:
+                                    raise IOError("WriteConsoleW failed with WinError: %s" % (WinError(get_last_error()),))
+                                if n.value == 0:
+                                    raise IOError("WriteConsoleW returned %r, n.value = 0" % (retval,))
                                 remaining -= n.value
                                 if remaining == 0: break
                                 text = text[n.value:]
@@ -169,12 +193,23 @@ def initialize():
         _complain("exception %r while fixing up sys.stdout and sys.stderr" % (e,))
 
     # This works around <http://bugs.python.org/issue2128>.
-    GetCommandLineW = WINFUNCTYPE(LPWSTR)(("GetCommandLineW", windll.kernel32))
-    CommandLineToArgvW = WINFUNCTYPE(POINTER(LPWSTR), LPCWSTR, POINTER(c_int)) \
-                            (("CommandLineToArgvW", windll.shell32))
+
+    # <https://msdn.microsoft.com/en-us/library/windows/desktop/ms683156%28v=vs.85%29.aspx>
+    GetCommandLineW = WINFUNCTYPE(
+        LPWSTR,
+        use_last_error=True
+    )(("GetCommandLineW", windll.kernel32))
+
+    # <https://msdn.microsoft.com/en-us/library/windows/desktop/bb776391%28v=vs.85%29.aspx>
+    CommandLineToArgvW = WINFUNCTYPE(
+        POINTER(LPWSTR),  LPCWSTR, POINTER(c_int),
+        use_last_error=True
+    )(("CommandLineToArgvW", windll.shell32))
 
     argc = c_int(0)
     argv_unicode = CommandLineToArgvW(GetCommandLineW(), byref(argc))
+    if argv_unicode is None:
+        raise WinError(get_last_error())
 
     # Because of <http://bugs.python.org/issue8775> (and similar limitations in
     # twisted), the 'bin/tahoe' script cannot invoke us with the actual Unicode arguments.