Browse Source

Parse status lines without std::regex

std::regex_match recursed once per character of a status line's reason
phrase, so a server that sent one as long as a whole line overflowed the
client's stack. file_extension() and append_query_params() ran the same
kind of regex over a request path and a caller's path.

Parse all three by hand, with the same results, except that a path that
starts with '?' now gets its parameters joined with '&'. Serving a static
file no longer defers the content type lookup to keep a regex off the 304
path, so it looks the type up once.
yhirose 2 ngày trước cách đây
mục cha
commit
2e278fd6e9
2 tập tin đã thay đổi với 139 bổ sung và 26 xóa
  1. 35 26
      httplib.h
  2. 104 0
      test/test.cc

+ 35 - 26
httplib.h

@@ -5917,10 +5917,14 @@ inline std::string encode_path(const std::string &s) {
 }
 
 inline std::string file_extension(const std::string &path) {
-  std::smatch m;
-  thread_local auto re = std::regex("\\.([a-zA-Z0-9]+)$");
-  if (std::regex_search(path, m, re)) { return m[1].str(); }
-  return std::string();
+  auto i = path.size();
+  while (i > 0 && is_ascii_alnum(path[i - 1])) {
+    i--;
+  }
+  if (i == path.size() || i == 0 || path[i - 1] != '.') {
+    return std::string();
+  }
+  return path.substr(i);
 }
 
 inline bool is_space_or_tab(char c) { return c == ' ' || c == '\t'; }
@@ -8292,19 +8296,35 @@ inline bool read_headers(Stream &strm, Headers &headers) {
   return true;
 }
 
+// Parsed by hand: std::regex_match recurses once per character of the reason
+// phrase, which a server can make as long as a whole line.
 inline bool parse_status_line(const char *line, std::string &version,
                               int &status, std::string &reason) {
+  if (std::strncmp(line, "HTTP/1.", 7) != 0 ||
+      (line[7] != '0' && line[7] != '1') || line[8] != ' ' ||
+      !is_ascii_digit(line[9]) || !is_ascii_digit(line[10]) ||
+      !is_ascii_digit(line[11])) {
+    return false;
+  }
+
+  auto p = line + 12;
+  auto reason_begin = p;
+  if (*p == ' ') {
+    reason_begin = ++p;
+    p += std::strcspn(p, "\r\n");
+  }
+  auto reason_end = p;
+
 #ifdef CPPHTTPLIB_ALLOW_LF_AS_LINE_TERMINATOR
-  thread_local const std::regex re("(HTTP/1\\.[01]) (\\d{3})(?: (.*?))?\r?\n");
+  if (*p == '\r') { p++; }
 #else
-  thread_local const std::regex re("(HTTP/1\\.[01]) (\\d{3})(?: (.*?))?\r\n");
+  if (*p++ != '\r') { return false; }
 #endif
+  if (*p++ != '\n' || *p != '\0') { return false; }
 
-  std::cmatch m;
-  if (!std::regex_match(line, m, re)) { return false; }
-  version = std::string(m[1]);
-  status = std::stoi(std::string(m[2]));
-  reason = std::string(m[3]);
+  version.assign(line, 8);
+  status = (line[9] - '0') * 100 + (line[10] - '0') * 10 + (line[11] - '0');
+  reason.assign(reason_begin, reason_end);
   return true;
 }
 
@@ -11636,9 +11656,8 @@ inline std::string sanitize_filename(const std::string &filename) {
 inline std::string append_query_params(const std::string &path,
                                        const Params &params) {
   std::string path_with_query = path;
-  thread_local const std::regex re("[^?]+\\?.*");
-  auto delm = std::regex_match(path, re) ? '&' : '?';
-  path_with_query += delm + detail::params_to_query_str(params);
+  path_with_query += path.find('?') == std::string::npos ? '?' : '&';
+  path_with_query += detail::params_to_query_str(params);
   return path_with_query;
 }
 
@@ -13937,18 +13956,10 @@ inline bool Server::handle_file_request(Request &req, Response &res) {
             res.set_header(kv.first, kv.second);
           }
 
-          auto content_type_of = [&]() {
-            return detail::find_content_type(
-                path, file_extension_and_mimetype_map_, default_file_mimetype_);
-          };
-
-          // Only the ETag needs the content type this early, and only to name
-          // the coding. Deciding it here would otherwise put a regex in front
-          // of the 304 below, which serving a file never used to pay for.
-          std::string content_type;
+          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_) {
-            content_type = content_type_of();
             encoding =
                 static_file_encoding(req, res, content_type, stat.size());
           }
@@ -13980,8 +13991,6 @@ inline bool Server::handle_file_request(Request &req, Response &res) {
             return false;
           }
 
-          if (!static_file_compression_) { content_type = content_type_of(); }
-
           detail::set_file_content_provider(res, mm, content_type, encoding);
 
           if (req.method != "HEAD" && file_request_handler_) {

+ 104 - 0
test/test.cc

@@ -548,6 +548,95 @@ TEST(ChunkedTransferEncodingTest, DetectsChunkedAsFinalCoding) {
   EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"chunked", ""})));
 }
 
+// Forward declarations (see the base64_encode note below).
+namespace httplib {
+namespace detail {
+bool parse_status_line(const char *line, std::string &version, int &status,
+                       std::string &reason);
+std::string file_extension(const std::string &path);
+} // namespace detail
+} // namespace httplib
+
+TEST(ParseStatusLineTest, ParsesVersionStatusAndReason) {
+  struct {
+    const char *line;
+    const char *version;
+    int status;
+    const char *reason;
+  } cases[] = {
+      {"HTTP/1.1 200 OK\r\n", "HTTP/1.1", 200, "OK"},
+      {"HTTP/1.0 404 Not Found\r\n", "HTTP/1.0", 404, "Not Found"},
+      // The reason phrase may be empty, or left out along with its space.
+      {"HTTP/1.1 204 \r\n", "HTTP/1.1", 204, ""},
+      {"HTTP/1.1 304\r\n", "HTTP/1.1", 304, ""},
+  };
+
+  for (const auto &c : cases) {
+    std::string version;
+    int status = 0;
+    std::string reason = "unset";
+    EXPECT_TRUE(detail::parse_status_line(c.line, version, status, reason))
+        << c.line;
+    EXPECT_EQ(c.version, version);
+    EXPECT_EQ(c.status, status);
+    EXPECT_EQ(c.reason, reason);
+  }
+}
+
+TEST(ParseStatusLineTest, RejectsMalformedLines) {
+  const char *lines[] = {
+      "HTTP/2 200 OK\r\n",
+      "HTTP/1.2 200 OK\r\n",
+      "HTTP/1.1 20 OK\r\n",
+      "HTTP/1.1 2000 OK\r\n",
+      "HTTP/1.1 2x0 OK\r\n",
+      "HTTP/1.1  200 OK\r\n",
+      "HTTP/1.1 200OK\r\n",
+      "HTTP/1.1 200 O\rK\r\n",
+      "HTTP/1.1 200 OK",
+      "HTTP/1.1 200 OK\r\nX",
+      "",
+#ifndef CPPHTTPLIB_ALLOW_LF_AS_LINE_TERMINATOR
+      "HTTP/1.1 200 OK\n",
+#endif
+  };
+
+  for (auto line : lines) {
+    std::string version;
+    int status = 0;
+    std::string reason;
+    EXPECT_FALSE(detail::parse_status_line(line, version, status, reason))
+        << line;
+  }
+}
+
+TEST(ParseStatusLineTest, AcceptsAReasonPhraseAsLongAsALine) {
+  // Long enough to overflow the stack when matched with std::regex.
+  auto long_reason = std::string(CPPHTTPLIB_MAX_LINE_LENGTH - 15, 'A');
+  auto line = "HTTP/1.1 200 " + long_reason + "\r\n";
+
+  std::string version;
+  int status = 0;
+  std::string reason;
+  EXPECT_TRUE(detail::parse_status_line(line.c_str(), version, status, reason));
+  EXPECT_EQ(200, status);
+  EXPECT_EQ(long_reason, reason);
+}
+
+TEST(FileExtensionTest, TakesTheAlphanumericSuffix) {
+  EXPECT_EQ("txt", detail::file_extension("a.txt"));
+  EXPECT_EQ("gz", detail::file_extension("/dir/a.tar.gz"));
+  EXPECT_EQ("htaccess", detail::file_extension(".htaccess"));
+  EXPECT_EQ("", detail::file_extension("noext"));
+  EXPECT_EQ("", detail::file_extension("a."));
+  EXPECT_EQ("", detail::file_extension("dir.d/file"));
+  EXPECT_EQ("", detail::file_extension("a.b-c"));
+
+  // Long enough to overflow the stack when matched with std::regex.
+  auto long_ext = std::string(CPPHTTPLIB_MAX_LINE_LENGTH, 'a');
+  EXPECT_EQ(long_ext, detail::file_extension("a." + long_ext));
+}
+
 // Forward declaration: in split builds split.py strips `inline` and moves the
 // definition into httplib.cc, so detail::base64_encode is not visible from the
 // public httplib.h. Re-declaring it here lets the tests link against the symbol
@@ -1379,6 +1468,21 @@ TEST(ParamsToQueryTest, ConvertParamsToQuery) {
   EXPECT_EQ(detail::params_to_query_str(dic), "key1=val1&key2=val2&key3=val3");
 }
 
+TEST(AppendQueryParamsTest, JoinsAnExistingQuery) {
+  Params params;
+  params.emplace("a", "1");
+
+  EXPECT_EQ("/p?a=1", append_query_params("/p", params));
+  EXPECT_EQ("/p?x=0&a=1", append_query_params("/p?x=0", params));
+  EXPECT_EQ("/p?&a=1", append_query_params("/p?", params));
+  EXPECT_EQ("?x=0&a=1", append_query_params("?x=0", params));
+
+  // Long enough to overflow the stack when matched with std::regex.
+  auto long_query = std::string(CPPHTTPLIB_MAX_LINE_LENGTH, 'x');
+  EXPECT_EQ("/p?" + long_query + "&a=1",
+            append_query_params("/p?" + long_query, params));
+}
+
 // Joins every entry of a Headers or Params into "key=value " so a whole
 // traversal can be compared in one assertion. Both are instantiations of the
 // same insertion-ordered container, so one helper serves both.