Re: patch, have find_in_given_path return file only

[Bruno Haible]

Hi,

Dmitry Goncharov wrote:
> A user reporter an issue in gnu make.
> https://savannah.gnu.org/bugs/index.php?57962.
> The issue is that find_in_given_path returns a directory which matches
> the desired name.

Indeed, while POSIX
  
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08
only says
  "The list shall be searched from beginning to end, applying the filename
   to each prefix, until an executable file with the specified name and
   appropriate execution permissions is found."
all shells that I've tested continue searching when they've hit a directory.

Test case:

$ mkdir p1 p2 p1/foo
$ { echo '#!/bin/sh'; echo 'echo "foo found in p2"'; } > p2/foo
$ chmod a+x p2/foo
$ PATH=p1:p2:$PATH foo

Also, all shell follow symlinks. Test case:

$ mkdir p1 p2 p1/bar
$ ln -s bar p1/foo
$ { echo '#!/bin/sh'; echo 'echo "foo found in p2"'; } > p2/foo
$ chmod a+x p2/foo
$ PATH=p1:p2:$PATH foo
foo found in p2
$ sh -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ dash -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ bash -c 'PATH=p1:p2:$PATH foo'
foo found in p2

> +                        stat(progpathname, &st) == 0 && S_ISREG(st.st_mode))

However, excluding other types of files (character devices, sockets, etc.)
is not what all shells do. Test cases:

1) For character devices:

$ ln -s bar p1/foo
$ sudo mknod p1/bar c 5 0
$ sudo chmod a+x p1/bar
$ dash -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ sh -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ bash -c 'PATH=p1:p2:$PATH foo'
(hangs reading)

2) For sockets:

On Linux:

$ mkdir p1 p2 p1/bar
$ ln -s /tmp/.X11-unix/X0 p1/foo
$ { echo '#!/bin/sh'; echo 'echo "foo found in p2"'; } > p2/foo
$ chmod a+x p2/foo
$ PATH=p1:p2:$PATH foo
$ dash -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ bash -c 'PATH=p1:p2:$PATH foo'
bash: p1/foo: No such device or address

and on Solaris:

$ ln -sf /var/run/.inetd.uds p1/foo
$ sh -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ ksh -c 'PATH=p1:p2:$PATH foo'
foo found in p2
$ bash -c 'PATH=p1:p2:$PATH foo'
bash: p1/foo: Permission denied

So, let me ignore directories and symlinks to directories only.

> The fix is likely needed for the windows specific piece of code in
> find_in_given_path as well.

Possibly, but I don't have time to make the necessary investigations on
Windows right now. Please feel free to make the investigations and provide
a patch, if you like to.


2020-03-07  Bruno Haible  <address@hidden>

        findprog, relocatable-prog: Ignore directories during PATH search.
        Reported by Frederick Eaton via Dmitry Goncharov in
        <https://lists.gnu.org/archive/html/bug-gnulib/2020-03/msg00003.html>.
        * lib/findprog.c (find_in_path): When the file found in a PATH element
        is a directory, continue searching.
        * lib/progreloc.c (maybe_executable): Likewise.

diff --git a/lib/findprog.c b/lib/findprog.c
index d0d4179..d834301 100644
--- a/lib/findprog.c
+++ b/lib/findprog.c
@@ -105,22 +105,29 @@ find_in_path (const char *progname)
          design flaw.  */
       if (eaccess (progpathname, X_OK) == 0)
         {
-          /* Found!  */
-          if (strcmp (progpathname, progname) == 0)
+          /* Check that the progpathname does not point to a directory.  */
+          struct stat statbuf;
+
+          if (stat (progpathname, &statbuf) >= 0
+              && ! S_ISDIR (statbuf.st_mode))
             {
-              free (progpathname);
-
-              /* Add the "./" prefix for real, that xconcatenated_filename()
-                 optimized away.  This avoids a second PATH search when the
-                 caller uses execlp/execvp.  */
-              progpathname = XNMALLOC (2 + strlen (progname) + 1, char);
-              progpathname[0] = '.';
-              progpathname[1] = '/';
-              memcpy (progpathname + 2, progname, strlen (progname) + 1);
+              /* Found!  */
+              if (strcmp (progpathname, progname) == 0)
+                {
+                  free (progpathname);
+
+                  /* Add the "./" prefix for real, that 
xconcatenated_filename()
+                     optimized away.  This avoids a second PATH search when the
+                     caller uses execlp/execvp.  */
+                  progpathname = XNMALLOC (2 + strlen (progname) + 1, char);
+                  progpathname[0] = '.';
+                  progpathname[1] = '/';
+                  memcpy (progpathname + 2, progname, strlen (progname) + 1);
+                }
+
+              free (path);
+              return progpathname;
             }
-
-          free (path);
-          return progpathname;
         }
 
       free (progpathname);
diff --git a/lib/progreloc.c b/lib/progreloc.c
index 2acf3fb..04cef32 100644
--- a/lib/progreloc.c
+++ b/lib/progreloc.c
@@ -154,7 +154,7 @@ static int executable_fd = -1;
 /* Define this function only when it's needed.  */
 #if !(defined WINDOWS_NATIVE || defined __EMX__)
 
-/* Tests whether a given pathname may belong to the executable.  */
+/* Tests whether a given filename may belong to the executable.  */
 static bool
 maybe_executable (const char *filename)
 {
@@ -173,18 +173,20 @@ maybe_executable (const char *filename)
       struct stat statfile;
 
       if (fstat (executable_fd, &statexe) >= 0)
-        {
-          if (stat (filename, &statfile) < 0)
-            return false;
-          if (!(statfile.st_dev
+        return (stat (filename, &statfile) >= 0
+                && statfile.st_dev
                 && statfile.st_dev == statexe.st_dev
-                && statfile.st_ino == statexe.st_ino))
-            return false;
-        }
+                && statfile.st_ino == statexe.st_ino);
     }
 # endif
 
-  return true;
+  /* Check that the filename does not point to a directory.  */
+  {
+    struct stat statfile;
+
+    return (stat (filename, &statfile) >= 0
+            && ! S_ISDIR (statfile.st_mode));
+  }
 }
 
 #endif