Subject: Re: bug#36502: Fwd: bug#36502: 27.0.50; infinite loop in file-name-case-insensitive-p

From: Ken Brown <[email-masked]>
Date: Mon, 22 Jul 2019 02:16:47 +0000

On 7/21/2019 10:21 AM, Eli Zaretskii wrote:
>> Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>,
>>   "[email-masked]" <[email-masked]>,
>>   "[email-masked]" <[email-masked]>,
>>   "[email-masked]" <[email-masked]>,
>>   "[email-masked]" <[email-masked]>
>> From: Paul Eggert <[email-masked]>
>> Date: Sat, 20 Jul 2019 19:32:30 -0700
>>
>> My impression from looking at uses of file-name-absolute-p
>> is that changing it to return nil here would improve correctness of the callers,
>> though there would be a performance cost for this case of course.
> 
> I agree.
> 
> So Ken, please push your changes, and I would appreciate if you could
> also make the change in file-name-absolute-p (as a separate change),
> if you have time.

Patch attached.  I didn't add a NEWS entry.  Do you think this requires one?

Ken
From 3657bc708e96c6955f64e111daf52c3c3667a6db Mon Sep 17 00:00:00 2001
From: Ken Brown <[email-masked]>
Date: Sun, 21 Jul 2019 22:04:07 -0400
Subject: [PATCH] Fix file-name-absolute-p for names starting with '~'

A file name starting with "~user" is now considered absolute only
if "user" is a valid login name.  See the discussion starting at
Bug#36502#64.
* src/fileio.c (expand_tilde): New static function, extracted from
Fexpand_file_name.
(Fexpand_file_name, file_name_absolute_p)
(search_embedded_absfilename):  Use it.
(Ffile_name_absolute_p): Update doc string.
* doc/lispref/files.texi (Relative File Names): Document the new
behavior of file-name-absolute-p.
---
 doc/lispref/files.texi | 10 ++++-
 src/fileio.c           | 84 ++++++++++++++++++++++++++++--------------
 2 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 0519f787dc..18325b2049 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2154,7 +2154,11 @@ Relative File Names
 
 @defun file-name-absolute-p filename
 This function returns @code{t} if file @var{filename} is an absolute
-file name or begins with @samp{~}, @code{nil} otherwise.
+file name or begins with @samp{~}, provided that an initial
+@samp{~@var{user}} corresponds to a valid login name @var{user}.  The
+function returns @code{nil} otherwise.  In the following examples,
+assume that there is a user named @samp{rms} but no user named
+@samp{quux}.
 
 @example
 @group
@@ -2162,6 +2166,10 @@ Relative File Names
      @result{} t
 @end group
 @group
+(file-name-absolute-p "~quux/foo")
+     @result{} nil
+@end group
+@group
 (file-name-absolute-p "rms/foo")
      @result{} nil
 @end group
diff --git a/src/fileio.c b/src/fileio.c
index 4c7625cad4..42455bb499 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -744,6 +744,35 @@ file_name_absolute_no_tilde_p (Lisp_Object name)
   return IS_ABSOLUTE_FILE_NAME (SSDATA (name));
 }
 
+/* NAME must start with "~user", where USER is not empty, followed by
+   nothing or a directory separator.  LENGTH is the length of the
+   "~user" prefix.  Return the absolute file name of USER's home
+   directory, or Qnil if there is no user named USER.  */
+static Lisp_Object
+expand_tilde (const char *name, ptrdiff_t length)
+{
+  char *p;
+  const char *dir;
+  struct passwd *pw;
+  Lisp_Object result;
+  USE_SAFE_ALLOCA;
+
+  p = SAFE_ALLOCA (length);
+  memcpy (p, name + 1, length - 1);
+  p[length] = 0;
+
+  pw = getpwnam (p);
+  if (pw)
+    {
+      dir = pw->pw_dir;
+      result = make_unibyte_string (dir, strlen (dir));
+    }
+  else
+    result = Qnil;
+  SAFE_FREE ();
+  return result;
+}
+
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
        doc: /* Convert filename NAME to absolute, and canonicalize it.
 Second arg DEFAULT-DIRECTORY is directory to start with if NAME is relative
@@ -788,7 +817,6 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
   char *target;
 
   ptrdiff_t tlen;
-  struct passwd *pw;
 #ifdef DOS_NT
   int drive = 0;
   bool collapse_newdir = true;
@@ -1153,25 +1181,20 @@ DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 	}
       else			/* ~user/filename */
 	{
-	  char *o, *p;
+	  char *p;
 	  for (p = nm; *p && !IS_DIRECTORY_SEP (*p); p++)
 	    continue;
-	  o = SAFE_ALLOCA (p - nm + 1);
-	  memcpy (o, nm, p - nm);
-	  o[p - nm] = 0;
 
 	  block_input ();
-	  pw = getpwnam (o + 1);
+	  Lisp_Object tem = expand_tilde (nm, p - nm);
 	  unblock_input ();
-	  if (pw)
+	  if (!NILP (tem))
 	    {
-	      Lisp_Object tem;
-
-	      newdir = pw->pw_dir;
-	      /* `getpwnam' may return a unibyte string, which will
-		 bite us when we expect the directory to be multibyte.  */
-	      tem = make_unibyte_string (newdir, strlen (newdir));
+	      newdir = SSDATA (tem);
 	      newdirlim = newdir + SBYTES (tem);
+	      /* `getpwnam', which was used in `expand_tilde', may
+		 return a unibyte string, which will bite us when we
+		 expect the directory to be multibyte.  */
 	      if (multibyte && !STRING_MULTIBYTE (tem))
 		{
 		  hdir = DECODE_FILE (tem);
@@ -1670,13 +1693,23 @@ DEAFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
 bool
 file_name_absolute_p (const char *filename)
 {
-  return
-    (IS_DIRECTORY_SEP (*filename) || *filename == '~'
+  bool result
+    = (IS_DIRECTORY_SEP (*filename)
 #ifdef DOS_NT
-     || (IS_DRIVE (*filename) && IS_DEVICE_SEP (filename[1])
-	 && IS_DIRECTORY_SEP (filename[2]))
+       || (IS_DRIVE (*filename) && IS_DEVICE_SEP (filename[1])
+	   && IS_DIRECTORY_SEP (filename[2]))
 #endif
      );
+  if (!result && *filename == '~')
+    {
+      const char *p;
+      for (p = filename + 1; *p && !IS_DIRECTORY_SEP (*p); p++);
+      if (p == filename + 1)
+	result = true;
+      else
+	result = !NILP (expand_tilde (filename, p - filename));
+    }
+  return result;
 }
 
 /* Put into BUF the concatenation of DIR and FILE, with an intervening
@@ -1794,20 +1827,13 @@ search_embedded_absfilename (char *nm, char *endp)
 	  for (s = p; *s && !IS_DIRECTORY_SEP (*s); s++);
 	  if (p[0] == '~' && s > p + 1)	/* We've got "/~something/".  */
 	    {
-	      USE_SAFE_ALLOCA;
-	      char *o = SAFE_ALLOCA (s - p + 1);
-	      struct passwd *pw;
-	      memcpy (o, p, s - p);
-	      o [s - p] = 0;
-
 	      /* If we have ~user and `user' exists, discard
 		 everything up to ~.  But if `user' does not exist, leave
 		 ~user alone, it might be a literal file name.  */
 	      block_input ();
-	      pw = getpwnam (o + 1);
+	      Lisp_Object tem = expand_tilde (p, s - p);
 	      unblock_input ();
-	      SAFE_FREE ();
-	      if (pw)
+	      if (!NILP (tem))
 		return p;
 	    }
 	  else
@@ -2698,8 +2724,10 @@ DEFUN ("make-symbolic-link", Fmake_symbolic_link, Smake_symbolic_link, 2, 3,
 
 DEFUN ("file-name-absolute-p", Ffile_name_absolute_p, Sfile_name_absolute_p,
        1, 1, 0,
-       doc: /* Return t if FILENAME is an absolute file name or starts with `~'.
-On Unix, absolute file names start with `/'.  */)
+       doc: /* Return t if FILENAME is an absolute file name.
+On Unix, absolute file names are usually required to start with `/';
+but here we also allow FILENAME to start with `~', provided that an
+initial ~USER corresponds to a valid login name USER.  */)
   (Lisp_Object filename)
 {
   CHECK_STRING (filename);
-- 
2.21.0