Subject: bug#26911: 25.2; eshell "cd .." doesn't work correctly with TRAMP

From: Paul Eggert <[email-masked]>
Date: Wed, 26 Aug 2020 13:39:52 -0700

I ran into the expand-file-name bug outside of Tramp, and fixed it by installing 
the attached patch into Emacs master. I hope it fixes Bug#26911 too.

With luck it (or something like it) might even bear on Bug#34834 too, so I'll cc 
this message there. (I don't use MS-Windows or Tramp so am not good at testing 
in those environments.)
>From 14fb657ba82da346d36f05f88da26f1c5498b798 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 26 Aug 2020 13:25:35 -0700
Subject: [PATCH] Fix expand-file-name symlink-to-dir bug

Problem reported by Yegor Timoshenko (Bug#26911),
and I ran into it myself recently in normal-top-level.
* doc/lispref/files.texi (File Name Expansion), etc/NEWS: Mention this.
* src/fileio.c (Fexpand_file_name): Expand "/a/b/." to "/a/b/" not
"/a/b", to avoid misinterpreting a symlink "/a/b".  Similarly,
expand "/a/b/c/.." to "/a/b/" not "/a/b".
* test/lisp/net/tramp-tests.el (tramp-test05-expand-file-name):
Adjust to match new behavior.
(tramp-test05-expand-file-name-relative): This test now succeeds,
at least on Fedora 31.
* test/src/fileio-tests.el:
(fileio-tests--expand-file-name-trailing-slash)	New test.
---
 doc/lispref/files.texi       | 16 ++++++++++++++--
 etc/NEWS                     |  6 ++++++
 src/fileio.c                 | 37 +++++++++++++++++++++---------------
 test/lisp/net/tramp-tests.el | 11 ++++-------
 test/src/fileio-tests.el     | 11 +++++++++++
 5 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 92cbc2a1c9..090c54f8cd 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2438,14 +2438,26 @@ File Name Expansion
 superroot above the root directory @file{/}.  On other filesystems,
 @file{/../} is interpreted exactly the same as @file{/}.
 
+If a filename must be that of a directory, its expansion must be too.
+For example, if a filename ends in @samp{/} or @samp{/.} or @samp{/..}
+then its expansion ends in @samp{/} so that it cannot be
+misinterpreted as the name of a symbolic link:
+
+@example
+@group
+(expand-file-name "/a///b//.")
+     @result{} "/a/b/"
+@end group
+@end example
+
 Expanding @file{.} or the empty string returns the default directory:
 
 @example
 @group
 (expand-file-name "." "/usr/spool/")
-     @result{} "/usr/spool"
+     @result{} "/usr/spool/"
 (expand-file-name "" "/usr/spool/")
-     @result{} "/usr/spool"
+     @result{} "/usr/spool/"
 @end group
 @end example
 
diff --git a/etc/NEWS b/etc/NEWS
index fae65a2227..14a75ca75d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1157,6 +1157,12 @@ region's (or buffer's) end.
 This function can be used by modes to add elements to the
 'choice' customization type of a variable.
 
++++
+** 'expand-file-name' no longer omits a trailing slash if the omission
+changes the filename's meaning.  E.g., (expand-file-name "/a/b/.") now
+returns "/a/b/" not "/a/b", which might be misinterpreted as the name
+of a symbolic link rather than of the directory it points to.
+
 +++
 ** New function 'file-modes-number-to-symbolic' to convert a numeric
 file mode specification into symbolic form.
diff --git a/src/fileio.c b/src/fileio.c
index 37072d9b6b..b70dff1c22 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -1065,7 +1065,7 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 #endif /* WINDOWSNT */
 #endif /* DOS_NT */
 
-  /* If nm is absolute, look for `/./' or `/../' or `//''sequences; if
+  /* If nm is absolute, look for "/./" or "/../" or "//" sequences; if
      none are found, we can probably return right away.  We will avoid
      allocating a new string if name is already fully expanded.  */
   if (
@@ -1398,7 +1398,7 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 
   if (newdir)
     {
-      if (nm[0] == 0 || IS_DIRECTORY_SEP (nm[0]))
+      if (IS_DIRECTORY_SEP (nm[0]))
 	{
 #ifdef DOS_NT
 	  /* If newdir is effectively "C:/", then the drive letter will have
@@ -1433,14 +1433,16 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 	  {
 	    *o++ = *p++;
 	  }
-	else if (p[1] == '.'
-		 && (IS_DIRECTORY_SEP (p[2])
-		     || p[2] == 0))
+	else if (p[1] == '.' && IS_DIRECTORY_SEP (p[2]))
 	  {
-	    /* If "/." is the entire filename, keep the "/".  Otherwise,
-	       just delete the whole "/.".  */
-	    if (o == target && p[2] == '\0')
-	      *o++ = *p;
+	    /* Replace "/./" with "/".  */
+	    p += 2;
+	  }
+	else if (p[1] == '.' && !p[2])
+	  {
+	    /* At the end of the file name, replace "/." with "/".
+	       The trailing "/" is for symlinks.  */
+	    *o++ = *p;
 	    p += 2;
 	  }
 	else if (p[1] == '.' && p[2] == '.'
@@ -1459,18 +1461,23 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 #ifdef WINDOWSNT
 	    char *prev_o = o;
 #endif
-	    while (o != target && (--o, !IS_DIRECTORY_SEP (*o)))
-	      continue;
+	    while (o != target)
+	      {
+		o--;
+		if (IS_DIRECTORY_SEP (*o))
+		  {
+		    /* Keep "/" at the end of the name, for symlinks.  */
+		    o += p[3] == 0;
+
+		    break;
+		  }
+	      }
 #ifdef WINDOWSNT
 	    /* Don't go below server level in UNC filenames.  */
 	    if (o == target + 1 && IS_DIRECTORY_SEP (*o)
 		&& IS_DIRECTORY_SEP (*target))
 	      o = prev_o;
-	    else
 #endif
-	    /* Keep initial / only if this is the whole name.  */
-	    if (o == target && IS_ANY_SEP (*o) && p[3] == 0)
-	      ++o;
 	    p += 3;
 	  }
 	else if (IS_DIRECTORY_SEP (p[1])
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 7e9ae33f84..aa00c07f79 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2131,16 +2131,16 @@ tramp-test05-expand-file-name
       (expand-file-name "/method:host:/path/../file") "/method:host:/file"))
     (should
      (string-equal
-      (expand-file-name "/method:host:/path/.") "/method:host:/path"))
+      (expand-file-name "/method:host:/path/.") "/method:host:/path/"))
     (should
      (string-equal
       (expand-file-name "/method:host:/path/..") "/method:host:/"))
     (should
      (string-equal
-      (expand-file-name "." "/method:host:/path/") "/method:host:/path"))
+      (expand-file-name "." "/method:host:/path/") "/method:host:/path/"))
     (should
      (string-equal
-      (expand-file-name "" "/method:host:/path/") "/method:host:/path"))
+      (expand-file-name "" "/method:host:/path/") "/method:host:/path/"))
     ;; Quoting local part.
     (should
      (string-equal
@@ -2155,12 +2155,9 @@ tramp-test05-expand-file-name
       "/method:host:/:/~/path/file"))))
 
 ;; The following test is inspired by Bug#26911 and Bug#34834.  They
-;; are rather bugs in `expand-file-name', and it fails for all Emacs
-;; versions.  Test added for later, when they are fixed.
+;; were bugs in `expand-file-name'.
 (ert-deftest tramp-test05-expand-file-name-relative ()
   "Check `expand-file-name'."
-  ;; Mark as failed until bug has been fixed.
-  :expected-result :failed
   (skip-unless (tramp--test-enabled))
 
   ;; These are the methods the test doesn't fail.
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index 96b03a0137..1516590795 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -108,6 +108,17 @@ fileio-tests--relative-HOME
       (should (equal (expand-file-name "~/bar") "x:/foo/bar")))
     (setenv "HOME" old-home)))
 
+(ert-deftest fileio-tests--expand-file-name-trailing-slash ()
+  (dolist (fooslashalias '("foo/" "foo//" "foo/." "foo//." "foo///././."
+                           "foo/a/.."))
+    (should (equal (expand-file-name fooslashalias "/") "/foo/"))
+    (should (equal (expand-file-name (concat "/" fooslashalias)) "/foo/")))
+  (should (equal (expand-file-name "." "/usr/spool/") "/usr/spool/"))
+  (should (equal (expand-file-name "" "/usr/spool/") "/usr/spool/"))
+  ;; Trailing "B/C/.." means B must be a directory.
+  (should (equal (expand-file-name "/a/b/c/..") "/a/b/"))
+  (should (equal (expand-file-name "/a/b/c/../") "/a/b/")))
+
 (ert-deftest fileio-tests--insert-file-interrupt ()
   (let ((text "-*- coding: binary -*-\n\xc3\xc3help")
         f)
-- 
2.17.1