Merge pull request #1744 from nghttp2/nghttpx-remove-h1-field-trailing-white-spaces

nghttpx: Remove trailing white spaces from HTTP/1.1 fields

src/shrpx-unittest.cc

@@ -202,6 +202,7 @@ int main(int argc, char *argv[]) {
       !CU_add_test(pSuite, "util_split_hostport",
                    shrpx::test_util_split_hostport) ||
       !CU_add_test(pSuite, "util_split_str", shrpx::test_util_split_str) ||
+      !CU_add_test(pSuite, "util_rstrip", shrpx::test_util_rstrip) ||
       !CU_add_test(pSuite, "gzip_inflate", test_nghttp2_gzip_inflate) ||
       !CU_add_test(pSuite, "buffer_write", nghttp2::test_buffer_write) ||
       !CU_add_test(pSuite, "pool_recycle", nghttp2::test_pool_recycle) ||

src/shrpx_downstream.h

@@ -75,6 +75,7 @@ public:
   const HeaderRefs &trailers() const { return trailers_; }
 
   HeaderRefs &headers() { return headers_; }
+  HeaderRefs &trailers() { return trailers_; }
 
   const void add_extra_buffer_size(size_t n) { buffer_size_ += n; }
   size_t buffer_size() const { return buffer_size_; }

src/shrpx_http_downstream_connection.cc

@@ -912,6 +912,12 @@ int htp_hdrs_completecb(llhttp_t *htp) {
   auto &resp = downstream->response();
   int rv;
 
+  auto &balloc = downstream->get_block_allocator();
+
+  for (auto &kv : resp.fs.headers()) {
+    kv.value = util::rstrip(balloc, kv.value);
+  }
+
   auto config = get_config();
   auto &loggingconf = config->logging;
 
@@ -1138,6 +1144,12 @@ int htp_bodycb(llhttp_t *htp, const char *data, size_t len) {
 namespace {
 int htp_msg_completecb(llhttp_t *htp) {
   auto downstream = static_cast<Downstream *>(htp->data);
+  auto &resp = downstream->response();
+  auto &balloc = downstream->get_block_allocator();
+
+  for (auto &kv : resp.fs.trailers()) {
+    kv.value = util::rstrip(balloc, kv.value);
+  }
 
   // llhttp does not treat "200 connection established" response
   // against CONNECT request, and in that case, this function is not

src/shrpx_https_upstream.cc

@@ -328,6 +328,11 @@ int htp_hdrs_completecb(llhttp_t *htp) {
 
   auto downstream = upstream->get_downstream();
   auto &req = downstream->request();
+  auto &balloc = downstream->get_block_allocator();
+
+  for (auto &kv : req.fs.headers()) {
+    kv.value = util::rstrip(balloc, kv.value);
+  }
 
   auto lgconf = log_config();
   lgconf->update_tstamp(std::chrono::system_clock::now());
@@ -397,7 +402,6 @@ int htp_hdrs_completecb(llhttp_t *htp) {
   downstream->inspect_http1_request();
 
   auto faddr = handler->get_upstream_addr();
-  auto &balloc = downstream->get_block_allocator();
   auto config = get_config();
 
   if (method != HTTP_CONNECT) {
@@ -555,6 +559,13 @@ int htp_msg_completecb(llhttp_t *htp) {
   }
   auto handler = upstream->get_client_handler();
   auto downstream = upstream->get_downstream();
+  auto &req = downstream->request();
+  auto &balloc = downstream->get_block_allocator();
+
+  for (auto &kv : req.fs.trailers()) {
+    kv.value = util::rstrip(balloc, kv.value);
+  }
+
   downstream->set_request_state(DownstreamState::MSG_COMPLETE);
   rv = downstream->end_upload_data();
   if (rv != 0) {

src/util.cc

@@ -1688,6 +1688,19 @@ int daemonize(int nochdir, int noclose) {
 #endif // !__APPLE__
 }
 
+StringRef rstrip(BlockAllocator &balloc, const StringRef &s) {
+  auto it = std::rbegin(s);
+  for (; it != std::rend(s) && (*it == ' ' || *it == '\t'); ++it)
+    ;
+
+  auto len = it - std::rbegin(s);
+  if (len == 0) {
+    return s;
+  }
+
+  return make_string_ref(balloc, StringRef{s.c_str(), s.size() - len});
+}
+
 #ifdef ENABLE_HTTP3
 int msghdr_get_local_addr(Address &dest, msghdr *msg, int family) {
   switch (family) {

src/util.h

@@ -916,6 +916,12 @@ std::mt19937 make_mt19937();
 // daemon() using fork().
 int daemonize(int nochdir, int noclose);
 
+// Returns |s| from which trailing white spaces (SPC or HTAB) are
+// removed.  If any white spaces are removed, new string is allocated
+// by |balloc| and returned.  Otherwise, the copy of |s| is returned
+// without allocation.
+StringRef rstrip(BlockAllocator &balloc, const StringRef &s);
+
 #ifdef ENABLE_HTTP3
 int msghdr_get_local_addr(Address &dest, msghdr *msg, int family);
 

src/util_test.cc

@@ -695,4 +695,14 @@ void test_util_split_str(void) {
       util::split_str(StringRef::from_lit("alpha,bravo,charlie"), ',', 1));
 }
 
+void test_util_rstrip(void) {
+  BlockAllocator balloc(4096, 4096);
+
+  CU_ASSERT("alpha" == util::rstrip(balloc, StringRef::from_lit("alpha")));
+  CU_ASSERT("alpha" == util::rstrip(balloc, StringRef::from_lit("alpha ")));
+  CU_ASSERT("alpha" == util::rstrip(balloc, StringRef::from_lit("alpha \t")));
+  CU_ASSERT("" == util::rstrip(balloc, StringRef::from_lit("")));
+  CU_ASSERT("" == util::rstrip(balloc, StringRef::from_lit("\t\t\t   ")));
+}
+
 } // namespace shrpx

src/util_test.h

@@ -69,6 +69,7 @@ void test_util_decode_hex(void);
 void test_util_extract_host(void);
 void test_util_split_hostport(void);
 void test_util_split_str(void);
+void test_util_rstrip(void);
 
 } // namespace shrpx