From 14fb657ba82da346d36f05f88da26f1c5498b798 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 92cbc2a1c91..090c54f8cd9 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -2438,14 +2438,26 @@ This is for the sake of filesystems that have the concept of a 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 fae65a22271..14a75ca75d7 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 37072d9b6bd..b70dff1c22c 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -1065,7 +1065,7 @@ the root directory. */) #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 @@ the root directory. */) 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 @@ the root directory. */) { *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 @@ the root directory. */) #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 7e9ae33f843..aa00c07f794 100644 --- a/test/lisp/net/tramp-tests.el +++ b/test/lisp/net/tramp-tests.el @@ -2131,16 +2131,16 @@ is greater than 10. (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 @@ is greater than 10. "/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 96b03a01372..1516590795e 100644 --- a/test/src/fileio-tests.el +++ b/test/src/fileio-tests.el @@ -108,6 +108,17 @@ Also check that an encoding error can appear in a symlink." (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.39.2