git: 93aeb68a8933 - stable/14 - atf: Don't be deterred by weird umasks.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 31 May 2025 12:28:31 UTC
The branch stable/14 has been updated by des:
URL: https://cgit.FreeBSD.org/src/commit/?id=93aeb68a8933e7458c795b31589900cbeae49e38
commit 93aeb68a8933e7458c795b31589900cbeae49e38
Author: Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-05-24 10:03:22 +0000
Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-05-31 12:28:28 +0000
atf: Don't be deterred by weird umasks.
If the current umask is weird, ATF goes to great lengths to tell us that
it can't possibly work in these conditions, instead of just dealing with
it. This makes it unreasonably hard to use ATF to test how our own code
handles unusual umasks.
MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: igoro, kevans, ngie
Differential Revision: https://reviews.freebsd.org/D50267
(cherry picked from commit 1a6a36a98ad5dc86862541b91686b00cf88e9f76)
---
contrib/atf/atf-c/check_test.c | 8 +---
contrib/atf/atf-c/detail/fs.c | 91 ++++--------------------------------
contrib/atf/atf-c/detail/fs_test.c | 44 ++++++++---------
contrib/atf/atf-sh/atf-check_test.sh | 24 +++++-----
4 files changed, 43 insertions(+), 124 deletions(-)
diff --git a/contrib/atf/atf-c/check_test.c b/contrib/atf/atf-c/check_test.c
index adaca649be25..06e2fad2d9a4 100644
--- a/contrib/atf/atf-c/check_test.c
+++ b/contrib/atf/atf-c/check_test.c
@@ -458,8 +458,7 @@ ATF_TC(exec_umask);
ATF_TC_HEAD(exec_umask, tc)
{
atf_tc_set_md_var(tc, "descr", "Checks that atf_check_exec_array "
- "correctly reports an error if the umask is too "
- "restrictive to create temporary files");
+ "works regardless of umask");
}
ATF_TC_BODY(exec_umask, tc)
{
@@ -473,10 +472,7 @@ ATF_TC_BODY(exec_umask, tc)
argv[2] = NULL;
umask(0222);
- atf_error_t err = atf_check_exec_array(argv, &result);
- ATF_CHECK(atf_is_error(err));
- ATF_CHECK(atf_error_is(err, "invalid_umask"));
- atf_error_free(err);
+ RE(atf_check_exec_array(argv, &result));
atf_fs_path_fini(&process_helpers);
}
diff --git a/contrib/atf/atf-c/detail/fs.c b/contrib/atf/atf-c/detail/fs.c
index 5ff7648c3c7e..6ea825aa8e56 100644
--- a/contrib/atf/atf-c/detail/fs.c
+++ b/contrib/atf/atf-c/detail/fs.c
@@ -54,71 +54,13 @@
* Prototypes for auxiliary functions.
* --------------------------------------------------------------------- */
-static bool check_umask(const mode_t, const mode_t);
static atf_error_t copy_contents(const atf_fs_path_t *, char **);
-static mode_t current_umask(void);
static atf_error_t do_mkdtemp(char *);
static atf_error_t normalize(atf_dynstr_t *, char *);
static atf_error_t normalize_ap(atf_dynstr_t *, const char *, va_list);
static void replace_contents(atf_fs_path_t *, const char *);
static const char *stat_type_to_string(const int);
-/* ---------------------------------------------------------------------
- * The "invalid_umask" error type.
- * --------------------------------------------------------------------- */
-
-struct invalid_umask_error_data {
- /* One of atf_fs_stat_*_type. */
- int m_type;
-
- /* The original path causing the error. */
- /* XXX: Ideally this would be an atf_fs_path_t, but if we create it
- * from the error constructor, we cannot delete the path later on.
- * Can't remember why atf_error_new does not take a hook for
- * deletion. */
- char m_path[1024];
-
- /* The umask that caused the error. */
- mode_t m_umask;
-};
-typedef struct invalid_umask_error_data invalid_umask_error_data_t;
-
-static
-void
-invalid_umask_format(const atf_error_t err, char *buf, size_t buflen)
-{
- const invalid_umask_error_data_t *data;
-
- PRE(atf_error_is(err, "invalid_umask"));
-
- data = atf_error_data(err);
- snprintf(buf, buflen, "Could not create the temporary %s %s because "
- "it will not have enough access rights due to the current "
- "umask %05o", stat_type_to_string(data->m_type),
- data->m_path, (unsigned int)data->m_umask);
-}
-
-static
-atf_error_t
-invalid_umask_error(const atf_fs_path_t *path, const int type,
- const mode_t failing_mask)
-{
- atf_error_t err;
- invalid_umask_error_data_t data;
-
- data.m_type = type;
-
- strncpy(data.m_path, atf_fs_path_cstring(path), sizeof(data.m_path));
- data.m_path[sizeof(data.m_path) - 1] = '\0';
-
- data.m_umask = failing_mask;
-
- err = atf_error_new("invalid_umask", &data, sizeof(data),
- invalid_umask_format);
-
- return err;
-}
-
/* ---------------------------------------------------------------------
* The "unknown_file_type" error type.
* --------------------------------------------------------------------- */
@@ -162,14 +104,6 @@ unknown_type_error(const char *path, int type)
* Auxiliary functions.
* --------------------------------------------------------------------- */
-static
-bool
-check_umask(const mode_t exp_mode, const mode_t min_mode)
-{
- const mode_t actual_mode = (~current_umask() & exp_mode);
- return (actual_mode & min_mode) == min_mode;
-}
-
static
atf_error_t
copy_contents(const atf_fs_path_t *p, char **buf)
@@ -189,15 +123,6 @@ copy_contents(const atf_fs_path_t *p, char **buf)
return err;
}
-static
-mode_t
-current_umask(void)
-{
- const mode_t current = umask(0);
- (void)umask(current);
- return current;
-}
-
static
atf_error_t
do_mkdtemp(char *tmpl)
@@ -794,11 +719,10 @@ atf_fs_mkdtemp(atf_fs_path_t *p)
{
atf_error_t err;
char *buf;
+ mode_t mask;
- if (!check_umask(S_IRWXU, S_IRWXU)) {
- err = invalid_umask_error(p, atf_fs_stat_dir_type, current_umask());
- goto out;
- }
+ mask = umask(0);
+ umask(mask & 077);
err = copy_contents(p, &buf);
if (atf_is_error(err))
@@ -814,6 +738,7 @@ atf_fs_mkdtemp(atf_fs_path_t *p)
out_buf:
free(buf);
out:
+ umask(mask);
return err;
}
@@ -823,11 +748,10 @@ atf_fs_mkstemp(atf_fs_path_t *p, int *fdout)
atf_error_t err;
char *buf;
int fd;
+ mode_t mask;
- if (!check_umask(S_IRWXU, S_IRWXU)) {
- err = invalid_umask_error(p, atf_fs_stat_reg_type, current_umask());
- goto out;
- }
+ mask = umask(0);
+ umask(mask & 077);
err = copy_contents(p, &buf);
if (atf_is_error(err))
@@ -844,6 +768,7 @@ atf_fs_mkstemp(atf_fs_path_t *p, int *fdout)
out_buf:
free(buf);
out:
+ umask(mask);
return err;
}
diff --git a/contrib/atf/atf-c/detail/fs_test.c b/contrib/atf/atf-c/detail/fs_test.c
index 7812be0334b8..a9cc34a4f48b 100644
--- a/contrib/atf/atf-c/detail/fs_test.c
+++ b/contrib/atf/atf-c/detail/fs_test.c
@@ -896,25 +896,25 @@ ATF_TC_BODY(mkdtemp_err, tc)
static
void
do_umask_check(atf_error_t (*const mk_func)(atf_fs_path_t *),
- atf_fs_path_t *path, const mode_t test_mask,
- const char *str_mask, const char *exp_name)
+ atf_error_t (*const rm_func)(const atf_fs_path_t *),
+ atf_fs_path_t *tmpl, const mode_t test_mask,
+ const char *exp_name)
{
- char buf[1024];
- int old_umask;
+ atf_fs_path_t path;
+ int pre_mask, post_mask;
atf_error_t err;
- printf("Creating temporary %s with umask %s\n", exp_name, str_mask);
+ printf("Creating temporary %s with umask %05o\n", exp_name, test_mask);
- old_umask = umask(test_mask);
- err = mk_func(path);
- (void)umask(old_umask);
+ RE(atf_fs_path_copy(&path, tmpl));
- ATF_REQUIRE(atf_is_error(err));
- ATF_REQUIRE(atf_error_is(err, "invalid_umask"));
- atf_error_format(err, buf, sizeof(buf));
- ATF_CHECK(strstr(buf, exp_name) != NULL);
- ATF_CHECK(strstr(buf, str_mask) != NULL);
- atf_error_free(err);
+ pre_mask = umask(test_mask);
+ err = mk_func(&path);
+ post_mask = umask(pre_mask);
+
+ ATF_REQUIRE(!atf_is_error(err));
+ ATF_CHECK_EQ(post_mask, test_mask);
+ RE(rm_func(&path));
}
ATF_TC(mkdtemp_umask);
@@ -929,11 +929,11 @@ ATF_TC_BODY(mkdtemp_umask, tc)
RE(atf_fs_path_init_fmt(&p, "testdir.XXXXXX"));
- do_umask_check(atf_fs_mkdtemp, &p, 00100, "00100", "directory");
- do_umask_check(atf_fs_mkdtemp, &p, 00200, "00200", "directory");
- do_umask_check(atf_fs_mkdtemp, &p, 00400, "00400", "directory");
- do_umask_check(atf_fs_mkdtemp, &p, 00500, "00500", "directory");
- do_umask_check(atf_fs_mkdtemp, &p, 00600, "00600", "directory");
+ do_umask_check(atf_fs_mkdtemp, atf_fs_rmdir, &p, 00100, "directory");
+ do_umask_check(atf_fs_mkdtemp, atf_fs_rmdir, &p, 00200, "directory");
+ do_umask_check(atf_fs_mkdtemp, atf_fs_rmdir, &p, 00400, "directory");
+ do_umask_check(atf_fs_mkdtemp, atf_fs_rmdir, &p, 00500, "directory");
+ do_umask_check(atf_fs_mkdtemp, atf_fs_rmdir, &p, 00600, "directory");
atf_fs_path_fini(&p);
}
@@ -1039,9 +1039,9 @@ ATF_TC_BODY(mkstemp_umask, tc)
RE(atf_fs_path_init_fmt(&p, "testfile.XXXXXX"));
- do_umask_check(mkstemp_discard_fd, &p, 00100, "00100", "regular file");
- do_umask_check(mkstemp_discard_fd, &p, 00200, "00200", "regular file");
- do_umask_check(mkstemp_discard_fd, &p, 00400, "00400", "regular file");
+ do_umask_check(mkstemp_discard_fd, atf_fs_unlink, &p, 00100, "regular file");
+ do_umask_check(mkstemp_discard_fd, atf_fs_unlink, &p, 00200, "regular file");
+ do_umask_check(mkstemp_discard_fd, atf_fs_unlink, &p, 00400, "regular file");
atf_fs_path_fini(&p);
}
diff --git a/contrib/atf/atf-sh/atf-check_test.sh b/contrib/atf/atf-sh/atf-check_test.sh
index 9542dfb0bd9f..91d024a85bcd 100644
--- a/contrib/atf/atf-sh/atf-check_test.sh
+++ b/contrib/atf/atf-sh/atf-check_test.sh
@@ -389,21 +389,19 @@ stdin_body()
atf_fail "atf-check does not seem to respect stdin"
}
-atf_test_case invalid_umask
-invalid_umask_head()
+atf_test_case unusual_umask
+unusual_umask_head()
{
- atf_set "descr" "Tests for a correct error condition if the umask is" \
- "too restrictive"
+ atf_set "descr" "Tests that atf-check doesn't care about unusual umasks"
}
-invalid_umask_body()
+unusual_umask_body()
{
- umask 0222
- ${Atf_Check} false 2>stderr && \
- atf_fail "atf-check returned 0 but it should have failed"
- cat stderr
- grep 'temporary.*current umask.*0222' stderr >/dev/null || \
- atf_fail "atf-check did not report an error related to the" \
- "current umask"
+ for mask in 022 027 0222 0177 0777 ; do
+ umask $mask
+ ${Atf_Check} true || \
+ atf_fail "atf-check failed with umask $mask"
+ done
+ umask 022
}
atf_init_test_cases()
@@ -435,7 +433,7 @@ atf_init_test_cases()
atf_add_test_case stdin
- atf_add_test_case invalid_umask
+ atf_add_test_case unusual_umask
}
# vim: syntax=sh:expandtab:shiftwidth=4:softtabstop=4