From 1923cb9df6a59bcd535390caa90aaacb816b62a5 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Tue, 28 Apr 2015 20:43:09 +0100 Subject: [PATCH] Unicode path fixes for drop-upload. Signed-off-by: Daira Hopwood --- src/allmydata/client.py | 5 +-- src/allmydata/frontends/drop_upload.py | 32 +++++++++---------- src/allmydata/test/test_client.py | 7 +++-- src/allmydata/test/test_drop_upload.py | 43 +++++++++++++------------- src/allmydata/util/encodingutil.py | 23 +++++++++++++- 5 files changed, 65 insertions(+), 45 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 24432787..703acab2 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -501,11 +501,12 @@ class Client(node.Node, pollmixin.PollMixin): "put the cap in a 'private/drop_upload_dircap' file, and delete this option.") upload_dircap = self.get_or_create_private_config("drop_upload_dircap") - local_dir_utf8 = self.get_config("drop_upload", "local.directory") + local_dir_config = self.get_config("drop_upload", "local.directory").decode("utf-8") + local_dir = abspath_expanduser_unicode(local_dir_config, base=self.basedir) try: from allmydata.frontends import drop_upload - s = drop_upload.DropUploader(self, upload_dircap, local_dir_utf8) + s = drop_upload.DropUploader(self, upload_dircap, local_dir) s.setServiceParent(self) s.startService() diff --git a/src/allmydata/frontends/drop_upload.py b/src/allmydata/frontends/drop_upload.py index 5814c112..046a49fd 100644 --- a/src/allmydata/frontends/drop_upload.py +++ b/src/allmydata/frontends/drop_upload.py @@ -8,32 +8,24 @@ from foolscap.api import eventually from allmydata.interfaces import IDirectoryNode -from allmydata.util.encodingutil import quote_output, get_filesystem_encoding -from allmydata.util.fileutil import abspath_expanduser_unicode +from allmydata.util.fileutil import abspath_expanduser_unicode, precondition_abspath +from allmydata.util.encodingutil import listdir_unicode, to_filepath, \ + unicode_from_filepath, quote_local_unicode_path, FilenameEncodingError from allmydata.immutable.upload import FileName class DropUploader(service.MultiService): name = 'drop-upload' - def __init__(self, client, upload_dircap, local_dir_utf8, inotify=None): - service.MultiService.__init__(self) - - try: - local_dir_u = abspath_expanduser_unicode(local_dir_utf8.decode('utf-8')) - if sys.platform == "win32": - local_dir = local_dir_u - else: - local_dir = local_dir_u.encode(get_filesystem_encoding()) - except (UnicodeEncodeError, UnicodeDecodeError): - raise AssertionError("The '[drop_upload] local.directory' parameter %s was not valid UTF-8 or " - "could not be represented in the filesystem encoding." - % quote_output(local_dir_utf8)) + def __init__(self, client, upload_dircap, local_dir, inotify=None): + precondition_abspath(local_dir) + service.MultiService.__init__(self) + self._local_dir = abspath_expanduser_unicode(local_dir) self._client = client self._stats_provider = client.stats_provider self._convergence = client.convergence - self._local_path = FilePath(local_dir) + self._local_path = to_filepath(self._local_dir) self.is_upload_ready = False @@ -42,9 +34,13 @@ class DropUploader(service.MultiService): self._inotify = inotify if not self._local_path.exists(): - raise AssertionError("The '[drop_upload] local.directory' parameter was %s but there is no directory at that location." % quote_output(local_dir_u)) + raise AssertionError("The '[drop_upload] local.directory' parameter was %s " + "but there is no directory at that location." + % quote_local_unicode_path(local_dir)) if not self._local_path.isdir(): - raise AssertionError("The '[drop_upload] local.directory' parameter was %s but the thing at that location is not a directory." % quote_output(local_dir_u)) + raise AssertionError("The '[drop_upload] local.directory' parameter was %s " + "but the thing at that location is not a directory." + % quote_local_unicode_path(local_dir)) # TODO: allow a path rather than a cap URI. self._parent = self._client.create_node_from_uri(upload_dircap) diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index 0e646edf..43f193f4 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -27,7 +27,7 @@ BASECONFIG_I = ("[client]\n" "introducer.furl = %s\n" ) -class Basic(testutil.ReallyEqualMixin, unittest.TestCase): +class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.TestCase): def test_loadable(self): basedir = "test_client.Basic.test_loadable" os.mkdir(basedir) @@ -328,7 +328,8 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase): self.patch(allmydata.frontends.drop_upload, 'DropUploader', MockDropUploader) upload_dircap = "URI:DIR2:blah" - local_dir_utf8 = u"loc\u0101l_dir".encode('utf-8') + local_dir_u = self.unicode_or_fallback(u"loc\u0101l_dir", u"local_dir") + local_dir_utf8 = local_dir_u.encode('utf-8') config = (BASECONFIG + "[storage]\n" + "enabled = false\n" + @@ -356,7 +357,7 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase): self.failUnless(isinstance(uploader, MockDropUploader), uploader) self.failUnlessReallyEqual(uploader.client, c1) self.failUnlessReallyEqual(uploader.upload_dircap, upload_dircap) - self.failUnlessReallyEqual(uploader.local_dir_utf8, local_dir_utf8) + self.failUnlessReallyEqual(os.path.basename(uploader.local_dir), local_dir_u) self.failUnless(uploader.inotify is None, uploader.inotify) self.failUnless(uploader.running) diff --git a/src/allmydata/test/test_drop_upload.py b/src/allmydata/test/test_drop_upload.py index 30ff8811..8e871cbc 100644 --- a/src/allmydata/test/test_drop_upload.py +++ b/src/allmydata/test/test_drop_upload.py @@ -2,19 +2,20 @@ import os, sys from twisted.trial import unittest -from twisted.python import filepath, runtime +from twisted.python import runtime from twisted.internet import defer from allmydata.interfaces import IDirectoryNode, NoSuchChildError -from allmydata.util import fake_inotify -from allmydata.util.encodingutil import get_filesystem_encoding +from allmydata.util import fake_inotify, fileutil +from allmydata.util.encodingutil import get_filesystem_encoding, to_filepath from allmydata.util.consumer import download_to_data from allmydata.test.no_network import GridTestMixin from allmydata.test.common_util import ReallyEqualMixin, NonASCIIPathMixin from allmydata.test.common import ShouldFailMixin from allmydata.frontends.drop_upload import DropUploader +from allmydata.util.fileutil import abspath_expanduser_unicode class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonASCIIPathMixin): @@ -23,6 +24,10 @@ class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonA with the real INotify. """ + def setUp(self): + GridTestMixin.setUp(self) + temp = self.mktemp() + self.basedir = abspath_expanduser_unicode(temp.decode(get_filesystem_encoding())) def _get_count(self, name): return self.stats_provider.get_stats()["counters"].get(name, 0) @@ -84,23 +89,21 @@ class DropUploadTestMixin(GridTestMixin, ShouldFailMixin, ReallyEqualMixin, NonA # (otherwise we would get a defer.AlreadyCalledError). Should we be relying on that? self.uploader.set_uploaded_callback(d.callback) - path_u = os.path.join(self.local_dir, name_u) - if sys.platform == "win32": - path = filepath.FilePath(path_u) - else: - path = filepath.FilePath(path_u.encode(get_filesystem_encoding())) + path_u = abspath_expanduser_unicode(name_u, base=self.local_dir) + path = to_filepath(path_u) # We don't use FilePath.setContent() here because it creates a temporary file that # is renamed into place, which causes events that the test is not expecting. - f = open(path.path, "wb") + f = open(path_u, "wb") try: if temporary and sys.platform != "win32": - os.unlink(path.path) + os.unlink(path_u) f.write(data) finally: f.close() if temporary and sys.platform == "win32": - os.unlink(path.path) + os.unlink(path_u) + fileutil.flush_volume(path_u) self.notify_close_write(path) if temporary: @@ -123,10 +126,14 @@ class MockTest(DropUploadTestMixin, unittest.TestCase): """This can run on any platform, and even if twisted.internet.inotify can't be imported.""" def test_errors(self): - self.basedir = "drop_upload.MockTest.test_errors" self.set_up_grid() - errors_dir = os.path.join(self.basedir, "errors_dir") + + errors_dir = abspath_expanduser_unicode(u"errors_dir", base=self.basedir) os.mkdir(errors_dir) + not_a_dir = abspath_expanduser_unicode(u"NOT_A_DIR", base=self.basedir) + fileutil.write(not_a_dir, "") + magicfolderdb = abspath_expanduser_unicode(u"magicfolderdb", base=self.basedir) + doesnotexist = abspath_expanduser_unicode(u"doesnotexist", base=self.basedir) client = self.g.clients[0] d = client.create_dirnode() @@ -135,16 +142,10 @@ class MockTest(DropUploadTestMixin, unittest.TestCase): upload_dircap = n.get_uri() readonly_dircap = n.get_readonly_uri() - self.shouldFail(AssertionError, 'invalid local.directory', 'could not be represented', - DropUploader, client, upload_dircap, '\xFF', inotify=fake_inotify) self.shouldFail(AssertionError, 'nonexistent local.directory', 'there is no directory', - DropUploader, client, upload_dircap, os.path.join(self.basedir, "Laputa"), inotify=fake_inotify) - - fp = filepath.FilePath(self.basedir).child('NOT_A_DIR') - fp.touch() + DropUploader, client, upload_dircap, doesnotexist, inotify=fake_inotify) self.shouldFail(AssertionError, 'non-directory local.directory', 'is not a directory', - DropUploader, client, upload_dircap, fp.path, inotify=fake_inotify) - + DropUploader, client, upload_dircap, not_a_dir, inotify=fake_inotify) self.shouldFail(AssertionError, 'bad upload.dircap', 'does not refer to a directory', DropUploader, client, 'bad', errors_dir, inotify=fake_inotify) self.shouldFail(AssertionError, 'non-directory upload.dircap', 'does not refer to a directory', diff --git a/src/allmydata/util/encodingutil.py b/src/allmydata/util/encodingutil.py index d14b08f6..ae54afb9 100644 --- a/src/allmydata/util/encodingutil.py +++ b/src/allmydata/util/encodingutil.py @@ -8,6 +8,7 @@ from types import NoneType from allmydata.util.assertutil import precondition from twisted.python import usage +from twisted.python.filepath import FilePath from allmydata.util import log from allmydata.util.fileutil import abspath_expanduser_unicode @@ -35,9 +36,10 @@ def check_encoding(encoding): filesystem_encoding = None io_encoding = None is_unicode_platform = False +use_unicode_filepath = False def _reload(): - global filesystem_encoding, io_encoding, is_unicode_platform + global filesystem_encoding, io_encoding, is_unicode_platform, use_unicode_filepath filesystem_encoding = canonical_encoding(sys.getfilesystemencoding()) check_encoding(filesystem_encoding) @@ -61,6 +63,8 @@ def _reload(): is_unicode_platform = sys.platform in ["win32", "darwin"] + use_unicode_filepath = sys.platform == "win32" or hasattr(FilePath, '_asTextPath') + _reload() @@ -245,6 +249,23 @@ def quote_local_unicode_path(path, quotemarks=True): return quote_output(path, quotemarks=quotemarks, quote_newlines=True) +def to_filepath(path): + precondition(isinstance(path, basestring), path=path) + + if isinstance(path, unicode) and not use_unicode_filepath: + path = path.encode(filesystem_encoding) + + return FilePath(path) + +def unicode_from_filepath(fp): + precondition(isinstance(fp, FilePath), fp=fp) + + path = fp.path + if isinstance(path, bytes): + path = path.decode(filesystem_encoding) + + return path + def unicode_platform(): """ -- 2.45.2