From 017797b7184fa6c3af5a7ddc9063879ce1a90d36 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Thu, 1 Dec 2022 17:56:36 +0000
Subject: [PATCH 01/15] Add fstat to OsSysCalls

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 envoy/api/os_sys_calls.h                     |  5 +++
 source/common/api/posix/os_sys_calls_impl.cc |  5 +++
 source/common/api/posix/os_sys_calls_impl.h  |  1 +
 source/common/api/win32/os_sys_calls_impl.cc |  5 +++
 source/common/api/win32/os_sys_calls_impl.h  |  1 +
 test/common/api/os_sys_calls_test.cc         | 35 ++++++++++++++++++++
 test/mocks/api/mocks.h                       |  1 +
 7 files changed, 53 insertions(+)

diff --git a/envoy/api/os_sys_calls.h b/envoy/api/os_sys_calls.h
index 28a591307443b..6a26a19edc70a 100644
--- a/envoy/api/os_sys_calls.h
+++ b/envoy/api/os_sys_calls.h
@@ -149,6 +149,11 @@ class OsSysCalls {
    */
   virtual SysCallIntResult stat(const char* pathname, struct stat* buf) PURE;
 
+  /**
+   * @see man 2 fstat
+   */
+  virtual SysCallIntResult fstat(os_fd_t fd, struct stat* buf) PURE;
+
   /**
    * @see man 2 setsockopt
    */
diff --git a/source/common/api/posix/os_sys_calls_impl.cc b/source/common/api/posix/os_sys_calls_impl.cc
index 3afac06a4c685..a142f949a61e7 100644
--- a/source/common/api/posix/os_sys_calls_impl.cc
+++ b/source/common/api/posix/os_sys_calls_impl.cc
@@ -194,6 +194,11 @@ SysCallIntResult OsSysCallsImpl::stat(const char* pathname, struct stat* buf) {
   return {rc, rc != -1 ? 0 : errno};
 }
 
+SysCallIntResult OsSysCallsImpl::fstat(os_fd_t fd, struct stat* buf) {
+  const int rc = ::fstat(fd, buf);
+  return {rc, rc != -1 ? 0 : errno};
+}
+
 SysCallIntResult OsSysCallsImpl::setsockopt(os_fd_t sockfd, int level, int optname,
                                             const void* optval, socklen_t optlen) {
   const int rc = ::setsockopt(sockfd, level, optname, optval, optlen);
diff --git a/source/common/api/posix/os_sys_calls_impl.h b/source/common/api/posix/os_sys_calls_impl.h
index 61a79c8aab063..0c4491eab7996 100644
--- a/source/common/api/posix/os_sys_calls_impl.h
+++ b/source/common/api/posix/os_sys_calls_impl.h
@@ -35,6 +35,7 @@ class OsSysCallsImpl : public OsSysCalls {
   SysCallPtrResult mmap(void* addr, size_t length, int prot, int flags, int fd,
                         off_t offset) override;
   SysCallIntResult stat(const char* pathname, struct stat* buf) override;
+  SysCallIntResult fstat(os_fd_t fd, struct stat* buf) override;
   SysCallIntResult setsockopt(os_fd_t sockfd, int level, int optname, const void* optval,
                               socklen_t optlen) override;
   SysCallIntResult getsockopt(os_fd_t sockfd, int level, int optname, void* optval,
diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index 1a630b5d9c6b6..27a61d65f3eb5 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -226,6 +226,11 @@ SysCallIntResult OsSysCallsImpl::stat(const char* pathname, struct stat* buf) {
   return {rc, rc != -1 ? 0 : errno};
 }
 
+SysCallIntResult OsSysCallsImpl::fstat(os_fd_t fd, struct stat* buf) {
+  const int rc = ::fstat(fd, buf);
+  return {rc, rc != -1 ? 0 : errno};
+}
+
 SysCallIntResult OsSysCallsImpl::setsockopt(os_fd_t sockfd, int level, int optname,
                                             const void* optval, socklen_t optlen) {
   const int rc = ::setsockopt(sockfd, level, optname, static_cast<const char*>(optval), optlen);
diff --git a/source/common/api/win32/os_sys_calls_impl.h b/source/common/api/win32/os_sys_calls_impl.h
index 44bc4993709f8..2afa41dd2ce3b 100644
--- a/source/common/api/win32/os_sys_calls_impl.h
+++ b/source/common/api/win32/os_sys_calls_impl.h
@@ -36,6 +36,7 @@ class OsSysCallsImpl : public OsSysCalls {
   SysCallPtrResult mmap(void* addr, size_t length, int prot, int flags, int fd,
                         off_t offset) override;
   SysCallIntResult stat(const char* pathname, struct stat* buf) override;
+  SysCallIntResult fstat(os_fd_t fd, struct stat* buf) override;
   SysCallIntResult setsockopt(os_fd_t sockfd, int level, int optname, const void* optval,
                               socklen_t optlen) override;
   SysCallIntResult getsockopt(os_fd_t sockfd, int level, int optname, void* optval,
diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 5748b257ac791..8a14079c07a8f 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -1,9 +1,44 @@
 #include "source/common/api/os_sys_calls_impl.h"
 
+#include "test/test_common/environment.h"
+
 #include "gtest/gtest.h"
 
 namespace Envoy {
 
+TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) {
+  auto& os_syscalls = Api::OsSysCallsSingleton::get();
+  std::string path{TestEnvironment::temporaryPath("envoy_test")};
+  TestEnvironment::createPath(path);
+  std::string file_path = path + "/file";
+  absl::string_view file_contents = "12345";
+  Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR);
+  EXPECT_NE(open_result.return_value_, -1);
+  EXPECT_EQ(open_result.errno_, 0);
+  os_fd_t fd = open_result.return_value_;
+  Api::SysCallSizeResult write_result =
+      os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
+  EXPECT_EQ(write_result.return_value_, file_contents.size());
+  EXPECT_EQ(write_result.errno_, 0);
+  struct stat fstat_value;
+  Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value);
+  EXPECT_EQ(fstat_result.return_value_, 0);
+  EXPECT_EQ(fstat_result.errno_, 0);
+  EXPECT_EQ(fstat_value.st_size, file_contents.size());
+  Api::SysCallIntResult close_result = os_syscalls.close(fd);
+  EXPECT_EQ(close_result.return_value_, 0);
+  EXPECT_EQ(close_result.errno_, 0);
+  struct stat stat_value;
+  Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value);
+  EXPECT_EQ(stat_result.return_value_, 0);
+  EXPECT_EQ(stat_result.errno_, 0);
+  EXPECT_EQ(stat_value.st_size, file_contents.size());
+  Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str());
+  EXPECT_EQ(unlink_result.return_value_, 0);
+  EXPECT_EQ(unlink_result.errno_, 0);
+  TestEnvironment::removePath(path);
+}
+
 TEST(OsSyscallsTest, SetAlternateGetifaddrs) {
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   const bool pre_alternate_support = os_syscalls.supportsGetifaddrs();
diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h
index 7d136a0784237..95f8bf8a8a179 100644
--- a/test/mocks/api/mocks.h
+++ b/test/mocks/api/mocks.h
@@ -94,6 +94,7 @@ class MockOsSysCalls : public OsSysCallsImpl {
   MOCK_METHOD(SysCallPtrResult, mmap,
               (void* addr, size_t length, int prot, int flags, int fd, off_t offset));
   MOCK_METHOD(SysCallIntResult, stat, (const char* name, struct stat* stat));
+  MOCK_METHOD(SysCallIntResult, fstat, (os_fd_t fd, struct stat* stat));
   MOCK_METHOD(SysCallIntResult, chmod, (const std::string& name, mode_t mode));
   MOCK_METHOD(int, setsockopt_,
               (os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen));

From 9e8d1b4228931515204b3ecc1bb6196220c14d34 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Thu, 1 Dec 2022 19:14:23 +0000
Subject: [PATCH 02/15] Add test comments for readability

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 8a14079c07a8f..5b375ddac4338 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -6,33 +6,40 @@
 
 namespace Envoy {
 
+// Test happy path for open, pwrite, fstat, close, stat and unlink.
 TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) {
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   std::string path{TestEnvironment::temporaryPath("envoy_test")};
   TestEnvironment::createPath(path);
   std::string file_path = path + "/file";
   absl::string_view file_contents = "12345";
+  // Test open
   Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR);
   EXPECT_NE(open_result.return_value_, -1);
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
+  // Test write
   Api::SysCallSizeResult write_result =
       os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
   EXPECT_EQ(write_result.return_value_, file_contents.size());
   EXPECT_EQ(write_result.errno_, 0);
+  // Test fstat
   struct stat fstat_value;
   Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value);
   EXPECT_EQ(fstat_result.return_value_, 0);
   EXPECT_EQ(fstat_result.errno_, 0);
   EXPECT_EQ(fstat_value.st_size, file_contents.size());
+  // Test close
   Api::SysCallIntResult close_result = os_syscalls.close(fd);
   EXPECT_EQ(close_result.return_value_, 0);
   EXPECT_EQ(close_result.errno_, 0);
+  // Test stat
   struct stat stat_value;
   Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value);
   EXPECT_EQ(stat_result.return_value_, 0);
   EXPECT_EQ(stat_result.errno_, 0);
   EXPECT_EQ(stat_value.st_size, file_contents.size());
+  // Test unlink
   Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str());
   EXPECT_EQ(unlink_result.return_value_, 0);
   EXPECT_EQ(unlink_result.errno_, 0);

From ec3358fceb6e79d80a3be5407e78b1f749c9a45a Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Thu, 1 Dec 2022 21:53:27 +0000
Subject: [PATCH 03/15] Satisfy the spelling demon

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 5b375ddac4338..c1725f03a1b24 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -6,40 +6,40 @@
 
 namespace Envoy {
 
-// Test happy path for open, pwrite, fstat, close, stat and unlink.
+// Test happy path for `open`, `pwrite`, `fstat`, `close`, `stat` and `unlink`.
 TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) {
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   std::string path{TestEnvironment::temporaryPath("envoy_test")};
   TestEnvironment::createPath(path);
   std::string file_path = path + "/file";
   absl::string_view file_contents = "12345";
-  // Test open
+  // Test `open`
   Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR);
   EXPECT_NE(open_result.return_value_, -1);
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
-  // Test write
+  // Test `write`
   Api::SysCallSizeResult write_result =
       os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
   EXPECT_EQ(write_result.return_value_, file_contents.size());
   EXPECT_EQ(write_result.errno_, 0);
-  // Test fstat
+  // Test `fstat`
   struct stat fstat_value;
   Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value);
   EXPECT_EQ(fstat_result.return_value_, 0);
   EXPECT_EQ(fstat_result.errno_, 0);
   EXPECT_EQ(fstat_value.st_size, file_contents.size());
-  // Test close
+  // Test `close`
   Api::SysCallIntResult close_result = os_syscalls.close(fd);
   EXPECT_EQ(close_result.return_value_, 0);
   EXPECT_EQ(close_result.errno_, 0);
-  // Test stat
+  // Test `stat`
   struct stat stat_value;
   Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value);
   EXPECT_EQ(stat_result.return_value_, 0);
   EXPECT_EQ(stat_result.errno_, 0);
   EXPECT_EQ(stat_value.st_size, file_contents.size());
-  // Test unlink
+  // Test `unlink`
   Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str());
   EXPECT_EQ(unlink_result.return_value_, 0);
   EXPECT_EQ(unlink_result.errno_, 0);

From 9bd5d0aa9d8ddaec5a3e37df6232b4e7e47e39a3 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Fri, 2 Dec 2022 16:12:20 +0000
Subject: [PATCH 04/15] Include fcntl.h for O_CREATE etc on windows

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index c1725f03a1b24..475cb134ca78e 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -1,3 +1,5 @@
+#include <fcntl.h>
+
 #include "source/common/api/os_sys_calls_impl.h"
 
 #include "test/test_common/environment.h"

From 13d21378f65fe11c4dcefc352be3669cf9e8e943 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Fri, 2 Dec 2022 21:02:19 +0000
Subject: [PATCH 05/15] Implement for windows, and test-cover pread and pwrite

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 source/common/api/win32/os_sys_calls_impl.cc | 22 ++++++++++++++++++--
 test/common/api/os_sys_calls_test.cc         | 13 +++++++++---
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index 27a61d65f3eb5..2eabc5114cc1b 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -144,12 +144,30 @@ SysCallSizeResult OsSysCallsImpl::readv(os_fd_t fd, const iovec* iov, int num_io
 
 SysCallSizeResult OsSysCallsImpl::pwrite(os_fd_t fd, const void* buffer, size_t length,
                                          off_t offset) const {
-  PANIC("not implemented");
+  OVERLAPPED overlapped;
+  overlapped.Offset = offset;
+  overlapped.OffsetHigh = offset & 0xffffffff00000000;
+  overlapped.hEvent = 0;
+  DWORD length_written;
+  bool success = ::WriteFile(fd, buffer, length, &length_written, overlapped);
+  if (!success) {
+    return {-1, ::GetLastError()};
+  }
+  return {length_written, 0};
 }
 
 SysCallSizeResult OsSysCallsImpl::pread(os_fd_t fd, void* buffer, size_t length,
                                         off_t offset) const {
-  PANIC("not implemented");
+  OVERLAPPED overlapped;
+  overlapped.Offset = offset;
+  overlapped.OffsetHigh = offset & 0xffffffff00000000;
+  overlapped.hEvent = 0;
+  DWORD length_read;
+  bool success = ::ReadFile(fd, buffer, length, &length_read, overlapped);
+  if (!success) {
+    return {-1, ::GetLastError()};
+  }
+  return {length_read, 0};
 }
 
 SysCallSizeResult OsSysCallsImpl::recv(os_fd_t socket, void* buffer, size_t length, int flags) {
diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 475cb134ca78e..61e4663d838f3 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -8,8 +8,8 @@
 
 namespace Envoy {
 
-// Test happy path for `open`, `pwrite`, `fstat`, `close`, `stat` and `unlink`.
-TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) {
+// Test happy path for `open`, `pwrite`, `pread`, `fstat`, `close`, `stat` and `unlink`.
+TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   auto& os_syscalls = Api::OsSysCallsSingleton::get();
   std::string path{TestEnvironment::temporaryPath("envoy_test")};
   TestEnvironment::createPath(path);
@@ -20,11 +20,18 @@ TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) {
   EXPECT_NE(open_result.return_value_, -1);
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
-  // Test `write`
+  // Test `pwrite`
   Api::SysCallSizeResult write_result =
       os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
   EXPECT_EQ(write_result.return_value_, file_contents.size());
   EXPECT_EQ(write_result.errno_, 0);
+  // Test `pread`
+  char read_buffer[5];
+  Api::SysCallSizeResult read_result = os_syscalls.pread(fd, read_buffer, sizeof(read_buffer), 0);
+  EXPECT_EQ(read_result.return_value_, sizeof(read_buffer));
+  EXPECT_EQ(read_result.errno_, 0);
+  absl::string_view read_buffer_view{read_buffer, sizeof(read_buffer)};
+  EXPECT_EQ(file_contents, read_buffer_view);
   // Test `fstat`
   struct stat fstat_value;
   Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value);

From 660c8db9d3dec8d1f1158ac446d09aaa6946d85e Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Fri, 2 Dec 2022 22:33:08 +0000
Subject: [PATCH 06/15] no pread and pwrite for Windows after all; make the
 test not use them

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 source/common/api/win32/os_sys_calls_impl.cc | 22 ++------------------
 test/common/api/os_sys_calls_test.cc         |  5 +++++
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index 2eabc5114cc1b..27a61d65f3eb5 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -144,30 +144,12 @@ SysCallSizeResult OsSysCallsImpl::readv(os_fd_t fd, const iovec* iov, int num_io
 
 SysCallSizeResult OsSysCallsImpl::pwrite(os_fd_t fd, const void* buffer, size_t length,
                                          off_t offset) const {
-  OVERLAPPED overlapped;
-  overlapped.Offset = offset;
-  overlapped.OffsetHigh = offset & 0xffffffff00000000;
-  overlapped.hEvent = 0;
-  DWORD length_written;
-  bool success = ::WriteFile(fd, buffer, length, &length_written, overlapped);
-  if (!success) {
-    return {-1, ::GetLastError()};
-  }
-  return {length_written, 0};
+  PANIC("not implemented");
 }
 
 SysCallSizeResult OsSysCallsImpl::pread(os_fd_t fd, void* buffer, size_t length,
                                         off_t offset) const {
-  OVERLAPPED overlapped;
-  overlapped.Offset = offset;
-  overlapped.OffsetHigh = offset & 0xffffffff00000000;
-  overlapped.hEvent = 0;
-  DWORD length_read;
-  bool success = ::ReadFile(fd, buffer, length, &length_read, overlapped);
-  if (!success) {
-    return {-1, ::GetLastError()};
-  }
-  return {length_read, 0};
+  PANIC("not implemented");
 }
 
 SysCallSizeResult OsSysCallsImpl::recv(os_fd_t socket, void* buffer, size_t length, int flags) {
diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 61e4663d838f3..1c004eba57312 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -20,6 +20,10 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_NE(open_result.return_value_, -1);
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
+#ifdef WIN32
+  // pwrite and pread are not supported.
+  ::_write(fd, file_contents.begin(), 5);
+#else
   // Test `pwrite`
   Api::SysCallSizeResult write_result =
       os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0);
@@ -32,6 +36,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_EQ(read_result.errno_, 0);
   absl::string_view read_buffer_view{read_buffer, sizeof(read_buffer)};
   EXPECT_EQ(file_contents, read_buffer_view);
+#endif
   // Test `fstat`
   struct stat fstat_value;
   Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value);

From 46afa094bb21727cd9ab62546684d96249770889 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Fri, 2 Dec 2022 22:38:28 +0000
Subject: [PATCH 07/15] Catch failure of write in the windows test path

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 1c004eba57312..6824255a3ba6a 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -21,8 +21,8 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
 #ifdef WIN32
-  // pwrite and pread are not supported.
-  ::_write(fd, file_contents.begin(), 5);
+  // pwrite and pread are not supported. Just write some bytes so we can still test stat.
+  EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.begin(), file_contents.size()));
 #else
   // Test `pwrite`
   Api::SysCallSizeResult write_result =

From 64a6ab84906f1a73e1b96605278cf7643d2303eb Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Mon, 5 Dec 2022 15:11:01 +0000
Subject: [PATCH 08/15] spell function names with backticks

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 6824255a3ba6a..152d92f00e5a0 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -21,7 +21,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;
 #ifdef WIN32
-  // pwrite and pread are not supported. Just write some bytes so we can still test stat.
+  // `pwrite` and `pread` are not supported. Just write some bytes so we can still test stat.
   EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.begin(), file_contents.size()));
 #else
   // Test `pwrite`

From 4ece43014888dc33a14e4438c97773e904fbb5a7 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Mon, 5 Dec 2022 16:31:33 +0000
Subject: [PATCH 09/15] Explain fcntl.h, and make it only included on Win32

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 152d92f00e5a0..94cb2e85e17e9 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -1,4 +1,6 @@
-#include <fcntl.h>
+#ifdef WIN32
+#include <fcntl.h> // Needed for `O_CREAT` and `O_RDWR` on Win32.
+#endif
 
 #include "source/common/api/os_sys_calls_impl.h"
 

From 545841522d1fedea4617cd284b27d48a7898aaf9 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Mon, 5 Dec 2022 16:34:35 +0000
Subject: [PATCH 10/15] check_format insists on adding this newline

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 94cb2e85e17e9..9d571421b9ae1 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -1,5 +1,6 @@
 #ifdef WIN32
 #include <fcntl.h> // Needed for `O_CREAT` and `O_RDWR` on Win32.
+
 #endif
 
 #include "source/common/api/os_sys_calls_impl.h"

From 6dfddbb6ac5524e3aa09fef6aa43d22b855cddc3 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Mon, 5 Dec 2022 18:22:24 +0000
Subject: [PATCH 11/15] Make Win32 OsSysCalls::close work on file descriptors,
 not just sockets

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 source/common/api/win32/os_sys_calls_impl.cc | 8 +++++++-
 test/common/api/os_sys_calls_test.cc         | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index 27a61d65f3eb5..6e44267106cf0 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -115,7 +115,13 @@ SysCallIntResult OsSysCallsImpl::ioctl(os_fd_t sockfd, unsigned long control_cod
 
 SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) {
   const int rc = ::closesocket(fd);
-  return {rc, rc != -1 ? 0 : ::WSAGetLastError()};
+  int socket_err = ::WSAGetLastError();
+  if (rc == -1) {
+    // If closesocket failed, maybe the descriptor was a file.
+    rc = ::close(fd);
+  }
+  // Assume the descriptor was a socket, and return the error from that one, if both failed.
+  return {rc, rc != -1 ? 0 : socket_err};
 }
 
 SysCallSizeResult OsSysCallsImpl::writev(os_fd_t fd, const iovec* iov, int num_iov) {
diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 9d571421b9ae1..d15c9ebcac51d 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -46,6 +46,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_EQ(fstat_result.return_value_, 0);
   EXPECT_EQ(fstat_result.errno_, 0);
   EXPECT_EQ(fstat_value.st_size, file_contents.size());
+#ifndef WIN32
   // Test `close`
   Api::SysCallIntResult close_result = os_syscalls.close(fd);
   EXPECT_EQ(close_result.return_value_, 0);

From 8a8b5c96c089ae6948fab122b483fb6c48714d85 Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Mon, 5 Dec 2022 19:59:52 +0000
Subject: [PATCH 12/15] Backtick the spelling demon again

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 source/common/api/win32/os_sys_calls_impl.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index 6e44267106cf0..df80b87d61fd6 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -117,7 +117,7 @@ SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) {
   const int rc = ::closesocket(fd);
   int socket_err = ::WSAGetLastError();
   if (rc == -1) {
-    // If closesocket failed, maybe the descriptor was a file.
+    // If `closesocket` failed, maybe the descriptor was a file.
     rc = ::close(fd);
   }
   // Assume the descriptor was a socket, and return the error from that one, if both failed.

From 37d6ca02fdfdea4dda6f031c6c9a67407ede585b Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Tue, 6 Dec 2022 15:15:53 +0000
Subject: [PATCH 13/15] Remove accidental ifndef

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index d15c9ebcac51d..9d571421b9ae1 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -46,7 +46,6 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   EXPECT_EQ(fstat_result.return_value_, 0);
   EXPECT_EQ(fstat_result.errno_, 0);
   EXPECT_EQ(fstat_value.st_size, file_contents.size());
-#ifndef WIN32
   // Test `close`
   Api::SysCallIntResult close_result = os_syscalls.close(fd);
   EXPECT_EQ(close_result.return_value_, 0);

From bdbf2ea2edceb65e0542094e7892c64e695e73cd Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Tue, 6 Dec 2022 21:18:14 +0000
Subject: [PATCH 14/15] Return code no longer const

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 source/common/api/win32/os_sys_calls_impl.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index df80b87d61fd6..69502f12cff28 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -114,8 +114,8 @@ SysCallIntResult OsSysCallsImpl::ioctl(os_fd_t sockfd, unsigned long control_cod
 }
 
 SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) {
-  const int rc = ::closesocket(fd);
-  int socket_err = ::WSAGetLastError();
+  int rc = ::closesocket(fd);
+  const int socket_err = ::WSAGetLastError();
   if (rc == -1) {
     // If `closesocket` failed, maybe the descriptor was a file.
     rc = ::close(fd);

From 7e9d5826b8728eff0aad356c3d185daceba41f5e Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Thu, 8 Dec 2022 20:38:31 +0000
Subject: [PATCH 15/15] Add permission mode to open in test for Windows

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
 test/common/api/os_sys_calls_test.cc | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc
index 9d571421b9ae1..432a43b737e02 100644
--- a/test/common/api/os_sys_calls_test.cc
+++ b/test/common/api/os_sys_calls_test.cc
@@ -18,8 +18,13 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) {
   TestEnvironment::createPath(path);
   std::string file_path = path + "/file";
   absl::string_view file_contents = "12345";
-  // Test `open`
-  Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR);
+// Test `open`
+#ifdef WIN32
+  int pmode = _S_IWRITE | _S_IREAD;
+#else
+  int pmode = S_IRUSR | S_IWUSR;
+#endif
+  Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR, pmode);
   EXPECT_NE(open_result.return_value_, -1);
   EXPECT_EQ(open_result.errno_, 0);
   os_fd_t fd = open_result.return_value_;