From c3e1dca35b093df9fff240396f83ef9aef3e9baa Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 14 Aug 2020 09:36:26 -0700 Subject: [PATCH 1/2] util: Allow symlinks in directory_create_or_exist Commit 9f60a77e0b updated the check to avoid having files or other objects instead of a directory. This missed the valid case that there might be a symlink to a directory. Updated the check accordingly to allow symlinks to directories. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14166 Signed-off-by: Christof Schmitt Reviewed-by: Volker Lendecke (cherry picked from commit 672212cecdd7a7de40acdc81c56e2996ea82c090) --- lib/util/util.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/util/util.c b/lib/util/util.c index a90d48f6f1b..59dc7bd6b71 100644 --- a/lib/util/util.c +++ b/lib/util/util.c @@ -343,6 +343,7 @@ _PUBLIC_ bool directory_exist(const char *dname) /** * Try to create the specified directory if it didn't exist. + * A symlink to a directory is also accepted as a valid existing directory. * * @retval true if the directory already existed * or was successfully created. @@ -376,9 +377,22 @@ _PUBLIC_ bool directory_create_or_exist(const char *dname, return false; } - if (!S_ISDIR(sbuf.st_mode)) { - return false; + if (S_ISDIR(sbuf.st_mode)) { + return true; } + + if (S_ISLNK(sbuf.st_mode)) { + ret = stat(dname, &sbuf); + if (ret != 0) { + return false; + } + + if (S_ISDIR(sbuf.st_mode)) { + return true; + } + } + + return false; } return true; -- 2.20.1 From f9b72bd3d7a5c5d435b164301bad9be0bc7c59b4 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 14 Aug 2020 12:18:51 -0700 Subject: [PATCH 2/2] util: Add cmocka unit test for directory_create_or_exists BUG: https://bugzilla.samba.org/show_bug.cgi?id=14166 Signed-off-by: Christof Schmitt Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Sun Aug 16 07:06:59 UTC 2020 on sn-devel-184 (cherry picked from commit e89ec78e9a262a6e7bb9082323083eb5f1609655) --- lib/util/tests/test_util.c | 234 +++++++++++++++++++++++++++++++++++++ lib/util/wscript_build | 6 + selftest/tests.py | 2 + 3 files changed, 242 insertions(+) create mode 100644 lib/util/tests/test_util.c diff --git a/lib/util/tests/test_util.c b/lib/util/tests/test_util.c new file mode 100644 index 00000000000..eebba39e70c --- /dev/null +++ b/lib/util/tests/test_util.c @@ -0,0 +1,234 @@ +/* + * Unix SMB/CIFS implementation. + * + * Unit test for util.c + * + * Copyright (C) Christof Schmitt 2020 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#include "lib/util/util.c" +#include + +struct test_paths { + char testdir[PATH_MAX]; + char none[PATH_MAX]; + char dir[PATH_MAX]; + mode_t dir_mode; + char file[PATH_MAX]; + mode_t file_mode; + char symlink_none[PATH_MAX]; + char symlink_dir[PATH_MAX]; + char symlink_file[PATH_MAX]; +}; + +static int group_setup(void **state) +{ + struct test_paths *paths = NULL; + char *testdir = NULL; + int ret, fd; + + umask(0); + + paths = malloc(sizeof(struct test_paths)); + assert_non_null(paths); + + strlcpy(paths->testdir, tmpdir(), sizeof(paths->testdir)); + strlcat(paths->testdir, "/test_util_XXXXXX", sizeof(paths->testdir)); + testdir = mkdtemp(paths->testdir); + assert_non_null(testdir); + + strlcpy(paths->none, testdir, sizeof(paths->none)); + strlcat(paths->none, "/none", sizeof(paths->none)); + + strlcpy(paths->dir, testdir, sizeof(paths->dir)); + strlcat(paths->dir, "/dir", sizeof(paths->dir)); + paths->dir_mode = 0750; + ret = mkdir(paths->dir, paths->dir_mode); + assert_return_code(ret, errno); + + strlcpy(paths->file, testdir, sizeof(paths->file)); + strlcat(paths->file, "/file", sizeof(paths->file)); + paths->file_mode = 0640; + fd = creat(paths->file, paths->file_mode); + assert_return_code(fd, errno); + ret = close(fd); + assert_return_code(ret, errno); + + strlcpy(paths->symlink_none, testdir, sizeof(paths->symlink_none)); + strlcat(paths->symlink_none, "/symlink_none", + sizeof(paths->symlink_none)); + ret = symlink("/none", paths->symlink_none); + assert_return_code(ret, errno); + + strlcpy(paths->symlink_dir, testdir, sizeof(paths->symlink_dir)); + strlcat(paths->symlink_dir, "/symlink_dir", sizeof(paths->symlink_dir)); + ret = symlink(paths->dir, paths->symlink_dir); + assert_return_code(ret, errno); + + strlcpy(paths->symlink_file, testdir, sizeof(paths->symlink_file)); + strlcat(paths->symlink_file, "/symlink_file", + sizeof(paths->symlink_file)); + ret = symlink(paths->file, paths->symlink_file); + assert_return_code(ret, errno); + + *state = paths; + + return 0; +} + +static int group_teardown(void **state) +{ + struct test_paths *paths = *state; + int ret; + + return 0; + + ret = rmdir(paths->dir); + assert_return_code(ret, errno); + + ret = unlink(paths->file); + assert_return_code(ret, errno); + + ret = unlink(paths->symlink_none); + assert_return_code(ret, errno); + + ret = unlink(paths->symlink_dir); + assert_return_code(ret, errno); + + ret = unlink(paths->symlink_file); + assert_return_code(ret, errno); + + ret = unlink(paths->testdir); + assert_return_code(ret, errno); + + free(paths); + return 0; +} + +static void test_directory_create_or_exists_none(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->none, 0775); + assert_true(b); + + ret = lstat(paths->none, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, 0775); + assert_true(S_ISDIR(sbuf.st_mode)); + + ret = rmdir(paths->none); + assert_return_code(ret, errno); +} + +static void test_directory_create_or_exists_dir(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->dir, 770); + assert_true(b); + + ret = lstat(paths->dir, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, paths->dir_mode); + assert_true(S_ISDIR(sbuf.st_mode)); +} + +static void test_directory_create_or_exists_file(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->file, 770); + assert_false(b); + + ret = lstat(paths->file, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, paths->file_mode); + assert_true(S_ISREG(sbuf.st_mode)); +} + +static void test_directory_create_or_exists_symlink_none(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->symlink_none, 770); + assert_false(b); + + ret = lstat(paths->symlink_none, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, 0777); + assert_true(S_ISLNK(sbuf.st_mode)); +} + +static void test_directory_create_or_exists_symlink_dir(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->symlink_dir, 770); + assert_true(b); + + ret = lstat(paths->symlink_dir, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, 0777); + assert_true(S_ISLNK(sbuf.st_mode)); +} + +static void test_directory_create_or_exists_symlink_file(void **state) +{ + struct test_paths *paths = *state; + bool b; + struct stat sbuf; + int ret; + + b = directory_create_or_exist(paths->symlink_file, 770); + assert_false(b); + + ret = lstat(paths->symlink_file, &sbuf); + assert_return_code(ret, errno); + assert_int_equal(sbuf.st_mode & 0777, 0777); + assert_true(S_ISLNK(sbuf.st_mode)); +} + +int main(int argc, char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_directory_create_or_exists_none), + cmocka_unit_test(test_directory_create_or_exists_dir), + cmocka_unit_test(test_directory_create_or_exists_file), + cmocka_unit_test(test_directory_create_or_exists_symlink_none), + cmocka_unit_test(test_directory_create_or_exists_symlink_dir), + cmocka_unit_test(test_directory_create_or_exists_symlink_file), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + return cmocka_run_group_tests(tests, group_setup, group_teardown); +} diff --git a/lib/util/wscript_build b/lib/util/wscript_build index 807c62636fd..bf3e44bf1d2 100644 --- a/lib/util/wscript_build +++ b/lib/util/wscript_build @@ -305,3 +305,9 @@ else: deps='cmocka replace talloc samba-util', local_include=False, for_selftest=True) + + bld.SAMBA_BINARY('test_util', + source='tests/test_util.c', + deps='cmocka replace talloc samba-util', + local_include=False, + for_selftest=True); diff --git a/selftest/tests.py b/selftest/tests.py index dd3cc5ec8fb..6918e1306c3 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -396,6 +396,8 @@ plantestsuite("samba.unittests.byteorder_verify", "none", [os.path.join(bindir(), "default/lib/util/test_byteorder_verify")]) plantestsuite("samba.unittests.util_paths", "none", [os.path.join(bindir(), "default/lib/util/test_util_paths")]) +plantestsuite("samba.unittests.util", "none", + [os.path.join(bindir(), "default/lib/util/test_util")]) plantestsuite("samba.unittests.ntlm_check", "none", [os.path.join(bindir(), "default/libcli/auth/test_ntlm_check")]) plantestsuite("samba.unittests.gnutls", "none", -- 2.20.1