Explorar el Código

Follow only 301, 302, 303, 307 and 308 redirects (#2646)

is_redirect treated any 3xx with a Location as a redirect, so with set_follow_location(true) a 304 or 305 that carried a Location was followed. Only 301, 302, 303, 307 and 308 are redirects a client follows (RFC 9110 section 15.4), as in Go, requests and urllib3. A Location is still required. The 304 check in process_request that is_redirect now covers is dropped.
Wleeaf hace 1 día
padre
commit
7d349cbeed
Se han modificado 2 ficheros con 54 adiciones y 5 borrados
  1. 12 5
      httplib.h
  2. 42 0
      test/test.cc

+ 12 - 5
httplib.h

@@ -16213,10 +16213,18 @@ inline bool ChunkedDecoder::parse_trailers_into(Headers &dest,
   return parse_trailers(lr, dest, src_headers);
 }
 
-// A 3xx without a Location, such as a 304, is a final response
+// Only 301, 302, 303, 307 and 308 redirect. Any other 3xx, such as a 304, or
+// a redirect without a Location, is a final response.
 inline bool is_redirect(const Response &res) {
-  return 300 < res.status && res.status < 400 &&
-         !res.get_header_value("Location").empty();
+  switch (res.status) {
+  case StatusCode::MovedPermanently_301:
+  case StatusCode::Found_302:
+  case StatusCode::SeeOther_303:
+  case StatusCode::TemporaryRedirect_307:
+  case StatusCode::PermanentRedirect_308:
+    return !res.get_header_value("Location").empty();
+  default: return false;
+  }
 }
 
 } // namespace detail
@@ -17026,8 +17034,7 @@ inline bool ClientImpl::process_request(Stream &strm, Request &req,
       }
     }
 
-    auto redirect = follow_location_ && detail::is_redirect(res) &&
-                    res.status != StatusCode::NotModified_304;
+    auto redirect = follow_location_ && detail::is_redirect(res);
 
     if (req.response_handler && !redirect) {
       if (!req.response_handler(res)) {

+ 42 - 0
test/test.cc

@@ -4255,6 +4255,48 @@ TEST(RedirectToDifferentPort, TrailingCharactersInPort) {
   EXPECT_FALSE(res);
 }
 
+TEST(RedirectTest, OtherStatusesWithLocationAreNotFollowed) {
+  Server svr;
+  std::atomic<bool> followed{false};
+
+  svr.Get("/304", [](const Request & /*req*/, Response &res) {
+    res.status = StatusCode::NotModified_304;
+    res.set_header("Location", "/target");
+  });
+
+  svr.Get("/305", [](const Request & /*req*/, Response &res) {
+    res.status = StatusCode::UseProxy_305;
+    res.set_header("Location", "/target");
+  });
+
+  svr.Get("/target", [&](const Request & /*req*/, Response &res) {
+    followed = true;
+    res.set_content("target", "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);
+
+  auto res = cli.Get("/304");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::NotModified_304, res->status);
+
+  res = cli.Get("/305");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::UseProxy_305, res->status);
+
+  EXPECT_FALSE(followed);
+}
+
 TEST(RedirectFromPageWithContent, Redirect) {
   Server svr;