Remove dot segments from absolute redirect locations (#2648)

resolve_relative_location removed "." and ".." segments only from a relative-path Location. A Location that starts with "/" or an absolute URI was requested as is, so a redirect to /dir/../target went out as /dir/../target and a server that does not normalize the path answered 404.

RFC 3986 section 5.2.2 applies remove_dot_segments to the target path of every reference that has a path, and Go and urllib3 do the same. The loop now lives in detail::remove_dot_segments, and ClientImpl::redirect applies it to an absolute path as well. Relative locations resolve as before.

Author: wleeaf

httplib.h

@@ -995,6 +995,30 @@ inline bool parse_url(const std::string &url, UrlComponents &uc) {
   return true;
 }
 
+// Removes "." and ".." segments from a path that starts with '/' (RFC 3986
+// section 5.2.4).
+inline std::string remove_dot_segments(const std::string &path) {
+  std::string out;
+  size_t i = 0;
+  while (i < path.size()) {
+    auto next = (std::min)(path.find('/', i + 1), path.size());
+    auto segment = path.substr(i + 1, next - i - 1);
+    auto is_last = next == path.size();
+    if (segment == "." || segment == "..") {
+      if (segment == "..") {
+        out.erase((std::min)(out.rfind('/'), out.size()));
+      }
+      if (is_last) { out += '/'; }
+    } else {
+      out += '/';
+      out += segment;
+    }
+    i = next;
+  }
+  if (out.empty()) { out = "/"; }
+  return out;
+}
+
 // Resolves a relative-path or query-only Location value against the path of
 // the request being redirected (RFC 3986 section 5.2). Absolute URIs and
 // references starting with '/' are returned unchanged.
@@ -1015,26 +1039,8 @@ inline std::string resolve_relative_location(const std::string &location,
 
   // Remove "." and ".." segments from the merged path.
   auto path_end = (std::min)(merged.find_first_of("?#"), merged.size());
-  std::string path;
-  size_t i = 0;
-  while (i < path_end) {
-    auto next = (std::min)(merged.find('/', i + 1), path_end);
-    auto segment = merged.substr(i + 1, next - i - 1);
-    auto is_last = next == path_end;
-    if (segment == "." || segment == "..") {
-      if (segment == "..") {
-        path.erase((std::min)(path.rfind('/'), path.size()));
-      }
-      if (is_last) { path += '/'; }
-    } else {
-      path += '/';
-      path += segment;
-    }
-    i = next;
-  }
-  if (path.empty()) { path = "/"; }
-
-  return path + merged.substr(path_end);
+  return remove_dot_segments(merged.substr(0, path_end)) +
+         merged.substr(path_end);
 }
 
 } // namespace detail
@@ -16405,6 +16411,10 @@ inline bool ClientImpl::redirect(Request &req, Response &res, Error &error) {
   if (next_scheme.empty()) { next_scheme = scheme; }
   if (next_host.empty()) { next_host = host_; }
   if (next_path.empty()) { next_path = "/"; }
+  // An absolute or absolute-path location can have dot segments too.
+  if (next_path[0] == '/') {
+    next_path = detail::remove_dot_segments(next_path);
+  }
 
   auto path = std::move(next_path) + next_query;
 

test/test.cc

@@ -20593,6 +20593,51 @@ TEST(RedirectTest, ResolveRelativeLocation) {
   }
 }
 
+TEST(RedirectTest, RemoveDotSegments) {
+  // RFC 3986 section 5.2.4
+  EXPECT_EQ("/a/g", detail::remove_dot_segments("/a/b/c/./../../g"));
+  EXPECT_EQ("/mid/6", detail::remove_dot_segments("/mid/content=5/../6"));
+  EXPECT_EQ("/g", detail::remove_dot_segments("/./g"));
+  EXPECT_EQ("/g", detail::remove_dot_segments("/../g"));
+  EXPECT_EQ("/", detail::remove_dot_segments("/.."));
+  EXPECT_EQ("/a/", detail::remove_dot_segments("/a/b/.."));
+  EXPECT_EQ("/a/b", detail::remove_dot_segments("/a/b"));
+}
+
+TEST(RedirectTest, AbsoluteLocationIsNormalized) {
+  Server svr;
+
+  svr.Get("/absolute-path", [](const Request & /*req*/, Response &res) {
+    res.set_redirect("/dir/../target?x=1");
+  });
+  svr.Get("/absolute-uri", [](const Request & /*req*/, Response &res) {
+    res.set_redirect("http://localhost:" + std::to_string(PORT) +
+                     "/dir/./../target");
+  });
+  svr.Get("/target", [](const Request &req, Response &res) {
+    res.set_content(req.path, "text/plain");
+  });
+
+  auto th = std::thread([&]() { svr.listen("localhost", PORT); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    th.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  Client cli("localhost", PORT);
+  cli.set_follow_location(true);
+
+  for (auto path : {"/absolute-path", "/absolute-uri"}) {
+    auto res = cli.Get(path);
+    ASSERT_TRUE(res) << path << ": " << to_string(res.error());
+    EXPECT_EQ(StatusCode::OK_200, res->status) << path;
+    EXPECT_EQ("/target", res->body) << path;
+  }
+}
+
 TEST(RedirectTest, RelativeLocationWithoutLeadingSlash) {
   Server svr;