Merge branch '251-slash'

closes #251

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

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