Add tests for embed_file

PiperOrigin-RevId: 623470785
Change-Id: I40c3bf7667ffcf888d241ca1d5f96b74e3e5ad68

sandboxed_api/BUILD.bazel

@@ -57,6 +57,18 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "embed_file_test",
+    srcs = ["embed_file_test.cc"],
+    copts = sapi_platform_copts(),
+    deps = [
+        ":embed_file",
+        "@com_google_absl//absl/memory",
+        "@com_google_absl//absl/strings",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 # The main Sandboxed-API library
 cc_library(
     name = "sapi",
@@ -83,6 +95,7 @@ cc_library(
         "//sandboxed_api/sandbox2:client",
         "//sandboxed_api/sandbox2:comms",
         "//sandboxed_api/util:file_base",
+        "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:runfiles",
         "//sandboxed_api/util:status",
         "@com_google_absl//absl/base:core_headers",

sandboxed_api/CMakeLists.txt

@@ -56,10 +56,10 @@ target_link_libraries(sapi_embed_file
   PRIVATE absl::strings
           sandbox2::util
           sapi::base
-          sapi::fileops
           sapi::raw_logging
   PUBLIC absl::flat_hash_map
          absl::synchronization
+         sapi::fileops
 )
 
 # sandboxed_api:sapi
@@ -194,6 +194,17 @@ target_link_libraries(sapi_client
 )
 
 if(BUILD_TESTING AND SAPI_BUILD_TESTING AND NOT CMAKE_CROSSCOMPILING)
+  # sandboxed_api:embed_file_test
+  add_executable(embed_file_test
+    embed_file_test.cc
+  )
+  target_link_libraries(embed_file_test PRIVATE
+    absl::memory
+    absl::strings
+    sapi::embed_file
+    sapi::test_main
+  )
+
   # sandboxed_api:testing
   add_library(sapi_testing ${SAPI_LIB_TYPE}
     testing.cc

sandboxed_api/embed_file.cc

@@ -31,6 +31,8 @@ namespace sapi {
 
 namespace {
 
+using ::sapi::file_util::fileops::FDCloser;
+
 #ifndef F_ADD_SEALS
 #define F_ADD_SEALS 1033
 #define F_SEAL_SEAL 0x0001
@@ -108,8 +110,8 @@ int EmbedFile::GetFdForFileToc(const FileToc* toc) {
     SAPI_RAW_VLOG(3,
                   "Returning pre-existing embed file entry for '%s', fd: %d "
                   "(orig name: '%s')",
-                  toc->name, entry->second, entry->first->name);
-    return entry->second;
+                  toc->name, entry->second.get(), entry->first->name);
+    return entry->second.get();
   }
 
   int embed_fd = CreateFdForFileToc(toc);
@@ -121,7 +123,7 @@ int EmbedFile::GetFdForFileToc(const FileToc* toc) {
   SAPI_RAW_VLOG(1, "Created new embed file entry for '%s' with fd: %d",
                 toc->name, embed_fd);
 
-  file_tocs_[toc] = embed_fd;
+  file_tocs_[toc] = FDCloser(embed_fd);
   return embed_fd;
 }
 

sandboxed_api/embed_file.h

@@ -19,9 +19,12 @@
 #include "absl/base/thread_annotations.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/synchronization/mutex.h"
+#include "sandboxed_api/util/fileops.h"
 
 namespace sapi {
 
+class EmbedFileTestPeer;
+
 // The class provides primitives for converting FileToc structures into
 // executable files.
 class EmbedFile {
@@ -39,14 +42,15 @@ class EmbedFile {
   int GetDupFdForFileToc(const FileToc* toc);
 
  private:
+  friend class EmbedFileTestPeer;  // For testing.
   // Creates an executable file for a given FileToc, and return its
   // file-descriptors (-1 in case of errors).
   static int CreateFdForFileToc(const FileToc* toc);
 
   EmbedFile() = default;
 
   // List of File TOCs and corresponding file-descriptors.
-  absl::flat_hash_map<const FileToc*, int> file_tocs_
+  absl::flat_hash_map<const FileToc*, file_util::fileops::FDCloser> file_tocs_
       ABSL_GUARDED_BY(file_tocs_mutex_);
   absl::Mutex file_tocs_mutex_;
 };

sandboxed_api/embed_file_test.cc

@@ -0,0 +1,86 @@
+#include "sandboxed_api/embed_file.h"
+
+#include <memory>
+#include <string>
+
+#include "sandboxed_api/file_toc.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "absl/memory/memory.h"
+#include "absl/strings/string_view.h"
+
+namespace sapi {
+
+class EmbedFileTestPeer {
+ public:
+  static std::unique_ptr<EmbedFile> NewInstance() {
+    return absl::WrapUnique(new EmbedFile());
+  }
+};
+
+namespace {
+
+using ::testing::Eq;
+using ::testing::Ne;
+
+constexpr absl::string_view kRegularContents = "Hello world!";
+constexpr FileToc kRegularToc = {
+    .name = "regular",
+    .data = kRegularContents.data(),
+    .size = kRegularContents.size(),
+    .md5digest = {},  // MD5 is unused in SAPI implementation
+};
+
+constexpr FileToc kFaultyToc = {
+    .name = "regular",
+    .data = nullptr,
+    .size = 100,
+    .md5digest = {},  // MD5 is unused in SAPI implementation
+};
+
+TEST(EmbedFileTest, GetRegularFd) {
+  std::unique_ptr<EmbedFile> embed_file = EmbedFileTestPeer::NewInstance();
+  int fd = embed_file->GetFdForFileToc(&kRegularToc);
+  EXPECT_THAT(fd, Ne(-1));
+}
+
+TEST(EmbedFileTest, DuplicateGetFdIsSame) {
+  std::unique_ptr<EmbedFile> embed_file = EmbedFileTestPeer::NewInstance();
+  int fd = embed_file->GetFdForFileToc(&kRegularToc);
+  EXPECT_THAT(fd, Ne(-1));
+  int fd2 = embed_file->GetFdForFileToc(&kRegularToc);
+  EXPECT_THAT(fd, Eq(fd2));
+}
+
+TEST(EmbedFileTest, GetDupFdReturnsFreshFd) {
+  std::unique_ptr<EmbedFile> embed_file = EmbedFileTestPeer::NewInstance();
+  int fd = embed_file->GetFdForFileToc(&kRegularToc);
+  EXPECT_THAT(fd, Ne(-1));
+  int dup_fd = embed_file->GetDupFdForFileToc(&kRegularToc);
+  EXPECT_THAT(fd, Ne(dup_fd));
+  close(dup_fd);
+}
+
+TEST(EmbedFileTest, FaultyTocFails) {
+  std::unique_ptr<EmbedFile> embed_file = EmbedFileTestPeer::NewInstance();
+  int fd = embed_file->GetFdForFileToc(&kFaultyToc);
+  EXPECT_THAT(fd, Eq(-1));
+  int dup_fd = embed_file->GetDupFdForFileToc(&kFaultyToc);
+  EXPECT_THAT(dup_fd, Eq(-1));
+}
+
+TEST(EmbedFileTest, OverlongNameTocFails) {
+  std::string overlong_name(1000, 'a');
+  FileToc overlong_name_toc = {
+      .name = overlong_name.c_str(),
+      .data = kRegularContents.data(),
+      .size = kRegularContents.size(),
+      .md5digest = {},  // MD5 is unused in SAPI implementation
+  };
+  std::unique_ptr<EmbedFile> embed_file = EmbedFileTestPeer::NewInstance();
+  int fd = embed_file->GetFdForFileToc(&overlong_name_toc);
+  EXPECT_THAT(fd, Eq(-1));
+}
+
+}  // namespace
+}  // namespace sapi