ソースを参照

Read the request line strictly

Any run of spaces or tabs separated the parts of the request line, a
fragment in the target was stripped, and an HTTP/1.1 request without
Host was served, so a server or a cache in front that follows RFC 9112
could see another request. Require a single SP between the parts, and
answer a target with '#' and an HTTP/1.1 request without Host with 400.
The client no longer sends the fragment of a URL.
yhirose 1 日 前
親
コミット
3420ba8fca
2 ファイル変更、69 行追加、27 行削除
  1. 29 26
      httplib.h
  2. 40 1
      test/test.cc

+ 29 - 26
httplib.h

@@ -9197,8 +9197,11 @@ inline std::string normalize_query_string(const std::string &query) {
 // Build the request target that goes on the wire from a caller-supplied path.
 // Shared by the buffered send path and the streaming API so that both put the
 // same bytes in the request line for the same input.
-inline std::string encode_request_target(const std::string &target,
+inline std::string encode_request_target(const std::string &uri,
                                          bool path_encode) {
+  // The fragment stays with the client (RFC 9110 §7.1).
+  auto target = uri.substr(0, uri.find('#'));
+
   // `substr(0, npos)` yields the whole string, which is what the no-query
   // case needs.
   auto query_pos = target.find('?');
@@ -13597,19 +13600,19 @@ inline bool Server::parse_request_line(const char *s, Request &req) const {
   len -= 2;
 
   {
-    size_t count = 0;
-
-    detail::split(s, s + len, ' ', [&](const char *b, const char *e) {
-      switch (count) {
-      case 0: req.method = std::string(b, e); break;
-      case 1: req.target = std::string(b, e); break;
-      case 2: req.version = std::string(b, e); break;
-      default: break;
-      }
-      count++;
-    });
-
-    if (count != 3) { return false; }
+    // RFC 9112 §3: exactly one SP between the method, the target and the
+    // version. Taking other whitespace as a separator too lets recipients
+    // that parse more strictly see a different request.
+    auto end = s + len;
+    auto sp1 = std::find(s, end, ' ');
+    auto sp2 = sp1 == end ? end : std::find(sp1 + 1, end, ' ');
+    if (sp2 == end || std::find(sp2 + 1, end, ' ') != end || sp1 == s ||
+        sp2 == sp1 + 1 || sp2 + 1 == end) {
+      return false;
+    }
+    req.method.assign(s, sp1);
+    req.target.assign(sp1 + 1, sp2);
+    req.version.assign(sp2 + 1, end);
   }
 
   // A method outside the built-in set is accepted only when a handler has been
@@ -13627,17 +13630,15 @@ inline bool Server::parse_request_line(const char *s, Request &req) const {
     return false;
   }
 
-  if (!detail::fields::is_request_target(req.target)) { return false; }
+  // A request-target has no fragment (RFC 9112 §3.2), and one stripped here
+  // would leave this server routing a different path than a cache or a
+  // filter in front of it sees.
+  if (!detail::fields::is_request_target(req.target) ||
+      req.target.find('#') != std::string::npos) {
+    return false;
+  }
 
   {
-    // Skip URL fragment
-    for (size_t i = 0; i < req.target.size(); i++) {
-      if (req.target[i] == '#') {
-        req.target.erase(i);
-        break;
-      }
-    }
-
     detail::divide(req.target, '?',
                    [&](const char *lhs_data, std::size_t lhs_size,
                        const char *rhs_data, std::size_t rhs_size) {
@@ -14768,10 +14769,12 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
     connection_closed = true;
   }
 
-  // RFC 9112 §3.2: Reject more than one Host field line or an invalid Host
-  // value, so that every recipient of the request sees the same authority.
+  // RFC 9112 §3.2: Reject more than one Host field line, an invalid Host
+  // value, or none in HTTP/1.1, so that every recipient of the request sees
+  // the same authority.
   if (req.get_header_value_count("Host") > 1 ||
-      !detail::fields::is_host(req.get_header_value("Host"))) {
+      !detail::fields::is_host(req.get_header_value("Host")) ||
+      (req.version == "HTTP/1.1" && !req.has_header("Host"))) {
     connection_closed = true;
     res.status = StatusCode::BadRequest_400;
     output_error_log(Error::InvalidHeaders, &req);

+ 40 - 1
test/test.cc

@@ -10434,6 +10434,7 @@ TEST(HeadersOrderTest, ReceivedFieldsKeepTheirOrder) {
   svr.wait_until_ready();
 
   const std::string req = "GET /order HTTP/1.1\r\n"
+                          "Host: localhost\r\n"
                           "X-First: 1\r\n"
                           "X-Dup: a\r\n"
                           "X-Second: 2\r\n"
@@ -10444,7 +10445,9 @@ TEST(HeadersOrderTest, ReceivedFieldsKeepTheirOrder) {
   std::string res;
   ASSERT_TRUE(send_request(5, req, &res, port));
   EXPECT_EQ("HTTP/1.1 200 OK", res.substr(0, 15));
-  EXPECT_EQ("X-First=1 X-Dup=a X-Second=2 X-Dup=b Connection=close ", received);
+  EXPECT_EQ(
+      "Host=localhost X-First=1 X-Dup=a X-Second=2 X-Dup=b Connection=close ",
+      received);
 }
 
 TEST(HeadersOrderTest, SentFieldsKeepTheirOrder) {
@@ -10836,10 +10839,34 @@ TEST(ServerRequestParsingTest, ExcessiveWhitespaceInUnparsableHeaderLine) {
                    "\r\n");
 }
 
+TEST(ServerRequestParsingTest, StrictRequestLine) {
+  const char *rejected[] = {
+      // RFC 9112 §3: one SP between the parts, and nothing else
+      "GET  /hi HTTP/1.1\r\nHost: localhost\r\n\r\n",
+      "GET\t/hi HTTP/1.1\r\nHost: localhost\r\n\r\n",
+      "GET /hi\tHTTP/1.1\r\nHost: localhost\r\n\r\n",
+      "GET /hi HTTP/1.1 \r\nHost: localhost\r\n\r\n",
+      // A request-target has no fragment
+      "GET /hi#x HTTP/1.1\r\nHost: localhost\r\n\r\n",
+      // An HTTP/1.1 request needs a Host
+      "GET /hi HTTP/1.1\r\n\r\n",
+  };
+  for (auto request : rejected) {
+    std::string out;
+    test_raw_request(request, &out);
+    EXPECT_EQ("HTTP/1.1 400 Bad Request", out.substr(0, 24)) << request;
+  }
+
+  std::string out;
+  test_raw_request("GET /hi HTTP/1.0\r\n\r\n", &out);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15));
+}
+
 TEST(ServerRequestParsingTest, AmbiguousTransferEncodingClosesConnection) {
   // RFC 9112 §6.1
   const char *requests[] = {
       "PUT /put_hi HTTP/1.1\r\n"
+      "Host: localhost\r\n"
       "Content-Length: 0\r\n"
       "Transfer-Encoding: chunked\r\n"
       "\r\n"
@@ -10921,6 +10948,7 @@ TEST(ServerRequestParsingTest, InvalidControlCharInURL) {
 TEST(ServerRequestParsingTest, NonAsciiInURLAccepted) {
   std::string out;
   test_raw_request("GET /hi?q=\xE3\x81\x82 HTTP/1.1\r\n"
+                   "Host: localhost\r\n"
                    "Connection: close\r\n\r\n",
                    &out);
   EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15));
@@ -11052,6 +11080,7 @@ TEST(ServerRequestParsingTest, EmptyFieldValue) {
   std::string out;
 
   test_raw_request("GET /header_field_value_check HTTP/1.1\r\n"
+                   "Host: localhost\r\n"
                    "Test: \r\n\r\n",
                    &out);
   EXPECT_EQ("HTTP/1.1 200 OK", out.substr(0, 15));
@@ -17556,6 +17585,7 @@ TEST(MultipartFormDataTest, AlternateFilename) {
   svr.wait_until_ready();
 
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Type: multipart/form-data;boundary=--------\r\n"
              "Content-Length: 399\r\n"
              "\r\n"
@@ -17624,6 +17654,7 @@ TEST(MultipartFormDataTest, AlternateFilenameLongValueAndCaseInsensitive) {
                      "------------\r\n";
 
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Type: multipart/form-data;boundary=--------\r\n"
              "Content-Length: " +
              std::to_string(body.size()) + "\r\n\r\n" + body;
@@ -17658,6 +17689,7 @@ TEST(MultipartFormDataTest, CloseDelimiterWithoutCRLF) {
   svr.wait_until_ready();
 
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Type: multipart/form-data;boundary=--------\r\n"
              "Content-Length: 146\r\n"
              "\r\n----------\r\n"
@@ -17702,6 +17734,7 @@ TEST(MultipartFormDataTest, ContentLength) {
   svr.wait_until_ready();
 
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Type: multipart/form-data;boundary=--------\r\n"
              "Content-Length: 167\r\n"
              "\r\n----------\r\n"
@@ -17756,6 +17789,7 @@ TEST(MultipartFormDataTest, AccessPartHeaders) {
   svr.wait_until_ready();
 
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Type: multipart/form-data;boundary=--------\r\n"
              "Content-Length: 232\r\n"
              "\r\n----------\r\n"
@@ -17810,6 +17844,7 @@ TEST(MultipartFormDataTest, LargeHeader) {
                         "--" +
                         boundary + "--\r\n";
   std::string header_prefix = "POST /test HTTP/1.1\r\n"
+                              "Host: localhost\r\n"
                               "Content-Type: multipart/form-data;boundary=" +
                               boundary +
                               "\r\n"
@@ -18176,6 +18211,7 @@ static void expect_split_multipart_ok(const std::string &body1,
   // response drain ends immediately instead of idling until the read timeout.
   const std::string head =
       "POST /post HTTP/1.1\r\n"
+      "Host: localhost\r\n"
       "Content-Type: multipart/form-data; boundary=zzzz\r\n"
       "Connection: close\r\n"
       "Content-Length: " +
@@ -20505,6 +20541,7 @@ TEST(RequestSmugglingTest, DuplicateContentLengthSameValues) {
 
   // Two Content-Length headers with same value — should be accepted (RFC 9110)
   auto req = "POST /test HTTP/1.1\r\n"
+             "Host: localhost\r\n"
              "Content-Length: 5\r\n"
              "Content-Length: 5\r\n"
              "\r\n"
@@ -20650,6 +20687,7 @@ TEST(HeaderSmugglingTest, DuplicateTrailerFieldLinesDeclareAllTrailers) {
   svr.wait_until_ready();
 
   const std::string req = "GET / HTTP/1.1\r\n"
+                          "Host: localhost\r\n"
                           "Transfer-Encoding: chunked\r\n"
                           "Trailer: X-Hello\r\n"
                           "Trailer: X-World, Content-Length\r\n"
@@ -20758,6 +20796,7 @@ TEST(HeaderSmugglingTest, DeclaredTrailerNamesAreCappedAtHeaderMaxCount) {
   trailer_decl += "\r\n";
 
   const std::string req = "GET / HTTP/1.1\r\n"
+                          "Host: localhost\r\n"
                           "Transfer-Encoding: chunked\r\n" +
                           trailer_decl +
                           "\r\n"