gtestutils: Don't follow symlinks when deleting tests' tempdir

Previously, when cleaning up the temporary directory tree created by
passing G_TEST_OPTION_ISOLATE_DIRS, any symbolic links in that tree
would be followed recursively. If the test case has created a symbolic
link in its temporary directory to a directory outside that tree, this
could lead to unexpected data loss; in particular, if the test case
author has (unwisely) created a symbolic link to /, they could lose all
data on the system.

On systems that have the ftw.h header, replace the current rm_rf()
implementation with one that uses nftw() to perform a depth-first
traversal (FTW_DEPTH) without following symbolic links (FTW_PHYS), and
without crossing mount points (FTW_MOUNT) in case a test has mounted
some other filesystem in the temporary directory.

The callback logs any error to the standard error stream, but returns 0
rather than -1 to allow nftw() to keep walking the tree rather than
terminating immediately. Suppose we are trying to clean up the following
tree:

    tmpdir/
      a/
        f/ (directory not readable for some reason)
        g/
          p
      b/
        c
        d

Since tmpdir/a/f is not readable, we can expect to fail to delete
tmpdir/a/f, tmpdir/a and tmpdir; but it is preferable to (attempt) to
delete the rest of the tree rather than failing outright. The cost is
that three errors will be logged (for tmpdir/a/f, tmpdir/a and tmpdir).

nftw() is part of POSIX.1-2001, SUSv1, and glibc ≥ 2.1, so should be
available on effectively every platform except Windows. (And Windows
does not enable symbolic links by default so the developer error is less
likely to occur there.)

The macOS ftw(3) manpage says:

> These functions are provided for compatibility with legacy code.  New
> code should use the fts(3) functions.

fts(3) does not seem to be part of any standard, but it does seem to be
equally widely supported. The Linux manpages do not indicate that
nftw() is deprecated.

Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/3290
This commit is contained in:
Will Thompson 2024-04-22 09:07:15 +01:00
parent dc24e78286
commit 5f6dda5bdc
2 changed files with 60 additions and 0 deletions

View File

@ -30,6 +30,9 @@
#include <fcntl.h> #include <fcntl.h>
#include <unistd.h> #include <unistd.h>
#endif #endif
#ifdef HAVE_FTW_H
#include <ftw.h>
#endif
#include <string.h> #include <string.h>
#include <stdlib.h> #include <stdlib.h>
#include <stdio.h> #include <stdio.h>
@ -1407,6 +1410,61 @@ parse_args (gint *argc_p,
*argc_p = e; *argc_p = e;
} }
#ifdef HAVE_FTW_H
static int
rm_rf_nftw_visitor (const char *fpath,
const struct stat *sb,
int typeflag,
struct FTW *ftwbuf)
{
switch (typeflag)
{
case FTW_DP:
case FTW_D:
case FTW_DNR:
if (g_rmdir (fpath) != 0)
{
int errsv = errno;
g_printerr ("Unable to clean up temporary directory %s: %s\n",
fpath,
g_strerror (errsv));
}
break;
default:
if (g_remove (fpath) != 0)
{
int errsv = errno;
g_printerr ("Unable to clean up temporary file %s: %s\n",
fpath,
g_strerror (errsv));
}
break;
}
return 0;
}
static void
rm_rf (const gchar *path)
{
/* nopenfd specifies the maximum number of directories that [n]ftw() will
* hold open simultaneously. Rather than attempt to determine how many file
* descriptors are available, we assume that 5 are available when tearing
* down a test case; if that assumption is invalid, the only harm is leaving
* a temporary directory on disk.
*/
const int nopenfd = 5;
int ret = nftw (path, rm_rf_nftw_visitor, nopenfd, FTW_DEPTH | FTW_MOUNT | FTW_PHYS);
if (ret != 0)
{
int errsv = errno;
g_printerr ("Unable to clean up temporary directory %s: %s\n",
path,
g_strerror (errsv));
}
}
#else
/* A fairly naive `rm -rf` implementation to clean up after unit tests. */ /* A fairly naive `rm -rf` implementation to clean up after unit tests. */
static void static void
rm_rf (const gchar *path) rm_rf (const gchar *path)
@ -1433,6 +1491,7 @@ rm_rf (const gchar *path)
g_rmdir (path); g_rmdir (path);
} }
#endif
/* Implement the %G_TEST_OPTION_ISOLATE_DIRS option, iff its enabled. Create /* Implement the %G_TEST_OPTION_ISOLATE_DIRS option, iff its enabled. Create
* a temporary directory for this unit test (disambiguated using @test_run_name) * a temporary directory for this unit test (disambiguated using @test_run_name)

View File

@ -385,6 +385,7 @@ headers = [
'dirent.h', # MSC does not come with this by default 'dirent.h', # MSC does not come with this by default
'float.h', 'float.h',
'fstab.h', 'fstab.h',
'ftw.h',
'grp.h', 'grp.h',
'inttypes.h', 'inttypes.h',
'libproc.h', 'libproc.h',