fix(metadata): access() should work according to mode flags

src/dwarfs/metadata_v2.cpp

@@ -1611,25 +1611,34 @@ int metadata_<LoggerPolicy>::access(inode_view iv, int mode, uid_t uid,
 
   int access_mode = 0;
 
-  auto test = [e_mode = iv.mode(), &access_mode](fs::perms r_bit,
+  auto test = [e_mode = iv.mode(), &access_mode, readonly = options_.readonly](
-                                                 fs::perms x_bit) {
+                  fs::perms r_bit, fs::perms w_bit, fs::perms x_bit) {
     if (e_mode & uint16_t(r_bit)) {
       access_mode |= R_OK;
     }
+    if (e_mode & uint16_t(w_bit)) {
+      if (!readonly) {
+        access_mode |= W_OK;
+      }
+    }
     if (e_mode & uint16_t(x_bit)) {
+#ifdef _WIN32
+      access_mode |= 1; // Windows has no notion of X_OK
+#else
       access_mode |= X_OK;
+#endif
     }
   };
 
   // Let's build the inode's access mask
-  test(fs::perms::others_read, fs::perms::others_exec);
+  test(fs::perms::others_read, fs::perms::others_write, fs::perms::others_exec);
 
   if (iv.getgid() == gid) {
-    test(fs::perms::group_read, fs::perms::group_exec);
+    test(fs::perms::group_read, fs::perms::group_write, fs::perms::group_exec);
   }
 
   if (iv.getuid() == uid) {
-    test(fs::perms::owner_read, fs::perms::owner_exec);
+    test(fs::perms::owner_read, fs::perms::owner_write, fs::perms::owner_exec);
   }
 
   return (access_mode & mode) == mode ? 0 : EACCES;

test/test_helpers.h

@@ -46,14 +46,14 @@ namespace dwarfs::test {
 struct simplestat {
   file_stat::ino_type ino;
   file_stat::mode_type mode;
-  file_stat::nlink_type nlink;
+  file_stat::nlink_type nlink{1};
-  file_stat::uid_type uid;
+  file_stat::uid_type uid{0};
-  file_stat::gid_type gid;
+  file_stat::gid_type gid{0};
-  file_stat::off_type size;
+  file_stat::off_type size{0};
-  file_stat::dev_type rdev;
+  file_stat::dev_type rdev{0};
-  file_stat::time_type atime;
+  file_stat::time_type atime{0};
-  file_stat::time_type mtime;
+  file_stat::time_type mtime{0};
-  file_stat::time_type ctime;
+  file_stat::time_type ctime{0};
 
   posix_file_type::value type() const {
     return static_cast<posix_file_type::value>(mode & posix_file_type::mask);

test/tool_main_test.cpp

@@ -797,6 +797,61 @@ TEST(mkdwarfs_test, metadata_time_resolution) {
   EXPECT_EQ(1080, stat.ctime);
 }
 
+TEST(mkdwarfs_test, metadata_access) {
+#ifdef _WIN32
+  static constexpr int const x_ok = 1;
+#else
+  static constexpr int const x_ok = X_OK;
+#endif
+
+  auto t = mkdwarfs_tester::create_empty();
+  t.add_root_dir();
+  t.os->add("access", {1001, 040742, 1, 222, 3333});
+  t.run("-l3 -i / -o -");
+
+  {
+    auto fs = t.fs_from_stdout();
+
+    auto iv = fs.find("/access");
+    ASSERT_TRUE(iv);
+
+    EXPECT_EQ(0, fs.access(*iv, F_OK, 1, 1));
+
+    EXPECT_EQ(EACCES, fs.access(*iv, R_OK, 1, 1));
+    EXPECT_EQ(0, fs.access(*iv, W_OK, 1, 1));
+    EXPECT_EQ(EACCES, fs.access(*iv, x_ok, 1, 1));
+
+    EXPECT_EQ(0, fs.access(*iv, R_OK, 1, 3333));
+    EXPECT_EQ(0, fs.access(*iv, W_OK, 1, 3333));
+    EXPECT_EQ(EACCES, fs.access(*iv, x_ok, 1, 3333));
+
+    EXPECT_EQ(0, fs.access(*iv, R_OK, 222, 7));
+    EXPECT_EQ(0, fs.access(*iv, W_OK, 222, 7));
+    EXPECT_EQ(0, fs.access(*iv, x_ok, 222, 7));
+  }
+
+  {
+    auto fs = t.fs_from_stdout({.metadata = {.readonly = true}});
+
+    auto iv = fs.find("/access");
+    ASSERT_TRUE(iv);
+
+    EXPECT_EQ(0, fs.access(*iv, F_OK, 1, 1));
+
+    EXPECT_EQ(EACCES, fs.access(*iv, R_OK, 1, 1));
+    EXPECT_EQ(EACCES, fs.access(*iv, W_OK, 1, 1));
+    EXPECT_EQ(EACCES, fs.access(*iv, x_ok, 1, 1));
+
+    EXPECT_EQ(0, fs.access(*iv, R_OK, 1, 3333));
+    EXPECT_EQ(EACCES, fs.access(*iv, W_OK, 1, 3333));
+    EXPECT_EQ(EACCES, fs.access(*iv, x_ok, 1, 3333));
+
+    EXPECT_EQ(0, fs.access(*iv, R_OK, 222, 7));
+    EXPECT_EQ(EACCES, fs.access(*iv, W_OK, 222, 7));
+    EXPECT_EQ(0, fs.access(*iv, x_ok, 222, 7));
+  }
+}
+
 TEST(mkdwarfs_test, chmod_norm) {
   std::string const image_file = "test.dwarfs";
 

test/tools_test.cpp

@@ -573,7 +573,7 @@ class driver_runner {
 #endif
 };
 
-bool check_readonly(fs::path const& p, bool readonly = false) {
+bool check_readonly(fs::path const& p, bool readonly) {
   auto st = fs::status(p);
   bool is_writable =
       (st.permissions() & fs::perms::owner_write) != fs::perms::none;
@@ -585,10 +585,21 @@ bool check_readonly(fs::path const& p, bool readonly = false) {
   }
 
 #ifndef _WIN32
-  if (::access(p.string().c_str(), W_OK) == 0) {
+  {
-    // access(W_OK) should never succeed
+    auto r = ::access(p.string().c_str(), W_OK);
-    ::perror("access");
+
-    return false;
+    if (readonly) {
+      if (r != -1 || errno != EACCES) {
+        std::cerr << "access(" << p << ", W_OK) = " << r << " (errno=" << errno
+                  << ") [readonly]\n";
+        return false;
+      }
+    } else {
+      if (r != 0) {
+        std::cerr << "access(" << p << ", W_OK) = " << r << "\n";
+        return false;
+      }
+    }
   }
 #endif