bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* patch, have find_in_given_path return file only
@ 2020-03-07  3:48 Dmitry Goncharov via Gnulib discussion list
  2020-03-07 18:57 ` Bruno Haible
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Goncharov via Gnulib discussion list @ 2020-03-07  3:48 UTC (permalink / raw
  To: bug-gnulib

Good morning.

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.
The fix is likely needed for the windows specific piece of code in
find_in_given_path as well.

regards, Dmitry



diff --git a/lib/findprog-in.c b/lib/findprog-in.c
index c254f2f..d89ec00 100644
--- a/lib/findprog-in.c
+++ b/lib/findprog-in.c
@@ -26,6 +26,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/stat.h>

 #include "filename.h"
 #include "concat-filename.h"
@@ -190,6 +191,7 @@ find_in_given_path (const char *progname, const char *path,
           dir = ".";

         /* Try all platform-dependent suffixes.  */
+        struct stat st;
         for (i = 0; i < sizeof (suffixes) / sizeof (suffixes[0]); i++)
           {
             const char *suffix = suffixes[i];
@@ -208,7 +210,8 @@ find_in_given_path (const char *progname, const char *path,
                    use it.  On other systems, let's hope that this program
                    is not installed setuid or setgid, so that it is ok to
                    call access() despite its design flaw.  */
-                if (eaccess (progpathname, X_OK) == 0)
+                if (eaccess (progpathname, X_OK) == 0 &&
+                        stat(progpathname, &st) == 0 && S_ISREG(st.st_mode))
                   {
                     /* Found!  */
                     if (strcmp (progpathname, progname) == 0)


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: patch, have find_in_given_path return file only
  2020-03-07  3:48 patch, have find_in_given_path return file only Dmitry Goncharov via Gnulib discussion list
@ 2020-03-07 18:57 ` Bruno Haible
  2020-04-10 14:00   ` Bruno Haible
  0 siblings, 1 reply; 3+ messages in thread
From: Bruno Haible @ 2020-03-07 18:57 UTC (permalink / raw
  To: bug-gnulib, Dmitry Goncharov

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  <bruno@clisp.org>

	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



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: patch, have find_in_given_path return file only
  2020-03-07 18:57 ` Bruno Haible
@ 2020-04-10 14:00   ` Bruno Haible
  0 siblings, 0 replies; 3+ messages in thread
From: Bruno Haible @ 2020-04-10 14:00 UTC (permalink / raw
  To: bug-gnulib; +Cc: Dmitry Goncharov

On 2020-03-07 I wrote:
> 2020-03-07  Bruno Haible  <bruno@clisp.org>
> 
> 	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.

Oops, this patch patch was incomplete. Includes and module description updates
were missing. I had to revert it.

Now I'm committing a completed variant.


2020-04-10  Bruno Haible  <bruno@clisp.org>

	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.
	* modules/findprog (Depends-on): Add sys_stat, stat.
	* modules/findprog-lgpl (Depends-on): Likewise.

	* lib/progreloc.c (maybe_executable): When the file found in a PATH
	element is a directory, continue searching.
	* lib/relocwrapper.c: Update comments.
	* modules/relocatable-prog-wrapper (Files): Add m4/largefile.m4.
	(configure.ac-early): New section.

diff --git a/lib/findprog.c b/lib/findprog.c
index d0d4179..b562e9d 100644
--- a/lib/findprog.c
+++ b/lib/findprog.c
@@ -25,6 +25,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#if !(defined _WIN32 || defined __CYGWIN__ || defined __EMX__ || defined __DJGPP__)
+# include <sys/stat.h>
+#endif
 
 /* Avoid collision between findprog.c and findprog-lgpl.c.  */
 #if IN_FINDPROG_LGPL || ! GNULIB_FINDPROG_LGPL
@@ -105,22 +108,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 b555211..45be1ca 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
diff --git a/lib/relocwrapper.c b/lib/relocwrapper.c
index dfe7e4f..e3dc197 100644
--- a/lib/relocwrapper.c
+++ b/lib/relocwrapper.c
@@ -19,14 +19,15 @@
    relocwrapper
     -> progname
     -> progreloc
+       -> stat
+          -> filename
+          -> pathmax
+          -> verify
        -> areadlink
           -> careadlinkat
              -> allocator
           -> readlink
              -> stat
-                -> filename
-                -> pathmax
-                -> verify
        -> canonicalize-lgpl
           -> filename
           -> malloca
diff --git a/modules/findprog b/modules/findprog
index d48158b..b5f3c20 100644
--- a/modules/findprog
+++ b/modules/findprog
@@ -9,9 +9,11 @@ m4/eaccess.m4
 
 Depends-on:
 stdbool
+sys_stat
 xalloc
 xconcat-filename
 access
+stat
 unistd
 
 configure.ac:
diff --git a/modules/findprog-lgpl b/modules/findprog-lgpl
index 477eccb..3c56f02 100644
--- a/modules/findprog-lgpl
+++ b/modules/findprog-lgpl
@@ -10,9 +10,11 @@ m4/eaccess.m4
 
 Depends-on:
 stdbool
+sys_stat
 strdup
 concat-filename
 access
+stat
 unistd
 
 configure.ac:
diff --git a/modules/relocatable-prog-wrapper b/modules/relocatable-prog-wrapper
index 3cc3d76..f97fdae 100644
--- a/modules/relocatable-prog-wrapper
+++ b/modules/relocatable-prog-wrapper
@@ -25,6 +25,7 @@ lib/relocatable.c
 lib/setenv.c
 lib/c-ctype.h
 lib/c-ctype.c
+m4/largefile.m4
 m4/malloca.m4
 m4/canonicalize.m4
 m4/eealloc.m4
@@ -50,6 +51,9 @@ string
 verify
 xalloc-oversized
 
+configure.ac-early:
+AC_REQUIRE([AC_SYS_LARGEFILE])
+
 configure.ac:
 AC_REQUIRE([AC_C_RESTRICT])
 gl_FUNC_READLINK_SEPARATE



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-10 14:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-07  3:48 patch, have find_in_given_path return file only Dmitry Goncharov via Gnulib discussion list
2020-03-07 18:57 ` Bruno Haible
2020-04-10 14:00   ` Bruno Haible

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).