Explorar el Código

Return a 3xx without a Location instead of failing the redirect

With follow_location on, every 3xx was taken for a redirect, so a 304
Not Modified, or any 3xx without a Location, failed with Error::Unknown.
Follow a 3xx only when it has a Location, as curl does, and return the
others as the response, as curl, Go, requests and urllib3 do. Response
handlers and content receivers now get such a response too.
yhirose hace 2 días
padre
commit
106edcf606
Se han modificado 2 ficheros con 81 adiciones y 4 borrados
  1. 9 4
      httplib.h
  2. 72 0
      test/test.cc

+ 9 - 4
httplib.h

@@ -16128,6 +16128,12 @@ 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
+inline bool is_redirect(const Response &res) {
+  return 300 < res.status && res.status < 400 &&
+         !res.get_header_value("Location").empty();
+}
+
 } // namespace detail
 
 inline void
@@ -16185,7 +16191,7 @@ inline bool ClientImpl::handle_request(Stream &strm, Request &req,
     }
   }
 
-  if (300 < res.status && res.status < 400 && follow_location_) {
+  if (follow_location_ && detail::is_redirect(res)) {
     req = std::move(req_save);
     // The client that followed the redirect has answered any challenge it
     // could. Retrying below would sign another host's challenge and send it
@@ -16931,9 +16937,8 @@ inline bool ClientImpl::process_request(Stream &strm, Request &req,
       }
     }
 
-    auto redirect = 300 < res.status && res.status < 400 &&
-                    res.status != StatusCode::NotModified_304 &&
-                    follow_location_;
+    auto redirect = follow_location_ && detail::is_redirect(res) &&
+                    res.status != StatusCode::NotModified_304;
 
     if (req.response_handler && !redirect) {
       if (!req.response_handler(res)) {

+ 72 - 0
test/test.cc

@@ -19849,6 +19849,78 @@ TEST(RedirectTest, LocationPathIsNotDecoded) {
   }
 }
 
+TEST(RedirectTest, NotModifiedIsReturned) {
+  Server svr;
+  svr.Get("/cached", [](const Request &, Response &res) {
+    res.status = StatusCode::NotModified_304;
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto thread = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    thread.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+  cli.set_follow_location(true);
+
+  auto res = cli.Get("/cached", Headers{{"If-None-Match", "\"v1\""}});
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::NotModified_304, res->status);
+}
+
+TEST(RedirectTest, NoLocationIsReturned) {
+  Server svr;
+  svr.Get("/no-location", [](const Request &, Response &res) {
+    res.status = StatusCode::Found_302;
+    res.set_content("no location", "text/plain");
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto thread = std::thread([&]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    thread.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+  svr.wait_until_ready();
+
+  Client cli(HOST, port);
+  cli.set_follow_location(true);
+
+  auto res = cli.Get("/no-location");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::Found_302, res->status);
+  EXPECT_EQ("no location", res->body);
+
+  // The callbacks see the body of the response that is returned
+  auto handler_called = false;
+  auto progress_called = false;
+  std::string body;
+  auto res2 = cli.Get(
+      "/no-location",
+      [&](const Response &) {
+        handler_called = true;
+        return true;
+      },
+      [&](const char *data, size_t data_length) {
+        body.append(data, data_length);
+        return true;
+      },
+      [&](size_t, size_t) {
+        progress_called = true;
+        return true;
+      });
+  ASSERT_TRUE(res2) << "Error: " << to_string(res2.error());
+  EXPECT_EQ(StatusCode::Found_302, res2->status);
+  EXPECT_TRUE(handler_called);
+  EXPECT_TRUE(progress_called);
+  EXPECT_EQ("no location", body);
+}
+
 TEST(RedirectTest, RedirectToUrlWithQueryParameters) {
   Server svr;