Browse Source

Fix query parsing when value has `=` characters (#1822)

* Implement string divider to replace splitter

* Divide query string in half

* Add a test case for query values containing the '=' character

* Add test cases for string divider

* Fix warnings
Jiwoo Park 1 year ago
parent
commit
3b6597bba9
2 changed files with 144 additions and 36 deletions
  1. 54 30
      httplib.h
  2. 90 6
      test/test.cc

+ 54 - 30
httplib.h

@@ -2178,6 +2178,16 @@ void read_file(const std::string &path, std::string &out);
 
 std::string trim_copy(const std::string &s);
 
+void divide(
+    const char *data, std::size_t size, char d,
+    std::function<void(const char *, std::size_t, const char *, std::size_t)>
+        fn);
+
+void divide(
+    const std::string &str, char d,
+    std::function<void(const char *, std::size_t, const char *, std::size_t)>
+        fn);
+
 void split(const char *b, const char *e, char d,
            std::function<void(const char *, const char *)> fn);
 
@@ -2201,6 +2211,8 @@ const char *get_header_value(const Headers &headers, const std::string &key,
 
 std::string params_to_query_str(const Params &params);
 
+void parse_query_text(const char *data, std::size_t size, Params &params);
+
 void parse_query_text(const std::string &s, Params &params);
 
 bool parse_multipart_boundary(const std::string &content_type,
@@ -2669,6 +2681,27 @@ inline std::string trim_double_quotes_copy(const std::string &s) {
   return s;
 }
 
+inline void
+divide(const char *data, std::size_t size, char d,
+       std::function<void(const char *, std::size_t, const char *, std::size_t)>
+           fn) {
+  const auto it = std::find(data, data + size, d);
+  const auto found = static_cast<std::size_t>(it != data + size);
+  const auto lhs_data = data;
+  const auto lhs_size = static_cast<std::size_t>(it - data);
+  const auto rhs_data = it + found;
+  const auto rhs_size = size - lhs_size - found;
+
+  fn(lhs_data, lhs_size, rhs_data, rhs_size);
+}
+
+inline void
+divide(const std::string &str, char d,
+       std::function<void(const char *, std::size_t, const char *, std::size_t)>
+           fn) {
+  divide(str.data(), str.size(), d, std::move(fn));
+}
+
 inline void split(const char *b, const char *e, char d,
                   std::function<void(const char *, const char *)> fn) {
   return split(b, e, d, (std::numeric_limits<size_t>::max)(), std::move(fn));
@@ -4392,22 +4425,22 @@ inline std::string params_to_query_str(const Params &params) {
   return query;
 }
 
-inline void parse_query_text(const std::string &s, Params &params) {
+inline void parse_query_text(const char *data, std::size_t size,
+                             Params &params) {
   std::set<std::string> cache;
-  split(s.data(), s.data() + s.size(), '&', [&](const char *b, const char *e) {
+  split(data, data + size, '&', [&](const char *b, const char *e) {
     std::string kv(b, e);
     if (cache.find(kv) != cache.end()) { return; }
-    cache.insert(kv);
+    cache.insert(std::move(kv));
 
     std::string key;
     std::string val;
-    split(b, e, '=', [&](const char *b2, const char *e2) {
-      if (key.empty()) {
-        key.assign(b2, e2);
-      } else {
-        val.assign(b2, e2);
-      }
-    });
+    divide(b, static_cast<std::size_t>(e - b), '=',
+           [&](const char *lhs_data, std::size_t lhs_size, const char *rhs_data,
+               std::size_t rhs_size) {
+             key.assign(lhs_data, lhs_size);
+             val.assign(rhs_data, rhs_size);
+           });
 
     if (!key.empty()) {
       params.emplace(decode_url(key, true), decode_url(val, true));
@@ -4415,6 +4448,10 @@ inline void parse_query_text(const std::string &s, Params &params) {
   });
 }
 
+inline void parse_query_text(const std::string &s, Params &params) {
+  parse_query_text(s.data(), s.size(), params);
+}
+
 inline bool parse_multipart_boundary(const std::string &content_type,
                                      std::string &boundary) {
   auto boundary_keyword = "boundary=";
@@ -6072,26 +6109,13 @@ inline bool Server::parse_request_line(const char *s, Request &req) const {
       }
     }
 
-    size_t count = 0;
-
-    detail::split(req.target.data(), req.target.data() + req.target.size(), '?',
-                  2, [&](const char *b, const char *e) {
-                    switch (count) {
-                    case 0:
-                      req.path = detail::decode_url(std::string(b, e), false);
-                      break;
-                    case 1: {
-                      if (e - b > 0) {
-                        detail::parse_query_text(std::string(b, e), req.params);
-                      }
-                      break;
-                    }
-                    default: break;
-                    }
-                    count++;
-                  });
-
-    if (count > 2) { return false; }
+    detail::divide(req.target, '?',
+                   [&](const char *lhs_data, std::size_t lhs_size,
+                       const char *rhs_data, std::size_t rhs_size) {
+                     req.path = detail::decode_url(
+                         std::string(lhs_data, lhs_size), false);
+                     detail::parse_query_text(rhs_data, rhs_size, req.params);
+                   });
   }
 
   return true;

+ 90 - 6
test/test.cc

@@ -116,6 +116,76 @@ TEST(TrimTests, TrimStringTests) {
   EXPECT_TRUE(detail::trim_copy("").empty());
 }
 
+TEST(DivideTest, DivideStringTests) {
+  auto divide = [](const std::string &str, char d) {
+    std::string lhs;
+    std::string rhs;
+
+    detail::divide(str, d,
+                   [&](const char *lhs_data, std::size_t lhs_size,
+                       const char *rhs_data, std::size_t rhs_size) {
+                     lhs.assign(lhs_data, lhs_size);
+                     rhs.assign(rhs_data, rhs_size);
+                   });
+
+    return std::make_pair(std::move(lhs), std::move(rhs));
+  };
+
+  {
+    const auto res = divide("", '=');
+    EXPECT_EQ(res.first, "");
+    EXPECT_EQ(res.second, "");
+  }
+
+  {
+    const auto res = divide("=", '=');
+    EXPECT_EQ(res.first, "");
+    EXPECT_EQ(res.second, "");
+  }
+
+  {
+    const auto res = divide(" ", '=');
+    EXPECT_EQ(res.first, " ");
+    EXPECT_EQ(res.second, "");
+  }
+
+  {
+    const auto res = divide("a", '=');
+    EXPECT_EQ(res.first, "a");
+    EXPECT_EQ(res.second, "");
+  }
+
+  {
+    const auto res = divide("a=", '=');
+    EXPECT_EQ(res.first, "a");
+    EXPECT_EQ(res.second, "");
+  }
+
+  {
+    const auto res = divide("=b", '=');
+    EXPECT_EQ(res.first, "");
+    EXPECT_EQ(res.second, "b");
+  }
+
+  {
+    const auto res = divide("a=b", '=');
+    EXPECT_EQ(res.first, "a");
+    EXPECT_EQ(res.second, "b");
+  }
+
+  {
+    const auto res = divide("a=b=", '=');
+    EXPECT_EQ(res.first, "a");
+    EXPECT_EQ(res.second, "b=");
+  }
+
+  {
+    const auto res = divide("a=b=c", '=');
+    EXPECT_EQ(res.first, "a");
+    EXPECT_EQ(res.second, "b=c");
+  }
+}
+
 TEST(SplitTest, ParseQueryString) {
   string s = "key1=val1&key2=val2&key3=val3";
   Params dic;
@@ -156,14 +226,28 @@ TEST(SplitTest, ParseInvalidQueryTests) {
 }
 
 TEST(ParseQueryTest, ParseQueryString) {
-  string s = "key1=val1&key2=val2&key3=val3";
-  Params dic;
+  {
+    std::string s = "key1=val1&key2=val2&key3=val3";
+    Params dic;
 
-  detail::parse_query_text(s, dic);
+    detail::parse_query_text(s, dic);
 
-  EXPECT_EQ("val1", dic.find("key1")->second);
-  EXPECT_EQ("val2", dic.find("key2")->second);
-  EXPECT_EQ("val3", dic.find("key3")->second);
+    EXPECT_EQ("val1", dic.find("key1")->second);
+    EXPECT_EQ("val2", dic.find("key2")->second);
+    EXPECT_EQ("val3", dic.find("key3")->second);
+  }
+
+  {
+    std::string s = "key1&key2=val1&key3=val1=val2&key4=val1=val2=val3";
+    Params dic;
+
+    detail::parse_query_text(s, dic);
+
+    EXPECT_EQ("", dic.find("key1")->second);
+    EXPECT_EQ("val1", dic.find("key2")->second);
+    EXPECT_EQ("val1=val2", dic.find("key3")->second);
+    EXPECT_EQ("val1=val2=val3", dic.find("key4")->second);
+  }
 }
 
 TEST(ParamsToQueryTest, ConvertParamsToQuery) {