diff --git a/src/dwarfs/metadata_v2.cpp b/src/dwarfs/metadata_v2.cpp index 81dfc17e..badc90a4 100644 --- a/src/dwarfs/metadata_v2.cpp +++ b/src/dwarfs/metadata_v2.cpp @@ -1288,30 +1288,24 @@ int metadata_::access(inode_view iv, int mode, uid_t uid, int access_mode = 0; int e_mode = iv.mode(); - auto test = [this, e_mode, &access_mode](uint16_t r_bit, uint16_t w_bit, - uint16_t x_bit) { + auto test = [e_mode, &access_mode](uint16_t r_bit, uint16_t x_bit) { if (e_mode & r_bit) { access_mode |= R_OK; } - if (e_mode & w_bit) { - if (!options_.readonly) { - access_mode |= W_OK; - } - } if (e_mode & x_bit) { access_mode |= X_OK; } }; // Let's build the inode's access mask - test(S_IROTH, S_IWOTH, S_IXOTH); + test(S_IROTH, S_IXOTH); if (iv.getgid() == gid) { - test(S_IRGRP, S_IWGRP, S_IXGRP); + test(S_IRGRP, S_IXGRP); } if (iv.getuid() == uid) { - test(S_IRUSR, S_IWUSR, S_IXUSR); + test(S_IRUSR, S_IXUSR); } return (access_mode & mode) == mode ? 0 : EACCES; diff --git a/test/dwarfs_tools.cpp b/test/dwarfs_tools.cpp index 56bdab3c..a01a5b4f 100644 --- a/test/dwarfs_tools.cpp +++ b/test/dwarfs_tools.cpp @@ -37,6 +37,8 @@ #include #include +#include + #include "test_helpers.h" namespace { @@ -176,8 +178,26 @@ bool wait_until_file_ready(std::filesystem::path const& path, return true; } -bool is_writable(std::filesystem::path const& p) { - return ::access(p.c_str(), W_OK) == 0; +bool check_readonly(std::filesystem::path const& p, bool readonly = false) { + struct ::stat buf; + if (::stat(p.c_str(), &buf) != 0) { + throw std::runtime_error("could not stat " + p.string()); + } + + bool is_writable = (buf.st_mode & S_IWUSR) != 0; + + if (is_writable == readonly) { + std::cerr << "readonly=" << readonly << ", st_mode=" << fmt::format("{0:o}", buf.st_mode) << std::endl; + return false; + } + + if (::access(p.c_str(), W_OK) == 0) { + // access(W_OK) should never succeed + ::perror("access"); + return false; + } + + return true; } ::nlink_t num_hardlinks(std::filesystem::path const& p) { @@ -240,7 +260,7 @@ TEST(tools, everything) { ASSERT_TRUE(check_run(diff_bin, "-qruN", data_dir, mountpoint)); EXPECT_EQ(1, num_hardlinks(mountpoint / "format.sh")); - EXPECT_TRUE(is_writable(mountpoint / "format.sh")); + EXPECT_TRUE(check_readonly(mountpoint / "format.sh")); } { @@ -249,7 +269,7 @@ TEST(tools, everything) { ASSERT_TRUE(check_run(diff_bin, "-qruN", data_dir, mountpoint)); EXPECT_EQ(3, num_hardlinks(mountpoint / "format.sh")); - EXPECT_TRUE(is_writable(mountpoint / "format.sh")); + EXPECT_TRUE(check_readonly(mountpoint / "format.sh")); } { @@ -259,7 +279,7 @@ TEST(tools, everything) { ASSERT_TRUE(check_run(diff_bin, "-qruN", data_dir, mountpoint)); EXPECT_EQ(3, num_hardlinks(mountpoint / "format.sh")); - EXPECT_FALSE(is_writable(mountpoint / "format.sh")); + EXPECT_TRUE(check_readonly(mountpoint / "format.sh", true)); } if (std::filesystem::exists(fuse2_bin)) { @@ -268,7 +288,7 @@ TEST(tools, everything) { ASSERT_TRUE(check_run(diff_bin, "-qruN", data_dir, mountpoint)); EXPECT_EQ(1, num_hardlinks(mountpoint / "format.sh")); - EXPECT_TRUE(is_writable(mountpoint / "format.sh")); + EXPECT_TRUE(check_readonly(mountpoint / "format.sh")); } { @@ -289,7 +309,7 @@ TEST(tools, everything) { ASSERT_TRUE(check_run(diff_bin, "-qruN", data_dir, mountpoint)); EXPECT_EQ(3, num_hardlinks(mountpoint / "format.sh")); - EXPECT_TRUE(is_writable(mountpoint / "format.sh")); + EXPECT_TRUE(check_readonly(mountpoint / "format.sh")); } auto meta_export = td / "test.meta";