Jelajahi Sumber

Keep a single content source in a Response

set_content() did not drop a content provider installed earlier, and
apply_ranges() then took the body's size as the Content-Length while the
provider was still the one written. The provider was asked for more bytes
than it had: a file under a mount point went out under the error status a
file request handler set to hide it, its mapping was read past the end,
and a user provider read past its own data.

Make every content setter clear the other sources so the last one wins,
let a body assigned directly after a provider or a file win as well, and
never read past the mapping in the file content provider.
yhirose 5 hari lalu
induk
melakukan
e3749669bc
2 mengubah file dengan 291 tambahan dan 32 penghapusan
  1. 61 32
      httplib.h
  2. 230 0
      test/test.cc

+ 61 - 32
httplib.h

@@ -8901,7 +8901,9 @@ inline void set_file_content_provider(Response &res,
   res.set_content_provider(
       m->size(), content_type,
       [m](size_t offset, size_t length, DataSink &sink) -> bool {
-        sink.write(m->data() + offset, length);
+        // Never read past the mapping.
+        if (offset >= m->size()) { return false; }
+        sink.write(m->data() + offset, (std::min)(length, m->size() - offset));
         return true;
       });
 
@@ -11829,6 +11831,43 @@ inline std::string MultipartFormDataWriter::finish() const {
 }
 
 // Response implementation
+namespace detail {
+
+// A response has one content source: `body`, a content provider, or a file.
+// Each content setter calls this so that the one called last wins.
+inline void clear_content_provider(Response &res) {
+  auto releaser = std::move(res.content_provider_resource_releaser_);
+  auto success = res.content_provider_success_;
+
+  res.content_length_ = 0;
+  res.content_provider_ = nullptr;
+  res.content_provider_resource_releaser_ = nullptr;
+  res.is_chunked_content_provider_ = false;
+  res.is_file_content_provider_ = false;
+  res.content_provider_success_ = false;
+  res.content_coding_ = EncodingType::None;
+  res.file_content_path_.clear();
+  res.file_content_content_type_.clear();
+
+  if (releaser) { releaser(success); }
+}
+
+inline void clear_content(Response &res) {
+  res.body.clear();
+  clear_content_provider(res);
+}
+
+// `body` is public, so it can be assigned directly after a provider or a file
+// was set. It came last, so it wins.
+inline void prefer_assigned_body(Response &res) {
+  if (!res.body.empty() &&
+      (res.content_provider_ || !res.file_content_path_.empty())) {
+    clear_content_provider(res);
+  }
+}
+
+} // namespace detail
+
 inline size_t Response::get_header_value_u64(const std::string &key, size_t def,
                                              size_t id) const {
   return detail::get_header_value_u64(headers, key, def, id);
@@ -11878,12 +11917,12 @@ inline void Response::set_redirect(const std::string &url, int stat) {
 
 inline void Response::set_content(const char *s, size_t n,
                                   const std::string &content_type) {
-  body.assign(s, n);
+  body.assign(s, n); // Before the releaser runs; it may own `s`.
+  detail::clear_content_provider(*this);
 
   auto rng = headers.equal_range("Content-Type");
   headers.erase(rng.first, rng.second);
   set_header("Content-Type", content_type);
-  content_coding_ = detail::EncodingType::None;
 }
 
 inline void Response::set_content(const std::string &s,
@@ -11894,57 +11933,52 @@ inline void Response::set_content(const std::string &s,
 inline void Response::set_content(std::string &&s,
                                   const std::string &content_type) {
   body = std::move(s);
+  detail::clear_content_provider(*this);
 
   auto rng = headers.equal_range("Content-Type");
   headers.erase(rng.first, rng.second);
   set_header("Content-Type", content_type);
-  content_coding_ = detail::EncodingType::None;
 }
 
 inline void Response::set_content_provider(
     size_t in_length, const std::string &content_type, ContentProvider provider,
     ContentProviderResourceReleaser resource_releaser) {
+  detail::clear_content(*this);
+
   set_header("Content-Type", content_type);
   content_length_ = in_length;
   if (in_length > 0) { content_provider_ = std::move(provider); }
   content_provider_resource_releaser_ = std::move(resource_releaser);
-  is_chunked_content_provider_ = false;
-  is_file_content_provider_ = false;
-  content_coding_ = detail::EncodingType::None;
 }
 
 inline void Response::set_content_provider(
     const std::string &content_type, ContentProviderWithoutLength provider,
     ContentProviderResourceReleaser resource_releaser) {
+  detail::clear_content(*this);
+
   set_header("Content-Type", content_type);
-  content_length_ = 0;
   content_provider_ = detail::ContentProviderAdapter(std::move(provider));
   content_provider_resource_releaser_ = std::move(resource_releaser);
-  is_chunked_content_provider_ = false;
-  is_file_content_provider_ = false;
-  content_coding_ = detail::EncodingType::None;
 }
 
 inline void Response::set_chunked_content_provider(
     const std::string &content_type, ContentProviderWithoutLength provider,
     ContentProviderResourceReleaser resource_releaser) {
-  set_header("Content-Type", content_type);
-  content_length_ = 0;
-  content_provider_ = detail::ContentProviderAdapter(std::move(provider));
-  content_provider_resource_releaser_ = std::move(resource_releaser);
+  set_content_provider(content_type, std::move(provider),
+                       std::move(resource_releaser));
   is_chunked_content_provider_ = true;
-  is_file_content_provider_ = false;
-  content_coding_ = detail::EncodingType::None;
 }
 
 inline void Response::set_file_content(const std::string &path,
                                        const std::string &content_type) {
+  detail::clear_content(*this);
+
   file_content_path_ = path;
   file_content_content_type_ = content_type;
 }
 
 inline void Response::set_file_content(const std::string &path) {
-  file_content_path_ = path;
+  set_file_content(path, std::string());
 }
 
 // Result implementation
@@ -13540,6 +13574,8 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
     need_apply_ranges = true;
   }
 
+  detail::prefer_assigned_body(res);
+
   std::string content_type;
   std::string boundary;
   if (need_apply_ranges) { apply_ranges(req, res, content_type, boundary); }
@@ -14247,9 +14283,7 @@ inline bool Server::routing(Request &req, Response &res, Stream &strm) {
           // Enforce the limit: override any status the handler may have set
           // and return false so the error path sends a plain 413 response.
           res.status = StatusCode::PayloadTooLarge_413;
-          res.body.clear();
-          res.content_length_ = 0;
-          res.content_provider_ = nullptr;
+          detail::clear_content(res);
           return false;
         }
         return true;
@@ -14372,12 +14406,9 @@ inline bool Server::apply_static_file_compression(const Request &req,
 
   res.body.swap(compressed);
 
-  // The provider was consumed in full, so a resource releaser registered with
-  // it should hear about a success when the response goes away.
+  // The provider was consumed in full, so its releaser hears about a success.
   res.content_provider_success_ = true;
-  res.content_provider_ = nullptr;
-  res.content_length_ = 0;
-  res.content_coding_ = detail::EncodingType::None;
+  detail::clear_content_provider(res);
 
   res.set_header("Content-Encoding", detail::encoding_name(type));
   res.set_header("Vary", "Accept-Encoding");
@@ -14865,15 +14896,15 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
                                       : StatusCode::PartialContent_206;
     }
 
+    // Before the Range checks below, so they see the body's own length.
+    detail::prefer_assigned_body(res);
+
     // Serve file content by using a content provider
     auto file_open_error = false;
     if (!res.file_content_path_.empty()) {
       const auto &path = res.file_content_path_;
       auto mm = std::make_shared<detail::mmap>(path.c_str());
       if (!mm->is_open()) {
-        res.body.clear();
-        res.content_length_ = 0;
-        res.content_provider_ = nullptr;
         res.status = StatusCode::NotFound_404;
         output_error_log(Error::OpenFile, &req);
         file_open_error = true;
@@ -14893,9 +14924,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
     if (file_open_error) {
       ret = write_response(strm, close_connection, req, res);
     } else if (detail::range_error(req, res)) {
-      res.body.clear();
-      res.content_length_ = 0;
-      res.content_provider_ = nullptr;
+      detail::clear_content(res);
       res.status = StatusCode::RangeNotSatisfiable_416;
       ret = write_response(strm, close_connection, req, res);
     } else {

+ 230 - 0
test/test.cc

@@ -12145,6 +12145,236 @@ TEST(ErrorHandlerWithContentProviderTest, ErrorHandler) {
   EXPECT_EQ("helloworld", res->body);
 }
 
+// A body set after a content provider must replace it, not be sent through it.
+TEST(ResponseContentSourceTest, FileRequestHandlerCanReplaceFile) {
+  Server svr;
+  svr.set_mount_point("/", "./www");
+  svr.set_file_request_handler([](const Request &req, Response &res) {
+    if (req.path == "/dir/test.html") {
+      res.status = StatusCode::Forbidden_403;
+      res.set_content(std::string(5000, 'D'), "text/plain");
+    } else if (req.path == "/file") {
+      res.status = StatusCode::Forbidden_403;
+      res.body = "Denied";
+    }
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto t = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+
+  {
+    auto res = cli.Get("/dir/test.html");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::Forbidden_403, res->status);
+    EXPECT_EQ(std::string(5000, 'D'), res->body);
+    EXPECT_EQ(1U, res->get_header_value_count("Content-Type"));
+  }
+
+  {
+    auto res = cli.Get("/file");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::Forbidden_403, res->status);
+    EXPECT_EQ("Denied", res->body);
+  }
+
+  {
+    auto res = cli.Get("/dir/index.html");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::OK_200, res->status);
+    EXPECT_EQ(0U, res->body.find("<html>"));
+  }
+}
+
+TEST(ResponseContentSourceTest, ErrorAndExceptionHandlersReplaceProvider) {
+  Server svr;
+
+  std::vector<bool> released;
+  std::mutex released_mutex;
+  auto releaser = [&](bool success) {
+    std::lock_guard<std::mutex> guard(released_mutex);
+    released.push_back(success);
+  };
+
+  auto provider = [](size_t offset, size_t length, DataSink &sink) {
+    return sink.write(&"hello"[offset], length);
+  };
+
+  svr.Get("/sized", [&](const Request &, Response &res) {
+    res.status = StatusCode::NotFound_404;
+    res.set_content_provider(5, "text/plain", provider, releaser);
+  });
+  svr.Get("/chunked", [&](const Request &, Response &res) {
+    res.status = StatusCode::ServiceUnavailable_503;
+    res.set_chunked_content_provider(
+        "text/plain",
+        [](size_t, DataSink &sink) {
+          sink.os << "chunk";
+          sink.done();
+          return true;
+        },
+        releaser);
+  });
+  svr.Get("/throw", [&](const Request &, Response &res) -> void {
+    res.set_content_provider(5, "text/plain", provider, releaser);
+    throw std::runtime_error("after the provider");
+  });
+
+  svr.set_error_handler([](const Request &req, Response &res) {
+    // Leave the exception handler's content to it.
+    if (req.path == "/throw") { return Server::HandlerResponse::Unhandled; }
+    res.set_content(std::string(1000, 'E'), "text/html");
+    return Server::HandlerResponse::Handled;
+  });
+  svr.set_exception_handler(
+      [](const Request &, Response &res, std::exception_ptr) {
+        res.status = StatusCode::InternalServerError_500;
+        res.set_content(std::string(1000, 'X'), "text/html");
+      });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto t = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+
+  {
+    auto res = cli.Get("/sized");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::NotFound_404, res->status);
+    EXPECT_EQ(std::string(1000, 'E'), res->body);
+    EXPECT_EQ("text/html", res->get_header_value("Content-Type"));
+  }
+
+  {
+    auto res = cli.Get("/chunked");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::ServiceUnavailable_503, res->status);
+    EXPECT_EQ(std::string(1000, 'E'), res->body);
+    EXPECT_FALSE(res->has_header("Transfer-Encoding"));
+  }
+
+  {
+    auto res = cli.Get("/throw");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::InternalServerError_500, res->status);
+    EXPECT_EQ(std::string(1000, 'X'), res->body);
+  }
+
+  std::lock_guard<std::mutex> guard(released_mutex);
+  EXPECT_EQ((std::vector<bool>{false, false, false}), released);
+}
+
+TEST(ResponseContentSourceTest, LastContentSetterWins) {
+  Server svr;
+
+  auto abc = [](size_t offset, size_t length, DataSink &sink) {
+    return sink.write(&"abc"[offset], length);
+  };
+
+  svr.Get("/file-then-body", [](const Request &, Response &res) {
+    res.set_file_content("./www/dir/test.html");
+    res.set_content("body", "text/plain");
+  });
+  svr.Get("/body-then-file", [](const Request &, Response &res) {
+    res.set_content(std::string(100, 'B'), "text/plain");
+    res.set_file_content("./www/dir/test.html");
+  });
+  svr.Get("/body-then-provider", [&](const Request &, Response &res) {
+    res.set_content(std::string(100, 'B'), "text/plain");
+    res.set_content_provider(3, "text/plain", abc);
+  });
+  svr.Get("/provider-then-body", [&](const Request &, Response &res) {
+    res.set_content_provider(3, "text/plain", abc);
+    res.set_content(std::string(100, 'B'), "text/plain");
+  });
+  svr.Get("/provider-then-direct-body", [&](const Request &, Response &res) {
+    res.set_content_provider(3, "text/plain", abc);
+    res.body = "direct";
+  });
+  svr.Get("/file-then-direct-body", [](const Request &, Response &res) {
+    res.set_file_content("./www/dir/test.html");
+    res.body = "direct";
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto t = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+
+  auto expect_body = [&](const char *path, const std::string &body) {
+    // Uncompressed, so Content-Length can be compared with the body.
+    auto res = cli.Get(path, Headers{{"Accept-Encoding", ""}});
+    ASSERT_TRUE(res) << path << " Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::OK_200, res->status) << path;
+    EXPECT_EQ(body, res->body) << path;
+    EXPECT_EQ(std::to_string(body.size()),
+              res->get_header_value("Content-Length"))
+        << path;
+  };
+
+  expect_body("/file-then-body", "body");
+  expect_body("/body-then-file", "test.html");
+  expect_body("/body-then-provider", "abc");
+  expect_body("/provider-then-body", std::string(100, 'B'));
+  expect_body("/provider-then-direct-body", "direct");
+  expect_body("/file-then-direct-body", "direct");
+
+  // Ranges are checked against the body, not the dropped provider.
+  {
+    auto res = cli.Get("/provider-then-direct-body",
+                       Headers{{make_range_header({{1, 2}})}});
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::PartialContent_206, res->status);
+    EXPECT_EQ("ir", res->body);
+  }
+  {
+    auto res = cli.Get("/file-then-direct-body",
+                       Headers{{make_range_header({{7, 8}})}});
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::RangeNotSatisfiable_416, res->status);
+  }
+}
+
+TEST(ResponseContentSourceTest, FileContentProviderStaysWithinFile) {
+  auto mm = std::make_shared<detail::mmap>("./www/dir/test.html");
+  ASSERT_TRUE(mm->is_open());
+
+  Response res;
+  detail::set_file_content_provider(res, mm, "text/html",
+                                    detail::EncodingType::None);
+
+  std::string out;
+  DataSink sink;
+  sink.write = [&](const char *d, size_t l) {
+    out.append(d, l);
+    return true;
+  };
+
+  EXPECT_TRUE(res.content_provider_(0, mm->size() + 4096, sink));
+  EXPECT_EQ("test.html", out);
+  EXPECT_FALSE(res.content_provider_(mm->size(), 1, sink));
+  EXPECT_EQ("test.html", out);
+}
+
 TEST(LongPollingTest, ClientCloseDetection) {
   Server svr;