Browse Source

Serve files without mapping them on POSIX

Files have been read with pread() since 44658e6, so the mapping went
unused. Keep only the descriptor, open it close-on-exec and without
waiting on a FIFO, and accept only a regular file, so that a device is
no longer served as an empty body.
yhirose 1 day ago
parent
commit
35342285a0
2 changed files with 61 additions and 44 deletions
  1. 15 34
      httplib.h
  2. 46 10
      test/test.cc

+ 15 - 34
httplib.h

@@ -352,7 +352,6 @@ using socklen_t = int;
 #include <netinet/tcp.h>
 #include <poll.h>
 #include <pthread.h>
-#include <sys/mman.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <unistd.h>
@@ -3990,6 +3989,8 @@ struct ChunkedDecoder {
   bool parse_trailers_into(Headers &dest, const Headers &src_headers);
 };
 
+// A regular file opened to be served: mapped on Windows, read with pread()
+// elsewhere.
 class mmap {
 public:
   mmap(const char *path);
@@ -4000,22 +4001,20 @@ public:
 
   bool is_open() const;
   size_t size() const;
-  const char *data() const;
   // Copies up to len bytes from offset into buf and returns how many: fewer,
-  // down to 0, if the file has shrunk since it was opened, where reading
-  // data() would raise SIGBUS.
+  // down to 0, if the file has shrunk since it was opened.
   size_t read(size_t offset, char *buf, size_t len) const;
 
 private:
 #if defined(_WIN32)
   HANDLE hFile_ = NULL;
   HANDLE hMapping_ = NULL;
+  void *addr_ = nullptr;
+  bool is_open_empty_file = false;
 #else
   int fd_ = -1;
 #endif
   size_t size_ = 0;
-  void *addr_ = nullptr;
-  bool is_open_empty_file = false;
 };
 
 // NOTE: https://www.rfc-editor.org/rfc/rfc9110#section-5
@@ -6518,47 +6517,34 @@ inline bool mmap::open(const char *path) {
     return false;
   }
 #else
-  fd_ = ::open(path, O_RDONLY);
+  // O_NONBLOCK so that opening a FIFO does not wait for a writer.
+  fd_ = ::open(path, O_RDONLY | O_NONBLOCK | O_CLOEXEC);
   if (fd_ == -1) { return false; }
 
+  // Only a regular file has a size to serve, unlike a directory or a device.
   struct stat sb;
-  if (fstat(fd_, &sb) == -1) {
+  if (fstat(fd_, &sb) == -1 || !S_ISREG(sb.st_mode) ||
+      static_cast<uint64_t>(sb.st_size) >
+          (std::numeric_limits<size_t>::max)()) {
     close();
     return false;
   }
   size_ = static_cast<size_t>(sb.st_size);
-
-  addr_ = ::mmap(NULL, size_, PROT_READ, MAP_PRIVATE, fd_, 0);
-
-  // Special treatment for an empty file...
-  if (addr_ == MAP_FAILED && size_ == 0) {
-    close();
-    is_open_empty_file = true;
-    return false;
-  }
-
-  if (addr_ == MAP_FAILED) {
-    // Clear the sentinel before `close()`, since `is_open()` only checks
-    // `addr_` against nullptr and `munmap()` must not be called with it.
-    addr_ = nullptr;
-    close();
-    return false;
-  }
 #endif
 
   return true;
 }
 
 inline bool mmap::is_open() const {
+#if defined(_WIN32)
   return is_open_empty_file ? true : addr_ != nullptr;
+#else
+  return fd_ != -1;
+#endif
 }
 
 inline size_t mmap::size() const { return size_; }
 
-inline const char *mmap::data() const {
-  return is_open_empty_file ? "" : static_cast<const char *>(addr_);
-}
-
 inline void mmap::close() {
 #if defined(_WIN32)
   if (addr_) {
@@ -6578,11 +6564,6 @@ inline void mmap::close() {
 
   is_open_empty_file = false;
 #else
-  if (addr_ != nullptr) {
-    munmap(addr_, size_);
-    addr_ = nullptr;
-  }
-
   if (fd_ != -1) {
     ::close(fd_);
     fd_ = -1;

+ 46 - 10
test/test.cc

@@ -12195,24 +12195,60 @@ TEST(MmapTest, OpenWhileFileHeldForWriting) {
   detail::mmap m(path);
   ASSERT_TRUE(m.is_open());
   EXPECT_EQ(strlen(content), m.size());
-  EXPECT_EQ(0, std::memcmp(content, m.data(), strlen(content)));
+  char buf[8];
+  ASSERT_EQ(strlen(content), m.read(0, buf, sizeof(buf)));
+  EXPECT_EQ(0, std::memcmp(content, buf, strlen(content)));
 }
 #endif
 
 #ifndef _WIN32
-// A failed ::mmap() must not be reported as an open mapping, otherwise data()
-// hands the caller the MAP_FAILED sentinel. A directory is the easiest way to
-// reach it, since ::open() and fstat() succeed for one but ::mmap() doesn't.
-TEST(MmapTest, FailedMappingIsNotOpen) {
-  const char *path = "./mmap_failed_mapping_test_dir";
+// A directory or a device has no size to serve.
+TEST(MmapTest, OnlyRegularFilesAreOpened) {
+  const char *path = "./mmap_not_regular_test_dir";
   ASSERT_EQ(0, ::mkdir(path, 0755));
   auto dir_cleanup = detail::scope_exit([&] { ::rmdir(path); });
 
+  for (auto p : {path, "/dev/null"}) {
+    detail::mmap m(p);
+    EXPECT_FALSE(m.is_open()) << p;
+    EXPECT_EQ(0U, m.size()) << p;
+  }
+
+  Server svr;
+  svr.Get("/null", [](const Request & /*req*/, Response &res) {
+    res.set_file_content("/dev/null");
+  });
+  auto port = svr.bind_to_any_port(HOST);
+  thread t = thread([&] { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+  });
+  svr.wait_until_ready();
+
+  auto res = Client(HOST, port).Get("/null");
+  ASSERT_TRUE(res);
+  EXPECT_EQ(StatusCode::NotFound_404, res->status);
+}
+#endif
+
+#ifdef __linux__
+// Nothing of the file stays mapped, where a truncation would raise SIGBUS.
+TEST(MmapTest, FileIsNotMapped) {
+  const char *path = "mmap_not_mapped_test.txt";
+  {
+    std::ofstream f(path, std::ios::binary);
+    f << "hello";
+  }
+  auto file_cleanup = detail::scope_exit([&] { std::remove(path); });
+
   detail::mmap m(path);
-  EXPECT_FALSE(m.is_open());
-  EXPECT_EQ(0U, m.size());
-  EXPECT_NE(static_cast<const void *>(m.data()),
-            static_cast<const void *>(MAP_FAILED));
+  ASSERT_TRUE(m.is_open());
+
+  std::ifstream maps("/proc/self/maps");
+  std::string mappings((std::istreambuf_iterator<char>(maps)),
+                       std::istreambuf_iterator<char>());
+  EXPECT_EQ(std::string::npos, mappings.find(path));
 }
 #endif