Encode the characters RFC 3986 doesn't allow in a client path

encode_path() sent '"', '<', '>', '\', '^', '`', '{', '|', '}', '['
and ']' raw, although RFC 3986 §3.3 doesn't allow them in a path.
Tomcat, Jetty and Undertow answer each of them with 400 by default, but
accept the percent-encoded form. It also stopped at an embedded NUL, so
"/a\0b" went out as "/a". Encode every byte outside pchar, keeping '/'
and '%' so that escapes pass through, and still encode '+', ',', ';'
and '\'' as before.

encode_request_target() now passes only the path to encode_path(). The
"http://host" of a target sent to a proxy and a CONNECT "host:port" are
sent as they are, so the brackets of an IPv6 host stay.

README.md

@@ -1471,7 +1471,7 @@ auto res = cli.Get("/already%20encoded/path"); // Use pre-encoded paths
 ```
 
 - `set_path_encode(bool on)` - Controls automatic encoding of special characters in URL paths
-  - `true` (default): Automatically encodes spaces, plus signs, newlines, and other special characters
+  - `true` (default): Percent-encodes characters not allowed in a URL path (spaces, control characters, `<`, `|`, non-ASCII bytes, etc.) and `+`, `,`, `;`, `'`. `%` is kept, so existing escapes aren't encoded twice
   - `false`: Sends paths as-is without encoding (useful for pre-encoded URLs)
 
 ### Performance Note for Local Connections

httplib.h

@@ -5999,32 +5999,24 @@ inline size_t FileStat::size() const {
   return ret_ >= 0 ? static_cast<size_t>(st_.st_size) : 0;
 }
 
+// Percent-encode every byte outside RFC 3986 pchar, keeping '/' and '%' so that
+// escapes pass through. '+', ',', ';' and '\'' are pchar but encoded too, as
+// some servers give '+' and ';' a special meaning.
 inline std::string encode_path(const std::string &s) {
+  static const char hex[] = "0123456789ABCDEF";
   std::string result;
   result.reserve(s.size());
 
-  for (size_t i = 0; s[i]; i++) {
-    switch (s[i]) {
-    case ' ': result += "%20"; break;
-    case '+': result += "%2B"; break;
-    case '\'': result += "%27"; break;
-    case ',': result += "%2C"; break;
-    // case ':': result += "%3A"; break; // ok? probably...
-    case ';': result += "%3B"; break;
-    default:
-      auto c = static_cast<uint8_t>(s[i]);
-      // Control characters (incl. CR/LF) and non-ASCII bytes are not allowed
-      // in a request-target as-is.
-      if (c < 0x20 || c == 0x7f || c >= 0x80) {
-        result += '%';
-        char hex[4];
-        auto len = snprintf(hex, sizeof(hex) - 1, "%02X", c);
-        assert(len == 2);
-        result.append(hex, static_cast<size_t>(len));
-      } else {
-        result += s[i];
-      }
-      break;
+  for (auto c : s) {
+    // The '\0' check keeps strchr from matching the terminator.
+    if (is_ascii_alnum(c) ||
+        (c != '\0' && std::strchr("-._~!$&()*=:@/%", c) != nullptr)) {
+      result += c;
+    } else {
+      auto uc = static_cast<unsigned char>(c);
+      result += '%';
+      result += hex[uc >> 4];
+      result += hex[uc & 0x0f];
     }
   }
 
@@ -9323,7 +9315,23 @@ inline std::string encode_request_target(const std::string &uri,
     query_part = target.substr(query_pos + 1);
   }
 
-  auto result = path_encode ? encode_path(path_part) : std::move(path_part);
+  std::string result;
+  if (path_encode) {
+    // Encode only the path: an absolute-form target (sent to a proxy) keeps
+    // its "http://host", and a CONNECT "host:port" has no path.
+    size_t path_pos = 0;
+    if (!path_part.empty() && path_part[0] != '/') {
+      auto scheme_end = path_part.find("://");
+      path_pos = scheme_end == std::string::npos
+                     ? path_part.size()
+                     : (std::min)(path_part.find('/', scheme_end + 3),
+                                  path_part.size());
+    }
+    result = path_part.substr(0, path_pos);
+    result += encode_path(path_part.substr(path_pos));
+  } else {
+    result = std::move(path_part);
+  }
 
   if (!query_part.empty()) {
     // When path encoding is disabled the caller has supplied an already-encoded

test/test.cc

@@ -4457,6 +4457,10 @@ TEST(PathUrlEncodeTest, StreamingMatchesBufferedTarget) {
       {"/a b?x=1", "/a%20b?x=1"},
       {"/\xE6\x97\xA5\xE6\x9C\xAC", "/%E6%97%A5%E6%9C%AC"},
       {"/a,b;c+d", "/a%2Cb%3Bc%2Bd"},
+      // Characters RFC 3986 doesn't allow in a path.
+      {"/\"<>\\^`{|}[]", "/%22%3C%3E%5C%5E%60%7B%7C%7D%5B%5D"},
+      // Escapes and the rest of pchar are kept.
+      {"/a%20b/!$&()*=:@~", "/a%20b/!$&()*=:@~"},
   };
 
   for (const auto &c : cases) {
@@ -4572,7 +4576,55 @@ TEST(PathUrlEncodeTest, ControlCharsInPathAreEncoded) {
     auto res = cli.Get("/a\tb\x1b\x7f");
     ASSERT_TRUE(res) << "Error: " << to_string(res.error());
     EXPECT_EQ("/a%09b%1B%7F", target);
+
+    // An embedded NUL doesn't cut the path short.
+    res = cli.Get(std::string("/a\0b", 4));
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ("/a%00b", target);
+  }
+}
+
+TEST(PathUrlEncodeTest, HostInProxyTargetIsNotEncoded) {
+  // Only the path of a target sent to a proxy is encoded, so the brackets of
+  // an IPv6 host stay.
+  Server proxy;
+
+  std::string target;
+  proxy.set_pre_routing_handler([&](const Request &req, Response &res) {
+    target = req.target;
+    res.status = StatusCode::Forbidden_403;
+    return Server::HandlerResponse::Handled;
+  });
+
+  auto port = proxy.bind_to_any_port(HOST);
+  auto thread = std::thread([&]() { proxy.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    proxy.stop();
+    thread.join();
+    ASSERT_FALSE(proxy.is_running());
+  });
+
+  proxy.wait_until_ready();
+
+  {
+    Client cli("::1", 8080);
+    cli.set_proxy(HOST, port);
+
+    auto res = cli.Get("/a b");
+    ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+    EXPECT_EQ("http://[::1]:8080/a%20b", target);
   }
+
+#ifdef CPPHTTPLIB_SSL_ENABLED
+  {
+    SSLClient cli("::1", 8443);
+    cli.set_proxy(HOST, port);
+
+    auto res = cli.Get("/");
+    EXPECT_EQ(Error::ProxyConnection, res.error());
+    EXPECT_EQ("[::1]:8443", target);
+  }
+#endif
 }
 
 TEST(PathUrlEncodeTest, StreamingCRLFRejectedWhenPathEncodeDisabled) {