diff --git a/src/wormhole/cli/cmd_send.py b/src/wormhole/cli/cmd_send.py index 773dd45..bbba2ce 100644 --- a/src/wormhole/cli/cmd_send.py +++ b/src/wormhole/cli/cmd_send.py @@ -243,11 +243,43 @@ class Sender: # be unicode or bytes. We need it to be something that can be # os.path.joined with the unicode args.what . what = os.path.join(args.cwd, args.what) - what = what.rstrip(os.sep) + + # We always tell the receiver to create a file (or directory) with the + # same basename as what the local user typed, even if the local object + # is a symlink to something with a different name. The normpath() is + # there to remove trailing slashes. + basename = os.path.basename(os.path.normpath(what)) + assert basename != "", what # normpath shouldn't allow this + + # We use realpath() instead of normpath() to locate the actual + # file/directory, because the path might contain symlinks, and + # normpath() would collapse those before resolving them. + # test_cli.OfferData.test_symlink_collapse tests this. + + # Unfortunately on windows, realpath() (on py3) is built out of + # normpath() because of a py2-era belief that windows lacks a working + # os.path.islink(): see https://bugs.python.org/issue9949 . The + # consequence is that "wormhole send PATH" might send the wrong file, + # if: + # * we're running on windows + # * PATH goes down through a symlink and then up with parent-directory + # navigation (".."), then back down again + # * the back-down-again portion of the path also exists under the + # original directory (an error is thrown if not) + + # I'd like to fix this. The core issue is sending directories with a + # trailing slash: we need to 1: open the right directory, and 2: strip + # the right parent path out of the filenames we get from os.walk(). We + # used to use what.rstrip() for this, but bug #251 reported this + # failing on windows-with-bash. realpath() works in both those cases, + # but fails with the up-down symlinks situation. I think we'll need to + # find a third way to strip the trailing slash reliably in all + # environments. + + what = os.path.realpath(what) if not os.path.exists(what): raise TransferError("Cannot send: no file/directory named '%s'" % args.what) - basename = os.path.basename(what) if os.path.isfile(what): # we're sending a file diff --git a/src/wormhole/test/test_cli.py b/src/wormhole/test/test_cli.py index ffc995a..6b0265c 100644 --- a/src/wormhole/test/test_cli.py +++ b/src/wormhole/test/test_cli.py @@ -183,6 +183,66 @@ class OfferData(unittest.TestCase): self.assertEqual(str(e), "'%s' is neither file nor directory" % filename) + def test_symlink(self): + if not hasattr(os, 'symlink'): + raise unittest.SkipTest("host OS does not support symlinks") + # build A/B1 -> B2 (==A/B2), and A/B2/C.txt + parent_dir = self.mktemp() + os.mkdir(parent_dir) + os.mkdir(os.path.join(parent_dir, "B2")) + with open(os.path.join(parent_dir, "B2", "C.txt"), "wb") as f: + f.write(b"success") + os.symlink("B2", os.path.join(parent_dir, "B1")) + # now send "B1/C.txt" from A, and it should get the right file + self.cfg.cwd = parent_dir + self.cfg.what = os.path.join("B1", "C.txt") + d, fd_to_send = build_offer(self.cfg) + self.assertEqual(d["file"]["filename"], "C.txt") + self.assertEqual(fd_to_send.read(), b"success") + + def test_symlink_collapse(self): + if not hasattr(os, 'symlink'): + raise unittest.SkipTest("host OS does not support symlinks") + # build A/B1, A/B1/D.txt + # A/B2/C2, A/B2/D.txt + # symlink A/B1/C1 -> A/B2/C2 + parent_dir = self.mktemp() + os.mkdir(parent_dir) + os.mkdir(os.path.join(parent_dir, "B1")) + with open(os.path.join(parent_dir, "B1", "D.txt"), "wb") as f: + f.write(b"fail") + os.mkdir(os.path.join(parent_dir, "B2")) + os.mkdir(os.path.join(parent_dir, "B2", "C2")) + with open(os.path.join(parent_dir, "B2", "D.txt"), "wb") as f: + f.write(b"success") + os.symlink(os.path.abspath(os.path.join(parent_dir, "B2", "C2")), + os.path.join(parent_dir, "B1", "C1")) + # Now send "B1/C1/../D.txt" from A. The correct traversal will be: + # * start: A + # * B1: A/B1 + # * C1: follow symlink to A/B2/C2 + # * ..: climb to A/B2 + # * D.txt: open A/B2/D.txt, which contains "success" + # If the code mistakenly uses normpath(), it would do: + # * normpath turns B1/C1/../D.txt into B1/D.txt + # * start: A + # * B1: A/B1 + # * D.txt: open A/B1/D.txt , which contains "fail" + self.cfg.cwd = parent_dir + self.cfg.what = os.path.join("B1", "C1", os.pardir, "D.txt") + d, fd_to_send = build_offer(self.cfg) + self.assertEqual(d["file"]["filename"], "D.txt") + self.assertEqual(fd_to_send.read(), b"success") + if os.name == "nt": + test_symlink_collapse.todo = "host OS has broken os.path.realpath()" + # ntpath.py's realpath() is built out of normpath(), and does not + # follow symlinks properly, so this test always fails. "wormhole send + # PATH" on windows will do the wrong thing. See + # https://bugs.python.org/issue9949" for details. I'm making this a + # TODO instead of a SKIP because 1: this causes an observable + # misbehavior (albeit in rare circumstances), 2: it probably used to + # work (sometimes, but not in #251). See cmd_send.py for more notes. + class LocaleFinder: def __init__(self): self._run_once = False