Serve only canonical paths from mount points

A mount point left ".", ".." and empty path segments for the file
system to resolve, while the pre-routing handler, where the README has
applications guard static files, saw the path as it was sent. A file
could then be reached under a spelling that a check on req.path did not
match. Serve a path from a mount point only when it has none of those
segments other than a trailing '/', so that the file served is the one
req.path names; any other path now gets 404. The README also notes the
other names a file system can give a file.

README.md

@@ -406,6 +406,8 @@ ret = svr.remove_mount_point("/");
 ret = svr.remove_mount_point("/public");
 ```
 
+A mount point serves a path only when it has no `.`, `..` or empty segment other than a trailing `/`, so a check on `req.path` in `set_pre_routing_handler()` sees the path of the file that is served. A path ending in `/`, and the mount point itself such as `/public`, serve the `index.html` in that directory. The file system can still give a file other names: another case on a case-insensitive file system, such as the default ones on macOS and Windows, and on Windows a name with a trailing `.` or space, or an 8.3 short name such as `PRIVAT~1`. Compare paths without regard to case on such a file system, and on Windows keep files that need a guard out of mounted directories.
+
 ```cpp
 // User defined file extension and MIME type mappings
 svr.set_file_extension_and_mimetype_mapping("cc", "text/x-c");

httplib.h

@@ -5955,45 +5955,25 @@ inline bool is_valid_close_payload(const std::string &payload) {
 
 namespace detail {
 
+// A path a mount point serves: absolute, with no ".", ".." or empty segment
+// except a trailing '/', so the file served is the one that req.path names to
+// the handlers that ran before it.
 inline bool is_valid_path(const std::string &path) {
-  size_t level = 0;
-  size_t i = 0;
+  if (path.empty() || path[0] != '/') { return false; }
 
-  // Skip slash
-  while (i < path.size() && path[i] == '/') {
-    i++;
-  }
-
-  while (i < path.size()) {
-    // Read component
-    auto beg = i;
-    while (i < path.size() && path[i] != '/') {
-      if (path[i] == '\0') {
-        return false;
-      } else if (path[i] == '\\') {
-        return false;
-      }
-      i++;
+  auto beg = size_t{1};
+  for (auto i = beg; i <= path.size(); i++) {
+    if (i < path.size() && path[i] != '/') {
+      if (path[i] == '\0' || path[i] == '\\') { return false; }
+      continue;
     }
-
     auto len = i - beg;
-    assert(len > 0);
-
-    if (!path.compare(beg, len, ".")) {
-      ;
-    } else if (!path.compare(beg, len, "..")) {
-      if (level == 0) { return false; }
-      level--;
-    } else {
-      level++;
-    }
-
-    // Skip slash
-    while (i < path.size() && path[i] == '/') {
-      i++;
+    if ((len == 0 && i < path.size()) || (len == 1 && path[beg] == '.') ||
+        (len == 2 && path[beg] == '.' && path[beg + 1] == '.')) {
+      return false;
     }
+    beg = i + 1;
   }
-
   return true;
 }
 
@@ -14330,6 +14310,8 @@ inline bool Server::read_content_core(
 }
 
 inline bool Server::handle_file_request(Request &req, Response &res) {
+  if (!detail::is_valid_path(req.path)) { return false; }
+
   for (const auto &entry : base_dirs_) {
     // Prefix match, on a path segment boundary. A mount point of "/mount"
     // covers "/mount" and "/mount/...", but must not swallow "/mountdir/...".
@@ -14340,85 +14322,80 @@ inline bool Server::handle_file_request(Request &req, Response &res) {
          req.path.size() == entry.mount_point.size() ||
          req.path[entry.mount_point.size()] == '/')) {
       std::string sub_path = "/" + req.path.substr(entry.mount_point.size());
-      if (detail::is_valid_path(sub_path)) {
-        auto path = entry.base_dir + sub_path;
-        if (path.back() == '/') { path += "index.html"; }
+      auto path = entry.base_dir + sub_path;
+      if (path.back() == '/') { path += "index.html"; }
 
-        // Defense-in-depth: is_valid_path blocks ".." traversal in the URL,
-        // but symlinks/junctions can still escape the base directory. A path
-        // that cannot be resolved, which includes one that does not exist, is
-        // not served from here.
-        std::string resolved_path;
-        if (!detail::canonicalize_path(path.c_str(), resolved_path)) {
-          continue;
-        }
-        if (!detail::is_path_within_base(resolved_path,
-                                         entry.resolved_base_dir)) {
-          res.status = StatusCode::Forbidden_403;
-          return true;
+      // Defense-in-depth: is_valid_path blocks ".." traversal in the URL,
+      // but symlinks/junctions can still escape the base directory. A path
+      // that cannot be resolved, which includes one that does not exist, is
+      // not served from here.
+      std::string resolved_path;
+      if (!detail::canonicalize_path(path.c_str(), resolved_path)) { continue; }
+      if (!detail::is_path_within_base(resolved_path,
+                                       entry.resolved_base_dir)) {
+        res.status = StatusCode::Forbidden_403;
+        return true;
+      }
+
+      detail::FileStat stat(path);
+
+      if (stat.is_dir()) {
+        // Relative, so "//dir" cannot name another host; "./" keeps a ':'
+        // in the name from reading as a scheme.
+        auto name = req.path.substr(req.path.rfind('/') + 1);
+        res.set_redirect("./" + encode_path_component(name) + "/",
+                         StatusCode::MovedPermanently_301);
+        return true;
+      }
+
+      if (stat.is_file()) {
+        for (const auto &kv : entry.headers) {
+          res.set_header(kv.first, kv.second);
         }
 
-        detail::FileStat stat(path);
-
-        if (stat.is_dir()) {
-          // Relative, so "//dir" cannot name another host; "./" keeps a ':'
-          // in the name from reading as a scheme.
-          auto name = req.path.substr(req.path.rfind('/') + 1);
-          res.set_redirect("./" + encode_path_component(name) + "/",
-                           StatusCode::MovedPermanently_301);
-          return true;
+        auto content_type = detail::find_content_type(
+            path, file_extension_and_mimetype_map_, default_file_mimetype_);
+        auto encoding = detail::EncodingType::None;
+        if (static_file_compression_) {
+          encoding = static_file_encoding(req, res, content_type, stat.size());
         }
 
-        if (stat.is_file()) {
-          for (const auto &kv : entry.headers) {
-            res.set_header(kv.first, kv.second);
-          }
+        // The ETag names the representation actually sent, so a client that
+        // cached the compressed form revalidates against the compressed ETag
+        // and still gets a 304, while one that took identity keeps the plain
+        // ETag.
+        auto etag = detail::compute_etag(
+            stat, encoding == detail::EncodingType::None
+                      ? std::string()
+                      : std::string("-") + detail::encoding_name(encoding));
+        if (!etag.empty()) { res.set_header("ETag", etag); }
 
-          auto content_type = detail::find_content_type(
-              path, file_extension_and_mimetype_map_, default_file_mimetype_);
-          auto encoding = detail::EncodingType::None;
-          if (static_file_compression_) {
-            encoding =
-                static_file_encoding(req, res, content_type, stat.size());
-          }
+        auto mtime = stat.mtime();
 
-          // The ETag names the representation actually sent, so a client that
-          // cached the compressed form revalidates against the compressed ETag
-          // and still gets a 304, while one that took identity keeps the plain
-          // ETag.
-          auto etag = detail::compute_etag(
-              stat, encoding == detail::EncodingType::None
-                        ? std::string()
-                        : std::string("-") + detail::encoding_name(encoding));
-          if (!etag.empty()) { res.set_header("ETag", etag); }
+        auto last_modified = detail::file_mtime_to_http_date(mtime);
+        if (!last_modified.empty()) {
+          res.set_header("Last-Modified", last_modified);
+        }
 
-          auto mtime = stat.mtime();
+        if (check_if_not_modified(req, res, etag, mtime)) { return true; }
 
-          auto last_modified = detail::file_mtime_to_http_date(mtime);
-          if (!last_modified.empty()) {
-            res.set_header("Last-Modified", last_modified);
-          }
+        check_if_range(req, etag, mtime);
 
-          if (check_if_not_modified(req, res, etag, mtime)) { return true; }
-
-          check_if_range(req, etag, mtime);
-
-          auto mm = std::make_shared<detail::mmap>(path.c_str());
-          if (!mm->is_open()) {
-            output_error_log(Error::OpenFile, &req);
-            return false;
-          }
-
-          detail::set_file_content_provider(res, mm, content_type, encoding);
-
-          if (req.method != "HEAD" && file_request_handler_) {
-            file_request_handler_(req, res);
-          }
-
-          return true;
-        } else {
+        auto mm = std::make_shared<detail::mmap>(path.c_str());
+        if (!mm->is_open()) {
           output_error_log(Error::OpenFile, &req);
+          return false;
         }
+
+        detail::set_file_content_provider(res, mm, content_type, encoding);
+
+        if (req.method != "HEAD" && file_request_handler_) {
+          file_request_handler_(req, res);
+        }
+
+        return true;
+      } else {
+        output_error_log(Error::OpenFile, &req);
       }
     }
   }

test/test.cc

@@ -7417,9 +7417,7 @@ TEST_F(ServerTest, GetMethodDirTest) {
 TEST_F(ServerTest, GetMethodDirTestWithDoubleDots) {
   auto res = cli_.Get("/dir/../dir/test.html");
   ASSERT_TRUE(res) << "Error: " << to_string(res.error());
-  EXPECT_EQ(StatusCode::OK_200, res->status);
-  EXPECT_EQ("text/html", res->get_header_value("Content-Type"));
-  EXPECT_EQ("test.html", res->body);
+  EXPECT_EQ(StatusCode::NotFound_404, res->status);
 }
 
 TEST_F(ServerTest, GetMethodInvalidPath) {
@@ -7451,9 +7449,7 @@ TEST_F(ServerTest, GetMethodDirMountTest) {
 TEST_F(ServerTest, GetMethodDirMountTestWithDoubleDots) {
   auto res = cli_.Get("/mount/dir/../dir/test.html");
   ASSERT_TRUE(res) << "Error: " << to_string(res.error());
-  EXPECT_EQ(StatusCode::OK_200, res->status);
-  EXPECT_EQ("text/html", res->get_header_value("Content-Type"));
-  EXPECT_EQ("test.html", res->body);
+  EXPECT_EQ(StatusCode::NotFound_404, res->status);
 }
 
 TEST_F(ServerTest, GetMethodInvalidMountPath) {
@@ -7496,8 +7492,8 @@ TEST_F(ServerTest, GetMethodDirMountWithoutTrailingSlash) {
 TEST_F(ServerTest, GetMethodDirWithLeadingDoubleSlash) {
   auto res = cli_.Get("//dir");
   ASSERT_TRUE(res) << "Error: " << to_string(res.error());
-  EXPECT_EQ(StatusCode::MovedPermanently_301, res->status);
-  EXPECT_EQ("./dir/", res->get_header_value("Location"));
+  EXPECT_EQ(StatusCode::NotFound_404, res->status);
+  EXPECT_FALSE(res->has_header("Location"));
 }
 
 TEST_F(ServerTest, PostMethod303) {
@@ -12486,6 +12482,40 @@ TEST(MountTest, MultibytesPathName) {
   EXPECT_EQ(U8("日本語コンテンツ"), res->body);
 }
 
+// A mount point serves no path with an empty, "." or ".." segment, so a guard
+// in the pre-routing handler sees the path of every file that is served
+TEST(MountTest, AliasedPathDoesNotPassPreRoutingGuard) {
+  Server svr;
+  ASSERT_TRUE(svr.set_mount_point("/", "./www"));
+  svr.set_pre_routing_handler([](const Request &req, Response &res) {
+    if (req.path == "/dir/test.html") {
+      res.status = StatusCode::Unauthorized_401;
+      return Server::HandlerResponse::Handled;
+    }
+    return Server::HandlerResponse::Unhandled;
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  ASSERT_LT(0, port);
+  thread t = 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);
+  for (auto path :
+       {"/./dir/test.html", "/dir/./test.html", "//dir/test.html",
+        "/dir//test.html", "/dir/../dir/test.html", "/%2e/dir/test.html"}) {
+    SCOPED_TRACE(path);
+    auto res = cli.Get(path);
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ(StatusCode::NotFound_404, res->status);
+  }
+}
+
 #ifdef _WIN32
 // Issue #2435: mmap::open() must succeed even when another handle holds
 // the file open for writing (e.g. an active log file).