[Windows] Adds a native windows filesystem implementation

include/envoy/common/platform.h

@@ -61,6 +61,7 @@ typedef ptrdiff_t ssize_t;
 typedef uint32_t mode_t;
 
 typedef SOCKET os_fd_t;
+using os_h_t = HANDLE;
 
 typedef unsigned int sa_family_t;
 
@@ -116,6 +117,7 @@ struct msghdr {
 #define IPV6_RECVPKTINFO IPV6_PKTINFO
 #endif
 
+#define INVALID_HANDLE INVALID_HANDLE_VALUE
 #define SOCKET_VALID(sock) ((sock) != INVALID_SOCKET)
 #define SOCKET_INVALID(sock) ((sock) == INVALID_SOCKET)
 #define SOCKET_FAILURE(rc) ((rc) == SOCKET_ERROR)
@@ -143,6 +145,9 @@ struct msghdr {
 #define SOCKET_ERROR_INVAL WSAEINVAL
 #define SOCKET_ERROR_ADDR_IN_USE WSAEADDRINUSE
 
+#define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED
+#define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE
+
 namespace Platform {
 constexpr absl::string_view null_device_path{"NUL"};
 }
@@ -206,7 +211,9 @@ constexpr absl::string_view null_device_path{"NUL"};
 #endif
 
 typedef int os_fd_t;
+using os_h_t = int;
 
+#define INVALID_HANDLE -1
 #define INVALID_SOCKET -1
 #define SOCKET_VALID(sock) ((sock) >= 0)
 #define SOCKET_INVALID(sock) ((sock) == -1)
@@ -231,6 +238,10 @@ typedef int os_fd_t;
 #define SOCKET_ERROR_INVAL EINVAL
 #define SOCKET_ERROR_ADDR_IN_USE EADDRINUSE
 
+// Mapping POSIX file errors to common error names
+#define HANDLE_ERROR_PERM EACCES
+#define HANDLE_ERROR_INVALID EBADF
+
 namespace Platform {
 constexpr absl::string_view null_device_path{"/dev/null"};
 }

source/common/filesystem/file_shared_impl.cc

@@ -1,43 +1,25 @@
 #include "common/filesystem/file_shared_impl.h"
 
-#include <cstring>
+#include "common/common/utility.h"
 
 namespace Envoy {
 namespace Filesystem {
 
-Api::IoError::IoErrorCode IoFileError::getErrorCode() const { return IoErrorCode::UnknownError; }
-
-std::string IoFileError::getErrorDetails() const {
-  // TODO(sunjayBhatia, wrowe): Disable clang-format until win32 implementation no longer uses POSIX
-  // subsystem, see https://github.com/envoyproxy/envoy/issues/11655
-  // clang-format off
-  return ::strerror(errno_);
-  // clang-format on
-}
-
-Api::IoCallBoolResult FileSharedImpl::open(FlagSet in) {
-  if (isOpen()) {
-    return resultSuccess<bool>(true);
+Api::IoError::IoErrorCode IoFileError::getErrorCode() const {
+  switch (errno_) {
+  case HANDLE_ERROR_PERM:
+    return IoErrorCode::Permission;
+  case HANDLE_ERROR_INVALID:
+    return IoErrorCode::BadFd;
+  default:
+    ENVOY_LOG_MISC(debug, "Unknown error code {} details {}", errno_, getErrorDetails());
+    return IoErrorCode::UnknownError;
   }
-
-  openFile(in);
-  return fd_ != -1 ? resultSuccess<bool>(true) : resultFailure<bool>(false, errno);
 }
 
-Api::IoCallSizeResult FileSharedImpl::write(absl::string_view buffer) {
-  const ssize_t rc = writeFile(buffer);
-  return rc != -1 ? resultSuccess<ssize_t>(rc) : resultFailure<ssize_t>(rc, errno);
-};
-
-Api::IoCallBoolResult FileSharedImpl::close() {
-  ASSERT(isOpen());
-
-  bool success = closeFile();
-  fd_ = -1;
-  return success ? resultSuccess<bool>(true) : resultFailure<bool>(false, errno);
-}
+std::string IoFileError::getErrorDetails() const { return errorDetails(errno_); }
 
-bool FileSharedImpl::isOpen() const { return fd_ != -1; };
+bool FileSharedImpl::isOpen() const { return fd_ != INVALID_HANDLE; };
 
 std::string FileSharedImpl::path() const { return path_; };
 

source/common/filesystem/file_shared_impl.h

@@ -37,23 +37,15 @@ template <typename T> Api::IoCallResult<T> resultSuccess(T result) {
 
 class FileSharedImpl : public File {
 public:
-  FileSharedImpl(std::string path) : fd_(-1), path_(std::move(path)) {}
+  FileSharedImpl(std::string path) : fd_(INVALID_HANDLE), path_(std::move(path)) {}
 
   ~FileSharedImpl() override = default;
 
-  // Filesystem::File
-  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(FlagSet in) PURE;
-  virtual ssize_t writeFile(absl::string_view buffer) PURE;
-  virtual bool closeFile() PURE;
-
-  int fd_;
+  os_h_t fd_;
   const std::string path_;
 };
 

source/common/filesystem/posix/filesystem_impl.cc

@@ -30,13 +30,26 @@ FileImplPosix::~FileImplPosix() {
   }
 }
 
-void FileImplPosix::openFile(FlagSet in) {
+Api::IoCallBoolResult FileImplPosix::open(FlagSet in) {
+  if (isOpen()) {
+    return resultSuccess(true);
+  }
+
   const auto flags_and_mode = translateFlag(in);
   fd_ = ::open(path_.c_str(), flags_and_mode.flags_, flags_and_mode.mode_);
+  return fd_ != -1 ? resultSuccess(true) : resultFailure(false, errno);
 }
 
-ssize_t FileImplPosix::writeFile(absl::string_view buffer) {
-  return ::write(fd_, buffer.data(), buffer.size());
+Api::IoCallSizeResult FileImplPosix::write(absl::string_view buffer) {
+  const ssize_t rc = ::write(fd_, buffer.data(), buffer.size());
+  return rc != -1 ? resultSuccess(rc) : resultFailure(rc, errno);
+};
+
+Api::IoCallBoolResult FileImplPosix::close() {
+  ASSERT(isOpen());
+  int rc = ::close(fd_);
+  fd_ = -1;
+  return (rc != -1) ? resultSuccess(true) : resultFailure(false, errno);
 }
 
 FileImplPosix::FlagsAndMode FileImplPosix::translateFlag(FlagSet in) {
@@ -62,8 +75,6 @@ FileImplPosix::FlagsAndMode FileImplPosix::translateFlag(FlagSet in) {
   return {out, mode};
 }
 
-bool FileImplPosix::closeFile() { return ::close(fd_) != -1; }
-
 FilePtr InstanceImplPosix::createFile(const std::string& path) {
   return std::make_unique<FileImplPosix>(path);
 }

source/common/filesystem/posix/filesystem_impl.h

@@ -21,11 +21,11 @@ class FileImplPosix : public FileSharedImpl {
     mode_t mode_ = 0;
   };
 
-  // Filesystem::FileSharedImpl
+  Api::IoCallBoolResult open(FlagSet flag) override;
+  Api::IoCallSizeResult write(absl::string_view buffer) override;
+  Api::IoCallBoolResult close() override;
+
   FlagsAndMode translateFlag(FlagSet in);
-  void openFile(FlagSet flags) override;
-  ssize_t writeFile(absl::string_view buffer) override;
-  bool closeFile() override;
 
 private:
   friend class FileSystemImplTest;

source/common/filesystem/win32/filesystem_impl.cc

@@ -26,40 +26,67 @@ FileImplWin32::~FileImplWin32() {
   }
 }
 
-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_);
+Api::IoCallBoolResult FileImplWin32::open(FlagSet in) {
+  if (isOpen()) {
+    return resultSuccess(true);
+  }
+
+  auto flags = translateFlag(in);
+  fd_ = CreateFileA(path_.c_str(), flags.access_, FILE_SHARE_READ | FILE_SHARE_WRITE, 0,
+                    flags.creation_, 0, NULL);
+  if (fd_ == INVALID_HANDLE) {
+    return resultFailure(false, ::GetLastError());
+  }
+  return resultSuccess(true);
 }
 
-ssize_t FileImplWin32::writeFile(absl::string_view buffer) {
-  return ::_write(fd_, buffer.data(), buffer.size());
+Api::IoCallSizeResult FileImplWin32::write(absl::string_view buffer) {
+  DWORD bytes_written;
+  BOOL result = WriteFile(fd_, buffer.data(), buffer.length(), &bytes_written, NULL);
+  if (result == 0) {
+    return resultFailure<ssize_t>(-1, ::GetLastError());
+  }
+  return resultSuccess<ssize_t>(bytes_written);
+};
+
+Api::IoCallBoolResult FileImplWin32::close() {
+  ASSERT(isOpen());
+
+  BOOL result = CloseHandle(fd_);
+  fd_ = INVALID_HANDLE;
+  if (result == 0) {
+    return resultFailure(false, ::GetLastError());
+  }
+  return resultSuccess(true);
 }
 
 FileImplWin32::FlagsAndMode FileImplWin32::translateFlag(FlagSet in) {
-  int out = 0;
-  int pmode = 0;
+  DWORD access = 0;
+  DWORD creation = OPEN_EXISTING;
+
   if (in.test(File::Operation::Create)) {
-    out |= _O_CREAT;
-    pmode |= _S_IREAD | _S_IWRITE;
+    creation = OPEN_ALWAYS;
+  }
+
+  if (in.test(File::Operation::Write)) {
+    access = GENERIC_WRITE;
   }
 
+  // Order of tests matter here. There reason for that
+  // is that `FILE_APPEND_DATA` should not be used together
+  // with `GENERIC_WRITE`. If both of them are used the file
+  // is not opened in append mode.
   if (in.test(File::Operation::Append)) {
-    out |= _O_APPEND;
+    access = FILE_APPEND_DATA;
   }
 
-  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;
+  if (in.test(File::Operation::Read)) {
+    access |= GENERIC_READ;
   }
 
-  return {out, pmode};
+  return {access, creation};
 }
 
-bool FileImplWin32::closeFile() { return ::_close(fd_) != -1; }
-
 FilePtr InstanceImplWin32::createFile(const std::string& path) {
   return std::make_unique<FileImplWin32>(path);
 }
@@ -78,11 +105,19 @@ bool InstanceImplWin32::directoryExists(const std::string& path) {
 }
 
 ssize_t InstanceImplWin32::fileSize(const std::string& path) {
-  struct _stat info;
-  if (::_stat(path.c_str(), &info) != 0) {
+  auto fd = CreateFileA(path.c_str(), GENERIC_READ, FILE_SHARE_READ, 0, OPEN_EXISTING, 0, NULL);
+  if (fd == INVALID_HANDLE) {
+    return -1;
+  }
+  ssize_t result = 0;
+  LARGE_INTEGER lFileSize;
+  BOOL bGetSize = GetFileSizeEx(fd, &lFileSize);
+  CloseHandle(fd);
+  if (!bGetSize) {
     return -1;
   }
-  return info.st_size;
+  result += lFileSize.QuadPart;
+  return result;
 }
 
 std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {

source/common/filesystem/win32/filesystem_impl.h

@@ -15,15 +15,15 @@ class FileImplWin32 : public FileSharedImpl {
 
 protected:
   struct FlagsAndMode {
-    int flags_ = 0;
-    int pmode_ = 0;
+    DWORD access_ = 0;
+    DWORD creation_ = 0;
   };
 
-  // Filesystem::FileSharedImpl
+  Api::IoCallBoolResult open(FlagSet flag) override;
+  Api::IoCallSizeResult write(absl::string_view buffer) override;
+  Api::IoCallBoolResult close() override;
+
   FlagsAndMode translateFlag(FlagSet in);
-  void openFile(FlagSet in) override;
-  ssize_t writeFile(absl::string_view buffer) override;
-  bool closeFile() override;
 
 private:
   friend class FileSystemImplTest;

test/common/filesystem/filesystem_impl_test.cc

@@ -19,7 +19,7 @@ static constexpr FlagSet DefaultFlags{
 
 class FileSystemImplTest : public testing::Test {
 protected:
-  int getFd(File* file) {
+  os_h_t getFd(File* file) {
 #ifdef WIN32
     auto file_impl = dynamic_cast<FileImplWin32*>(file);
 #else
@@ -240,10 +240,10 @@ TEST_F(FileSystemImplTest, OpenTwice) {
   ::unlink(new_file_path.c_str());
 
   FilePtr file = file_system_.createFile(new_file_path);
-  EXPECT_EQ(getFd(file.get()), -1);
+  EXPECT_EQ(getFd(file.get()), INVALID_HANDLE);
 
   const Api::IoCallBoolResult result1 = file->open(DefaultFlags);
-  const int initial_fd = getFd(file.get());
+  const os_h_t initial_fd = getFd(file.get());
   EXPECT_TRUE(result1.rc_);
   EXPECT_TRUE(file->isOpen());
 
@@ -319,8 +319,7 @@ TEST_F(FileSystemImplTest, WriteAfterClose) {
   EXPECT_TRUE(bool_result2.rc_);
   const Api::IoCallSizeResult size_result = file->write(" new data");
   EXPECT_EQ(-1, size_result.rc_);
-  EXPECT_EQ(IoFileError::IoErrorCode::UnknownError, size_result.err_->getErrorCode());
-  EXPECT_EQ("Bad file descriptor", size_result.err_->getErrorDetails());
+  EXPECT_EQ(IoFileError::IoErrorCode::BadFd, size_result.err_->getErrorCode());
 }
 
 TEST_F(FileSystemImplTest, NonExistingFileAndReadOnly) {
@@ -345,7 +344,11 @@ TEST_F(FileSystemImplTest, ExistingReadOnlyFileAndWrite) {
     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");
+#ifdef WIN32
+    EXPECT_EQ(IoFileError::IoErrorCode::Permission, result.err_->getErrorCode());
+#else
+    EXPECT_EQ(IoFileError::IoErrorCode::BadFd, result.err_->getErrorCode());
+#endif
   }
 
   auto contents = TestEnvironment::readFileToStringForTest(file_path);