Drop invalid response header fields before any header writer

The server left response header fields with an invalid name or value,
such as a value carrying CR/LF that would split the response, for the
default header writer to skip, so a writer set with set_header_writer()
wrote them. Drop such fields from res.headers right after the handlers
that can fill it directly have run, before the server reads it for
framing and range decisions and before any header writer sees it. The
client already rejects invalid request headers before writing them.

httplib.h

@@ -8419,6 +8419,18 @@ inline void set_header(Headers &headers, const std::string &key,
   if (fields::is_field_valid(key, val)) { headers.emplace(key, val); }
 }
 
+// Drops the fields set_header() would refuse, from headers an application can
+// fill directly: CR/LF in one would split the message.
+inline void erase_invalid_fields(Headers &headers) {
+  for (auto it = headers.begin(); it != headers.end();) {
+    if (fields::is_field_valid(it->first, it->second)) {
+      ++it;
+    } else {
+      it = headers.erase(it);
+    }
+  }
+}
+
 inline bool read_headers(Stream &strm, Headers &headers) {
   const auto bufsiz = 2048;
   char buf[bufsiz];
@@ -8896,13 +8908,6 @@ inline ssize_t write_response_line(Stream &strm, int status) {
 inline ssize_t write_headers(Stream &strm, const Headers &headers) {
   ssize_t write_len = 0;
   for (const auto &x : headers) {
-    // Skip fields with invalid names or values to prevent response splitting
-    // via CR/LF injection, matching set_header(). The client validates request
-    // headers up front in check_and_write_headers, but the server passes
-    // res.headers straight to this writer, and res.headers is a public field
-    // an application can populate directly with request-derived values.
-    if (!fields::is_field_valid(x.first, x.second)) { continue; }
-
     std::string s;
     s = x.first;
     s += ": ";
@@ -14004,6 +14009,10 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
     need_apply_ranges = true;
   }
 
+  // Before anything below reads res.headers, which the handlers may have
+  // filled directly
+  detail::erase_invalid_fields(res.headers);
+
   if (!detail::body_allowed_for_status(res.status)) {
     detail::clear_content(res);
   }
@@ -14047,6 +14056,7 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
 
   if (post_routing_handler_) {
     post_routing_handler_(req, res);
+    detail::erase_invalid_fields(res.headers);
     // Again, for a body or framing headers the handler changed.
     detail::set_framing_headers(req, res, boundary, content_type);
   }

test/test.cc

@@ -11231,48 +11231,66 @@ TEST(ServerResponseSplittingTest, ChunkedTrailerCRLFInjection) {
 }
 
 TEST(ServerResponseSplittingTest, ResponseHeaderCRLFInjection) {
-  Server svr;
-  svr.Get("/injected-header", [&](const Request & /*req*/, Response &res) {
-    // res.headers is a public field an application can populate directly,
-    // bypassing set_header()'s validation (e.g. reflecting a request value).
-    // A valid header that must survive.
-    res.headers.emplace("X-Safe", "safe");
-    // Attacker-controlled value carrying CR/LF (response splitting).
-    res.headers.emplace("X-Reflected",
-                        "legit\r\nInjected-Header: pwned\r\nX-Evil: also");
-    // Attacker-controlled name carrying CR/LF.
-    res.headers.emplace("X-Bad\r\nSet-Cookie: a=1", "x");
-    res.set_content("body", "text/plain");
-  });
+  // A custom header writer gets only the fields the default one writes
+  auto verbatim_writer = [](Stream &strm, Headers &hdrs) -> ssize_t {
+    std::string s;
+    for (const auto &h : hdrs) {
+      s += h.first + ": " + h.second + "\r\n";
+    }
+    s += "\r\n";
+    return strm.write(s.data(), s.size());
+  };
 
-  thread t = thread([&] { svr.listen(HOST, PORT); });
-  auto se = detail::scope_exit([&] {
-    svr.stop();
-    t.join();
-    ASSERT_FALSE(svr.is_running());
-  });
+  for (auto custom_writer : {false, true}) {
+    SCOPED_TRACE(custom_writer ? "custom header writer" : "default writer");
+    Server svr;
+    if (custom_writer) { svr.set_header_writer(verbatim_writer); }
+    svr.set_post_routing_handler([](const Request &, Response &res) {
+      res.headers.emplace("X-Late", "late\r\nInjected-Late: pwned");
+    });
+    svr.Get("/injected-header", [&](const Request & /*req*/, Response &res) {
+      // res.headers is a public field an application can populate directly,
+      // bypassing set_header()'s validation (e.g. reflecting a request value).
+      // A valid header that must survive.
+      res.headers.emplace("X-Safe", "safe");
+      // Attacker-controlled value carrying CR/LF (response splitting).
+      res.headers.emplace("X-Reflected",
+                          "legit\r\nInjected-Header: pwned\r\nX-Evil: also");
+      // Attacker-controlled name carrying CR/LF.
+      res.headers.emplace("X-Bad\r\nSet-Cookie: a=1", "x");
+      res.set_content("body", "text/plain");
+    });
 
-  svr.wait_until_ready();
+    thread t = thread([&] { svr.listen(HOST, PORT); });
+    auto se = detail::scope_exit([&] {
+      svr.stop();
+      t.join();
+      ASSERT_FALSE(svr.is_running());
+    });
 
-  const std::string req = std::string("GET /injected-header HTTP/1.1\r\n") +
-                          "Host: " + HOST +
-                          "\r\n"
-                          "Connection: close\r\n"
-                          "\r\n";
+    svr.wait_until_ready();
 
-  std::string res;
-  ASSERT_TRUE(send_request(5, req, &res));
+    const std::string req = std::string("GET /injected-header HTTP/1.1\r\n") +
+                            "Host: " + HOST +
+                            "\r\n"
+                            "Connection: close\r\n"
+                            "\r\n";
 
-  // The injected fields must not appear anywhere in the raw response.
-  EXPECT_EQ(std::string::npos, res.find("Injected-Header"));
-  EXPECT_EQ(std::string::npos, res.find("X-Evil"));
-  EXPECT_EQ(std::string::npos, res.find("Set-Cookie"));
+    std::string res;
+    ASSERT_TRUE(send_request(5, req, &res));
 
-  // Invalid headers are dropped entirely, matching set_header().
-  EXPECT_EQ(std::string::npos, res.find("X-Reflected:"));
+    // The injected fields must not appear anywhere in the raw response.
+    EXPECT_EQ(std::string::npos, res.find("Injected-Header"));
+    EXPECT_EQ(std::string::npos, res.find("X-Evil"));
+    EXPECT_EQ(std::string::npos, res.find("Set-Cookie"));
+    EXPECT_EQ(std::string::npos, res.find("Injected-Late"));
 
-  // The valid header still passes through.
-  EXPECT_NE(std::string::npos, res.find("X-Safe: safe"));
+    // Invalid headers are dropped entirely, matching set_header().
+    EXPECT_EQ(std::string::npos, res.find("X-Reflected:"));
+
+    // The valid header still passes through.
+    EXPECT_NE(std::string::npos, res.find("X-Safe: safe"));
+  }
 }
 
 // Forward declaration: in split builds split.py strips `inline` and moves the