bug#28156: Emacs quietly munges symlink contents

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

Eli Zaretskii wrote:

> We have quoting for these cases.

Quoting does not work for these cases. If I try to rename a symlink to the 
literal string '~eggert' on my machine, Emacs will misbehave as described and 
there is no way to quote the string naming the symlink to fix this.

> doesn't Emacs behave here like Unix shell commands do?

They differ in many ways. Two examples. First, the Unix shell command 'ln -s 
~/$$ def' expands both the "~" and the "$$", whereas (make-symbolic-link "~/$$" 
"def") expands only the "~". Second, the Unix shell command "ln -s def" creates 
a symlink to itself, whereas (make-symbolic-link "def") is an error. Briefly, 
the Unix shell commands are higher-level than the Emacs primitives.

The main points here are (1) Emacs functions should let users create and 
manipulate whatever symlinks they want to, and (2) the documentation has long 
said that symlinks are not expanded.

> If make-symbolic-link would create a link like this:
> 
>   ttt -> ~/bin/etags
> 
> (which is what your proposed change does, right?), then programs which
> follow the link will probably fail

Yes, they will fail unless there is a directory named '~'. That is the intent. 
If I want to create a symlink to my home directory, I can use expand-file-name 
on the link target, before calling make-symbolic-link.

> this text should be clarified.

Sure, that's easy. Revised patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-munge-contents-of-local-symbolic-links.patch --]
[-- Type: text/x-patch; name="0001-Do-not-munge-contents-of-local-symbolic-links.patch", Size: 6547 bytes --]

From 6a3f716feac756a6e80ba4c571155377bd1cba28 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 20 Aug 2017 03:13:52 -0700
Subject: [PATCH] Do not munge contents of local symbolic links

This lets Emacs deal with arbitrary local symlinks without
mishandling their contents.  For example,
(progn (shell-command "ln -fs '~' 'x'") (rename-file "x" "/tmp/x"))
now consistently creates a symbolic link from '/tmp/x' to '~'.
Formerly, it did that only if the working directory was on the
same filesystem as /tmp; otherwise, it expanded the '~' to
the user's home directory.
* etc/NEWS: Document the change.
* src/fileio.c (Fmake_symbolic_link): Do not expand leading
"~" in the target, and look for special constructs only in
the new link name, not the target.
(emacs_readlinkat): Do not preprend "/:" to the link target if
it starts with "/" and contains ":" before NUL.
* test/lisp/net/tramp-tests.el (tramp-test21-file-links):
Adjust to new behavior.
* test/src/fileio-tests.el (try-link): Rename from try-char,
and accept a string instead of a char.  All uses changed.
(fileio-tests--symlink-failure): Also test leading ~, and "/:",
to test the new behavior.
---
 etc/NEWS                     | 15 +++++++++++++++
 src/fileio.c                 | 18 ------------------
 test/lisp/net/tramp-tests.el |  9 ++++++---
 test/src/fileio-tests.el     | 21 ++++++++++-----------
 4 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7774d75..3bafb87 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1204,6 +1204,21 @@ instead of to utf-8.  Before this change, Emacs would sometimes
 mishandle file names containing these control characters.
 
 +++
+** 'file-attributes', 'file-symlink-p' and 'make-symbolic-link' no
+longer quietly mutate the target of a local symbolic link, so that
+Emacs can access and copy them reliably regardless of their contents.
+Two changes are involved.  First, 'file-attributes' and
+'file-symlink-p' no longer prepend "/:" to symbolic links whose
+targets begin with "/" and contain ":".  For example, if a symbolic
+link "x" has a target "/y:z", (file-symlink-p "x") now returns "/y:z"
+rather than "/:/y:z".  Second, 'make-symbolic-link' no longer expands
+"~" at the start of a link target before creating the link.  For
+example, (make-symbolic-link "~y" "x") now creates a symbolic link
+with target being the string "~y" and not the home directory of the
+user y.  To create a symbolic link to y's home directory, use
+(make-symbolic-link (expand-file-name "~y") "x").
+
++++
 ** Module functions are now implemented slightly differently; in
 particular, the function 'internal--module-call' has been removed.
 Code that depends on undocumented internals of the module system might
diff --git a/src/fileio.c b/src/fileio.c
index f954ac1..ce243c3 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2423,21 +2423,8 @@ This happens for interactive use with M-x.  */)
   Lisp_Object encoded_target, encoded_linkname;
 
   CHECK_STRING (target);
-  /* If the link target has a ~, we must expand it to get
-     a truly valid file name.  Otherwise, do not expand;
-     we want to permit links to relative file names.  */
-  if (SREF (target, 0) == '~')
-    target = Fexpand_file_name (target, Qnil);
-
   linkname = expand_cp_target (target, linkname);
 
-  /* If the file name has special constructs in it,
-     call the corresponding file handler.  */
-  handler = Ffind_file_name_handler (target, Qmake_symbolic_link);
-  if (!NILP (handler))
-    return call4 (handler, Qmake_symbolic_link, target,
-		  linkname, ok_if_already_exists);
-
   /* If the new link name has special constructs in it,
      call the corresponding file handler.  */
   handler = Ffind_file_name_handler (linkname, Qmake_symbolic_link);
@@ -2635,11 +2622,6 @@ emacs_readlinkat (int fd, char const *filename)
     return Qnil;
 
   val = build_unibyte_string (buf);
-  if (buf[0] == '/' && strchr (buf, ':'))
-    {
-      AUTO_STRING (slash_colon, "/:");
-      val = concat2 (slash_colon, val);
-    }
   if (buf != readlink_buf)
     xfree (buf);
   val = DECODE_FILE (val);
diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 9dc276b..0740996 100644
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -2538,13 +2538,16 @@ tramp--test-backtrace
 	    (should-error (make-symbolic-link tmp-name1 tmp-name2))
 	    (make-symbolic-link tmp-name1 tmp-name2 'ok-if-already-exists)
 	    (should (file-symlink-p tmp-name2))
-	    ;; `tmp-name3' is a local file name.
-	    (should-error (make-symbolic-link tmp-name1 tmp-name3)))
+	    ;; `tmp-name3' is a local file name, so its target is not
+	    ;; interpreted.
+	    (make-symbolic-link tmp-name1 tmp-name3)
+	    (should (equal tmp-name1 (file-symlink-p tmp-name3))))
 
 	;; Cleanup.
 	(ignore-errors
 	  (delete-file tmp-name1)
-	  (delete-file tmp-name2)))
+	  (delete-file tmp-name2)
+	  (delete-file tmp-name3)))
 
       ;; Check `add-name-to-file'.
       (unwind-protect
diff --git a/test/src/fileio-tests.el b/test/src/fileio-tests.el
index 2ef1b55..5103d2f 100644
--- a/test/src/fileio-tests.el
+++ b/test/src/fileio-tests.el
@@ -19,14 +19,13 @@
 
 (require 'ert)
 
-(defun try-char (char link)
-  (let ((target (string char)))
-    (make-symbolic-link target link)
-    (let* ((read-link (file-symlink-p link))
-           (failure (unless (string-equal target read-link)
-                      (list 'string-equal target read-link))))
-      (delete-file link)
-      failure)))
+(defun try-link (target link)
+  (make-symbolic-link target link)
+  (let* ((read-link (file-symlink-p link))
+         (failure (unless (string-equal target read-link)
+                    (list 'string-equal target read-link))))
+    (delete-file link)
+    failure))
 
 (defun fileio-tests--symlink-failure ()
   (let* ((dir (make-temp-file "fileio" t))
@@ -36,9 +35,9 @@ fileio-tests--symlink-failure
               (char 0))
           (while (and (not failure) (< char 127))
             (setq char (1+ char))
-            (unless (= char ?~)
-              (setq failure (try-char char link))))
-          failure)
+            (setq failure (try-link (string char) link)))
+          (or failure
+              (try-link "/:" link)))
       (delete-directory dir t))))
 
 (ert-deftest fileio-tests--odd-symlink-chars ()
-- 
2.7.4