Add flags when open file

include/envoy/filesystem/filesystem.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <bitset>
 #include <cstdint>
 #include <memory>
 #include <string>
@@ -13,20 +14,29 @@
 namespace Envoy {
 namespace Filesystem {
 
+using FlagSet = std::bitset<4>;
+
 /**
  * Abstraction for a basic file on disk.
  */
 class File {
 public:
   virtual ~File() = default;
 
+  enum Operation {
+    Read,
+    Write,
+    Create,
+    Append,
+  };
+
   /**
-   * Open the file with O_RDWR | O_APPEND | O_CREAT
+   * Open the file with Flag
    * The file will be closed when this object is destructed
    *
    * @return bool whether the open succeeded
    */
-  virtual Api::IoCallBoolResult open() PURE;
+  virtual Api::IoCallBoolResult open(FlagSet flags) PURE;
 
   /**
    * Write the buffer to the file. The file must be explicitly opened before writing.

source/common/access_log/access_log_manager_impl.cc

@@ -41,8 +41,16 @@ AccessLogFileImpl::AccessLogFileImpl(Filesystem::FilePtr&& file, Event::Dispatch
   open();
 }
 
+Filesystem::FlagSet AccessLogFileImpl::defaultFlags() {
+  static constexpr Filesystem::FlagSet default_flags{
+      1 << Filesystem::File::Operation::Read | 1 << Filesystem::File::Operation::Write |
+      1 << Filesystem::File::Operation::Create | 1 << Filesystem::File::Operation::Append};
+
+  return default_flags;
+}
+
 void AccessLogFileImpl::open() {
-  const Api::IoCallBoolResult result = file_->open();
+  const Api::IoCallBoolResult result = file_->open(defaultFlags());
   if (!result.rc_) {
     throw EnvoyException(
         fmt::format("unable to open file '{}': {}", file_->path(), result.err_->getErrorDetails()));

source/common/access_log/access_log_manager_impl.h

@@ -83,6 +83,9 @@ class AccessLogFileImpl : public AccessLogFile {
   void open();
   void createFlushStructures();
 
+  // return default flags set which used by open
+  static Filesystem::FlagSet defaultFlags();
+
   // Minimum size before the flush thread will be told to flush.
   static const uint64_t MIN_FLUSH_SIZE = 1024 * 64;
 

source/common/filesystem/file_shared_impl.cc

@@ -9,12 +9,12 @@ Api::IoError::IoErrorCode IoFileError::getErrorCode() const { return IoErrorCode
 
 std::string IoFileError::getErrorDetails() const { return ::strerror(errno_); }
 
-Api::IoCallBoolResult FileSharedImpl::open() {
+Api::IoCallBoolResult FileSharedImpl::open(FlagSet in) {
   if (isOpen()) {
     return resultSuccess<bool>(true);
   }
 
-  openFile();
+  openFile(in);
   return fd_ != -1 ? resultSuccess<bool>(true) : resultFailure<bool>(false, errno);
 }
 
@@ -36,4 +36,4 @@ bool FileSharedImpl::isOpen() const { return fd_ != -1; };
 std::string FileSharedImpl::path() const { return path_; };
 
 } // namespace Filesystem
-} // namespace Envoy
+} // namespace Envoy

source/common/filesystem/file_shared_impl.h

@@ -42,14 +42,14 @@ class FileSharedImpl : public File {
   ~FileSharedImpl() override = default;
 
   // Filesystem::File
-  Api::IoCallBoolResult open() override;
+  Api::IoCallBoolResult open(FlagSet flag) override;
   Api::IoCallSizeResult write(absl::string_view buffer) override;
   Api::IoCallBoolResult close() override;
   bool isOpen() const override;
   std::string path() const override;
 
 protected:
-  virtual void openFile() PURE;
+  virtual void openFile(FlagSet in) PURE;
   virtual ssize_t writeFile(absl::string_view buffer) PURE;
   virtual bool closeFile() PURE;
 

source/common/filesystem/posix/filesystem_impl.cc

@@ -28,17 +28,38 @@ FileImplPosix::~FileImplPosix() {
   }
 }
 
-void FileImplPosix::openFile() {
-  const int flags = O_RDWR | O_APPEND | O_CREAT;
-  const int mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
-
-  fd_ = ::open(path_.c_str(), flags, mode);
+void FileImplPosix::openFile(FlagSet in) {
+  const auto flags_and_mode = translateFlag(in);
+  fd_ = ::open(path_.c_str(), flags_and_mode.flags_, flags_and_mode.mode_);
 }
 
 ssize_t FileImplPosix::writeFile(absl::string_view buffer) {
   return ::write(fd_, buffer.data(), buffer.size());
 }
 
+FileImplPosix::FlagsAndMode FileImplPosix::translateFlag(FlagSet in) {
+  int out = 0;
+  mode_t mode = 0;
+  if (in.test(File::Operation::Create)) {
+    out |= O_CREAT;
+    mode |= S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+  }
+
+  if (in.test(File::Operation::Append)) {
+    out |= O_APPEND;
+  }
+
+  if (in.test(File::Operation::Read) && in.test(File::Operation::Write)) {
+    out |= O_RDWR;
+  } else if (in.test(File::Operation::Read)) {
+    out |= O_RDONLY;
+  } else if (in.test(File::Operation::Write)) {
+    out |= O_WRONLY;
+  }
+
+  return {out, mode};
+}
+
 bool FileImplPosix::closeFile() { return ::close(fd_) != -1; }
 
 FilePtr InstanceImplPosix::createFile(const std::string& path) {

source/common/filesystem/posix/filesystem_impl.h

@@ -16,8 +16,14 @@ class FileImplPosix : public FileSharedImpl {
   ~FileImplPosix() override;
 
 protected:
+  struct FlagsAndMode {
+    int flags_ = 0;
+    mode_t mode_ = 0;
+  };
+
   // Filesystem::FileSharedImpl
-  void openFile() override;
+  FlagsAndMode translateFlag(FlagSet in);
+  void openFile(FlagSet flags) override;
   ssize_t writeFile(absl::string_view buffer) override;
   bool closeFile() override;
 

source/common/filesystem/win32/filesystem_impl.cc

@@ -33,17 +33,38 @@ FileImplWin32::~FileImplWin32() {
   }
 }
 
-void FileImplWin32::openFile() {
-  const int flags = _O_RDWR | _O_APPEND | _O_CREAT;
-  const int mode = _S_IREAD | _S_IWRITE;
-
-  fd_ = ::_open(path_.c_str(), flags, mode);
+void FileImplWin32::openFile(FlagSet in) {
+  const auto flags_and_mode = translateFlag(in);
+  fd_ = ::open(path_.c_str(), flags_and_mode.flags_, flags_and_mode.pmode_);
 }
 
 ssize_t FileImplWin32::writeFile(absl::string_view buffer) {
   return ::_write(fd_, buffer.data(), buffer.size());
 }
 
+FileImplWin32::FlagsAndMode FileImplWin32::translateFlag(FlagSet in) {
+  int out = 0;
+  int pmode = 0;
+  if (in.test(File::Operation::Create)) {
+    out |= _O_CREAT;
+    pmode |= _S_IREAD | _S_IWRITE;
+  }
+
+  if (in.test(File::Operation::Append)) {
+    out |= _O_APPEND;
+  }
+
+  if (in.test(File::Operation::Read) && in.test(File::Operation::Write)) {
+    out |= _O_RDWR;
+  } else if (in.test(File::Operation::Read)) {
+    out |= _O_RDONLY;
+  } else if (in.test(File::Operation::Write)) {
+    out |= _O_WRONLY;
+  }
+
+  return {out, pmode};
+}
+
 bool FileImplWin32::closeFile() { return ::_close(fd_) != -1; }
 
 FilePtr InstanceImplWin32::createFile(const std::string& path) {

source/common/filesystem/win32/filesystem_impl.h

@@ -14,8 +14,14 @@ class FileImplWin32 : public FileSharedImpl {
   ~FileImplWin32();
 
 protected:
+  struct FlagsAndMode {
+    int flags_ = 0;
+    int pmode_ = 0;
+  };
+
   // Filesystem::FileSharedImpl
-  void openFile() override;
+  FlagsAndMode translateFlag(FlagSet in);
+  void openFile(FlagSet in) override;
   ssize_t writeFile(absl::string_view buffer) override;
   bool closeFile() override;
 

test/common/filesystem/filesystem_impl_test.cc

@@ -12,6 +12,10 @@
 namespace Envoy {
 namespace Filesystem {
 
+static constexpr FlagSet DefaultFlags{
+    1 << Filesystem::File::Operation::Read | 1 << Filesystem::File::Operation::Write |
+    1 << Filesystem::File::Operation::Create | 1 << Filesystem::File::Operation::Append};
+
 class FileSystemImplTest : public testing::Test {
 protected:
   int getFd(File* file) {
@@ -154,7 +158,7 @@ TEST_F(FileSystemImplTest, Open) {
   ::unlink(new_file_path.c_str());
 
   FilePtr file = file_system_.createFile(new_file_path);
-  const Api::IoCallBoolResult result = file->open();
+  const Api::IoCallBoolResult result = file->open(DefaultFlags);
   EXPECT_TRUE(result.rc_);
   EXPECT_TRUE(file->isOpen());
 }
@@ -166,21 +170,21 @@ TEST_F(FileSystemImplTest, OpenTwice) {
   FilePtr file = file_system_.createFile(new_file_path);
   EXPECT_EQ(getFd(file.get()), -1);
 
-  const Api::IoCallBoolResult result1 = file->open();
+  const Api::IoCallBoolResult result1 = file->open(DefaultFlags);
   const int initial_fd = getFd(file.get());
   EXPECT_TRUE(result1.rc_);
   EXPECT_TRUE(file->isOpen());
 
   // check that we don't leak a file descriptor
-  const Api::IoCallBoolResult result2 = file->open();
+  const Api::IoCallBoolResult result2 = file->open(DefaultFlags);
   EXPECT_EQ(initial_fd, getFd(file.get()));
   EXPECT_TRUE(result2.rc_);
   EXPECT_TRUE(file->isOpen());
 }
 
 TEST_F(FileSystemImplTest, OpenBadFilePath) {
   FilePtr file = file_system_.createFile("");
-  const Api::IoCallBoolResult result = file->open();
+  const Api::IoCallBoolResult result = file->open(DefaultFlags);
   EXPECT_FALSE(result.rc_);
 }
 
@@ -190,7 +194,7 @@ TEST_F(FileSystemImplTest, ExistingFile) {
 
   {
     FilePtr file = file_system_.createFile(file_path);
-    const Api::IoCallBoolResult open_result = file->open();
+    const Api::IoCallBoolResult open_result = file->open(DefaultFlags);
     EXPECT_TRUE(open_result.rc_);
     std::string data(" new data");
     const Api::IoCallSizeResult result = file->write(data);
@@ -207,7 +211,7 @@ TEST_F(FileSystemImplTest, NonExistingFile) {
 
   {
     FilePtr file = file_system_.createFile(new_file_path);
-    const Api::IoCallBoolResult open_result = file->open();
+    const Api::IoCallBoolResult open_result = file->open(DefaultFlags);
     EXPECT_TRUE(open_result.rc_);
     std::string data(" new data");
     const Api::IoCallSizeResult result = file->write(data);
@@ -223,7 +227,7 @@ TEST_F(FileSystemImplTest, Close) {
   ::unlink(new_file_path.c_str());
 
   FilePtr file = file_system_.createFile(new_file_path);
-  const Api::IoCallBoolResult result1 = file->open();
+  const Api::IoCallBoolResult result1 = file->open(DefaultFlags);
   EXPECT_TRUE(result1.rc_);
   EXPECT_TRUE(file->isOpen());
 
@@ -237,7 +241,7 @@ TEST_F(FileSystemImplTest, WriteAfterClose) {
   ::unlink(new_file_path.c_str());
 
   FilePtr file = file_system_.createFile(new_file_path);
-  const Api::IoCallBoolResult bool_result1 = file->open();
+  const Api::IoCallBoolResult bool_result1 = file->open(DefaultFlags);
   EXPECT_TRUE(bool_result1.rc_);
   const Api::IoCallBoolResult bool_result2 = file->close();
   EXPECT_TRUE(bool_result2.rc_);
@@ -247,5 +251,34 @@ TEST_F(FileSystemImplTest, WriteAfterClose) {
   EXPECT_EQ("Bad file descriptor", size_result.err_->getErrorDetails());
 }
 
+TEST_F(FileSystemImplTest, NonExistingFileAndReadOnly) {
+  const std::string new_file_path = TestEnvironment::temporaryPath("envoy_this_not_exist");
+  ::unlink(new_file_path.c_str());
+
+  static constexpr FlagSet flag(static_cast<size_t>(Filesystem::File::Operation::Read));
+  FilePtr file = file_system_.createFile(new_file_path);
+  const Api::IoCallBoolResult open_result = file->open(flag);
+  EXPECT_FALSE(open_result.rc_);
+}
+
+TEST_F(FileSystemImplTest, ExistingReadOnlyFileAndWrite) {
+  const std::string file_path =
+      TestEnvironment::writeStringToFileForTest("test_envoy", "existing file");
+
+  {
+    static constexpr FlagSet flag(static_cast<size_t>(Filesystem::File::Operation::Read));
+    FilePtr file = file_system_.createFile(file_path);
+    const Api::IoCallBoolResult open_result = file->open(flag);
+    EXPECT_TRUE(open_result.rc_);
+    std::string data(" new data");
+    const Api::IoCallSizeResult result = file->write(data);
+    EXPECT_TRUE(result.rc_ < 0);
+    EXPECT_EQ(result.err_->getErrorDetails(), "Bad file descriptor");
+  }
+
+  auto contents = TestEnvironment::readFileToStringForTest(file_path);
+  EXPECT_EQ("existing file", contents);
+}
+
 } // namespace Filesystem
 } // namespace Envoy

test/extensions/quic_listeners/quiche/platform/quic_test_output_impl.cc

@@ -42,9 +42,15 @@ void QuicRecordTestOutputToFile(const std::string& filename, QuicStringPiece dat
     return;
   }
 
+  static constexpr Envoy::Filesystem::FlagSet DefaultFlags{
+      1 << Envoy::Filesystem::File::Operation::Read |
+      1 << Envoy::Filesystem::File::Operation::Write |
+      1 << Envoy::Filesystem::File::Operation::Create |
+      1 << Envoy::Filesystem::File::Operation::Append};
+
   const std::string output_path = output_dir + filename;
   Envoy::Filesystem::FilePtr file = file_system.createFile(output_path);
-  if (!file->open().rc_) {
+  if (!file->open(DefaultFlags).rc_) {
     QUIC_LOG(ERROR) << "Failed to open test output file: " << output_path;
     return;
   }

test/mocks/filesystem/mocks.cc

@@ -9,7 +9,7 @@ namespace Filesystem {
 MockFile::MockFile() : num_opens_(0), num_writes_(0), is_open_(false) {}
 MockFile::~MockFile() = default;
 
-Api::IoCallBoolResult MockFile::open() {
+Api::IoCallBoolResult MockFile::open(FlagSet) {
   Thread::LockGuard lock(open_mutex_);
 
   Api::IoCallBoolResult result = open_();

test/mocks/filesystem/mocks.h

@@ -19,7 +19,7 @@ class MockFile : public File {
   ~MockFile() override;
 
   // Filesystem::File
-  Api::IoCallBoolResult open() override;
+  Api::IoCallBoolResult open(FlagSet flag) override;
   Api::IoCallSizeResult write(absl::string_view buffer) override;
   Api::IoCallBoolResult close() override;
   bool isOpen() const override { return is_open_; };