From a4e45a13b65c496a0c53b58992a4be2e3d923325 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Thu, 3 Sep 2020 20:16:33 +0300 Subject: [PATCH] Fix 'expand-file-name' for remote files This reverts most of commit 14fb657ba82da346d36f05f88da26f1c5498b798 and its followup fixes, and instead fixes the original bugs in a different manner that doesn't affect any unrelated use cases. As part of this, the code which caused 'expand-file-name' to enforce a trailing slash on expanded directories is removed, as this kind of semantic processing is outside of 'expand-file-name's scope. * src/fileio.c (Fexpand_file_name): If expanding default_directory yields a remote file name, call its handlers. (Bug#26911) (Bug#34834) * doc/lispref/files.texi (File Name Expansion): Remove the requirement that expanding a directory name yields a directory name, i.e. that the expansion must end in a slash. * etc/NEWS: Remove the announcement of the changed behavior of 'expand-file-name' wrt trailing slashes. * test/src/fileio-tests.el (fileio-tests--HOME-trailing-slash) (fileio-tests--expand-file-name-trailing-slash): Remove tests. * test/lisp/net/tramp-tests.el (tramp-test05-expand-file-name): No need to expect different results in Emacs 28 and later. --- doc/lispref/files.texi | 16 +------ etc/NEWS | 6 --- src/fileio.c | 86 +++++++++++++----------------------- test/lisp/net/tramp-tests.el | 9 ++-- test/src/fileio-tests.el | 37 ---------------- 5 files changed, 35 insertions(+), 119 deletions(-) diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index 090c54f8cd9..92cbc2a1c91 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -2438,26 +2438,14 @@ 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 1cb1b7ee4fd..38cfeaee9b8 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1181,12 +1181,6 @@ 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 c91af36fdf6..1e4ca82e5f3 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -827,9 +827,9 @@ the root directory. */) ptrdiff_t tlen; #ifdef DOS_NT int drive = 0; + bool collapse_newdir = true; bool is_escaped = 0; #endif /* DOS_NT */ - bool collapse_newdir = true; ptrdiff_t length, nbytes; Lisp_Object handler, result, handled_name; bool multibyte; @@ -947,6 +947,22 @@ the root directory. */) ) { default_directory = Fexpand_file_name (default_directory, Qnil); + + /* The above expansion might have produced a remote file name, + so give the handlers one last chance to DTRT. This can + happen when both NAME and DEFAULT-DIRECTORY arguments are + relative file names, and the buffer's default-directory is + remote. */ + handler = Ffind_file_name_handler (default_directory, + Qexpand_file_name); + if (!NILP (handler)) + { + handled_name = call3 (handler, Qexpand_file_name, + name, default_directory); + if (STRINGP (handled_name)) + return handled_name; + error ("Invalid handler in `file-name-handler-alist'"); + } } } multibyte = STRING_MULTIBYTE (name); @@ -1065,7 +1081,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 ( @@ -1183,7 +1199,9 @@ the root directory. */) newdir = SSDATA (hdir); newdirlim = newdir + SBYTES (hdir); } +#ifdef DOS_NT collapse_newdir = false; +#endif } else /* ~user/filename */ { @@ -1203,7 +1221,9 @@ the root directory. */) while (*++nm && !IS_DIRECTORY_SEP (*nm)) continue; +#ifdef DOS_NT collapse_newdir = false; +#endif } /* If we don't find a user of that name, leave the name @@ -1370,15 +1390,12 @@ the root directory. */) } #endif /* DOS_NT */ - length = newdirlim - newdir; - -#ifdef DOS_NT /* Ignore any slash at the end of newdir, unless newdir is just "/" or "//". */ + length = newdirlim - newdir; while (length > 1 && IS_DIRECTORY_SEP (newdir[length - 1]) && ! (length == 2 && IS_DIRECTORY_SEP (newdir[0]))) length--; -#endif /* Now concatenate the directory and name to new space in the stack frame. */ tlen = length + file_name_as_directory_slop + (nmlim - nm) + 1; @@ -1392,16 +1409,12 @@ the root directory. */) #else /* not DOS_NT */ target = SAFE_ALLOCA (tlen); #endif /* not DOS_NT */ + *target = 0; nbytes = 0; if (newdir) { -#ifndef DOS_NT - bool treat_as_absolute = !collapse_newdir; -#else - bool treat_as_absolute = !nm[0] || IS_DIRECTORY_SEP (nm[0]); -#endif - if (treat_as_absolute) + if (nm[0] == 0 || IS_DIRECTORY_SEP (nm[0])) { #ifdef DOS_NT /* If newdir is effectively "C:/", then the drive letter will have @@ -1413,23 +1426,13 @@ the root directory. */) && newdir[1] == '\0')) #endif { - /* With ~ or ~user, leave NEWDIR as-is to avoid transforming - it from a symlink (or a regular file!) into a directory. */ memcpy (target, newdir, length); + target[length] = 0; nbytes = length; } } else nbytes = file_name_as_directory (target, newdir, length, multibyte); - -#ifndef DOS_NT - /* If TARGET ends in a directory separator, omit leading - directory separators from NM so that concatenating a TARGET "/" - to an NM "/foo" does not result in the incorrect "//foo". */ - if (nbytes && IS_DIRECTORY_SEP (target[nbytes - 1])) - while (IS_DIRECTORY_SEP (nm[0])) - nm++; -#endif } memcpy (target + nbytes, nm, nmlim - nm + 1); @@ -1446,20 +1449,6 @@ the root directory. */) { *o++ = *p++; } -#ifndef DOS_NT - else if (p[1] == '.' && IS_DIRECTORY_SEP (p[2])) - { - /* 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 else if (p[1] == '.' && (IS_DIRECTORY_SEP (p[2]) || p[2] == 0)) @@ -1470,7 +1459,6 @@ the root directory. */) *o++ = *p; p += 2; } -#endif else if (p[1] == '.' && p[2] == '.' /* `/../' is the "superroot" on certain file systems. Turned off on DOS_NT systems because they have no @@ -1484,35 +1472,21 @@ the root directory. */) #endif && (IS_DIRECTORY_SEP (p[3]) || p[3] == 0)) { -#ifndef DOS_NT - while (o != target) - { - o--; - if (IS_DIRECTORY_SEP (*o)) - { - /* Keep "/" at the end of the name, for symlinks. */ - o += p[3] == 0; - - break; - } - } -#else -# ifdef WINDOWSNT +#ifdef WINDOWSNT char *prev_o = o; -# endif +#endif while (o != target && (--o, !IS_DIRECTORY_SEP (*o))) continue; -# ifdef WINDOWSNT +#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 +#endif /* Keep initial / only if this is the whole name. */ if (o == target && IS_ANY_SEP (*o) && p[3] == 0) ++o; -#endif 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 71c6302e0ee..c170d2c6bf0 100644 --- a/test/lisp/net/tramp-tests.el +++ b/test/lisp/net/tramp-tests.el @@ -2139,19 +2139,16 @@ is greater than 10. (expand-file-name "/method:host:/path/../file") "/method:host:/file")) (should (string-equal - (expand-file-name "/method:host:/path/.") - (if (tramp--test-emacs28-p) "/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/") - (if (tramp--test-emacs28-p) "/method:host:/path/" "/method:host:/path"))) + (expand-file-name "." "/method:host:/path/") "/method:host:/path")) (should (string-equal - (expand-file-name "" "/method:host:/path/") - (if (tramp--test-emacs28-p) "/method:host:/path/" "/method:host:/path"))) + (expand-file-name "" "/method:host:/path/") "/method:host:/path")) ;; Quoting local part. (should (string-equal diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el index bedda83bbdb..ed381d151ee 100644 --- a/test/src/fileio-tests.el +++ b/test/src/fileio-tests.el @@ -107,43 +107,6 @@ Also check that an encoding error can appear in a symlink." (setenv "HOME" "x:foo") (should (equal (expand-file-name "~/bar") "x:/foo/bar"))))) -(ert-deftest fileio-tests--HOME-trailing-slash () - "Test that expand-file-name of \"~\" respects trailing slash." - :expected-result (if (memq system-type '(windows-nt ms-dos)) - :failed - :passed) - (let ((process-environment (copy-sequence process-environment))) - (dolist (home - (if (memq system-type '(windows-nt ms-dos)) - '("c:/a/b/c" "c:/a/b/c/") - '("/a/b/c" "/a/b/c/"))) - (setenv "HOME" home) - (should (equal (expand-file-name "~") (expand-file-name home)))))) - -(ert-deftest fileio-tests--expand-file-name-trailing-slash () - (dolist (fooslashalias '("foo/" "foo//" "foo/." "foo//." "foo///././." - "foo/a/..")) - (if (memq system-type '(windows-nt ms-dos)) - (progn - (should (equal (expand-file-name fooslashalias "c:/") "c:/foo/")) - (should (equal (expand-file-name (concat "c:/" fooslashalias)) - "c:/foo/")) - (should (equal (expand-file-name "." "c:/usr/spool/") - "c:/usr/spool/")) - (should (equal (expand-file-name "" "c:/usr/spool/") - "c:/usr/spool/"))) - (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. - (if (memq system-type '(windows-nt ms-dos)) - (progn - (should (equal (expand-file-name "c:/a/b/c/..") "c:/a/b/")) - (should (equal (expand-file-name "c:/a/b/c/../") "c:/a/b/"))) - (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