nghttpx: Forward multiple via, xff, and xfp header fields

Previously, for Via, X-Forwarded-For, and X-Forwarded-Proto header
field, nghttpx only forwarded the last header field of each.  With
this commit, nghttpx forwards all of them if it is configured to do
so.

src/http2.cc

@@ -358,15 +358,21 @@ nghttp2_nv make_nv_nocopy(const StringRef &name, const StringRef &value,
 
 namespace {
 void copy_headers_to_nva_internal(std::vector<nghttp2_nv> &nva,
-                                  const HeaderRefs &headers, uint8_t nv_flags) {
+                                  const HeaderRefs &headers, uint8_t nv_flags,
-  for (auto &kv : headers) {
+                                  uint32_t flags) {
-    if (kv.name.empty() || kv.name[0] == ':') {
+  auto it_forwarded = std::end(headers);
+  auto it_xff = std::end(headers);
+  auto it_xfp = std::end(headers);
+  auto it_via = std::end(headers);
+
+  for (auto it = std::begin(headers); it != std::end(headers); ++it) {
+    auto kv = &(*it);
+    if (kv->name.empty() || kv->name[0] == ':') {
       continue;
     }
-    switch (kv.token) {
+    switch (kv->token) {
     case HD_COOKIE:
     case HD_CONNECTION:
-    case HD_FORWARDED:
     case HD_HOST:
     case HD_HTTP2_SETTINGS:
     case HD_KEEP_ALIVE:
@@ -375,51 +381,157 @@ void copy_headers_to_nva_internal(std::vector<nghttp2_nv> &nva,
     case HD_TE:
     case HD_TRANSFER_ENCODING:
     case HD_UPGRADE:
-    case HD_VIA:
-    case HD_X_FORWARDED_FOR:
-    case HD_X_FORWARDED_PROTO:
       continue;
+    case HD_FORWARDED:
+      if (flags & HDOP_STRIP_FORWARDED) {
+        continue;
+      }
+
+      if (it_forwarded == std::end(headers)) {
+        it_forwarded = it;
+        continue;
+      }
+
+      kv = &(*it_forwarded);
+      it_forwarded = it;
+      break;
+    case HD_X_FORWARDED_FOR:
+      if (flags & HDOP_STRIP_X_FORWARDED_FOR) {
+        continue;
+      }
+
+      if (it_xff == std::end(headers)) {
+        it_xff = it;
+        continue;
+      }
+
+      kv = &(*it_xff);
+      it_xff = it;
+      break;
+    case HD_X_FORWARDED_PROTO:
+      if (flags & HDOP_STRIP_X_FORWARDED_PROTO) {
+        continue;
+      }
+
+      if (it_xfp == std::end(headers)) {
+        it_xfp = it;
+        continue;
+      }
+
+      kv = &(*it_xfp);
+      it_xfp = it;
+      break;
+    case HD_VIA:
+      if (flags & HDOP_STRIP_VIA) {
+        continue;
+      }
+
+      if (it_via == std::end(headers)) {
+        it_via = it;
+        continue;
+      }
+
+      kv = &(*it_via);
+      it_via = it;
+      break;
     }
-    nva.push_back(make_nv_internal(kv.name, kv.value, kv.no_index, nv_flags));
+    nva.push_back(
+        make_nv_internal(kv->name, kv->value, kv->no_index, nv_flags));
   }
 }
 } // namespace
 
 void copy_headers_to_nva(std::vector<nghttp2_nv> &nva,
-                         const HeaderRefs &headers) {
+                         const HeaderRefs &headers, uint32_t flags) {
-  copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NONE);
+  copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NONE, flags);
 }
 
 void copy_headers_to_nva_nocopy(std::vector<nghttp2_nv> &nva,
-                                const HeaderRefs &headers) {
+                                const HeaderRefs &headers, uint32_t flags) {
   copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NO_COPY_NAME |
-                                                 NGHTTP2_NV_FLAG_NO_COPY_VALUE);
+                                                 NGHTTP2_NV_FLAG_NO_COPY_VALUE,
+                               flags);
 }
 
 void build_http1_headers_from_headers(DefaultMemchunks *buf,
-                                      const HeaderRefs &headers) {
+                                      const HeaderRefs &headers,
-  for (auto &kv : headers) {
+                                      uint32_t flags) {
-    if (kv.name.empty() || kv.name[0] == ':') {
+  auto it_forwarded = std::end(headers);
+  auto it_xff = std::end(headers);
+  auto it_xfp = std::end(headers);
+  auto it_via = std::end(headers);
+
+  for (auto it = std::begin(headers); it != std::end(headers); ++it) {
+    auto kv = &(*it);
+    if (kv->name.empty() || kv->name[0] == ':') {
       continue;
     }
-    switch (kv.token) {
+    switch (kv->token) {
     case HD_CONNECTION:
     case HD_COOKIE:
-    case HD_FORWARDED:
     case HD_HOST:
     case HD_HTTP2_SETTINGS:
     case HD_KEEP_ALIVE:
     case HD_PROXY_CONNECTION:
     case HD_SERVER:
     case HD_UPGRADE:
-    case HD_VIA:
-    case HD_X_FORWARDED_FOR:
-    case HD_X_FORWARDED_PROTO:
       continue;
+    case HD_FORWARDED:
+      if (flags & HDOP_STRIP_FORWARDED) {
+        continue;
+      }
+
+      if (it_forwarded == std::end(headers)) {
+        it_forwarded = it;
+        continue;
+      }
+
+      kv = &(*it_forwarded);
+      it_forwarded = it;
+      break;
+    case HD_X_FORWARDED_FOR:
+      if (flags & HDOP_STRIP_X_FORWARDED_FOR) {
+        continue;
+      }
+
+      if (it_xff == std::end(headers)) {
+        it_xff = it;
+        continue;
+      }
+
+      kv = &(*it_xff);
+      it_xff = it;
+      break;
+    case HD_X_FORWARDED_PROTO:
+      if (flags & HDOP_STRIP_X_FORWARDED_PROTO) {
+        continue;
+      }
+
+      if (it_xfp == std::end(headers)) {
+        it_xfp = it;
+        continue;
+      }
+
+      kv = &(*it_xfp);
+      it_xfp = it;
+      break;
+    case HD_VIA:
+      if (flags & HDOP_STRIP_VIA) {
+        continue;
+      }
+
+      if (it_via == std::end(headers)) {
+        it_via = it;
+        continue;
+      }
+
+      kv = &(*it_via);
+      it_via = it;
+      break;
     }
-    capitalize(buf, kv.name);
+    capitalize(buf, kv->name);
     buf->append(": ");
-    buf->append(kv.value);
+    buf->append(kv->value);
     buf->append("\r\n");
   }
 }

src/http2.h

@@ -187,24 +187,50 @@ nghttp2_nv make_nv_ls_nocopy(const char (&name)[N], const StringRef &value) {
           NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE};
 }
 
+enum HeaderBuildOp {
+  HDOP_NONE,
+  // Forwarded header fields must be stripped.  If this flag is not
+  // set, all Forwarded header fields other than last one are added.
+  HDOP_STRIP_FORWARDED = 1,
+  // X-Forwarded-For header fields must be stripped.  If this flag is
+  // not set, all X-Forwarded-For header fields other than last one
+  // are added.
+  HDOP_STRIP_X_FORWARDED_FOR = 1 << 1,
+  // X-Forwarded-Proto header fields must be stripped.  If this flag
+  // is not set, all X-Forwarded-Proto header fields other than last
+  // one are added.
+  HDOP_STRIP_X_FORWARDED_PROTO = 1 << 2,
+  // Via header fields must be stripped.  If this flag is not set, all
+  // Via header fields other than last one are added.
+  HDOP_STRIP_VIA = 1 << 3,
+  // Strip above all header fields.
+  HDOP_STRIP_ALL = HDOP_STRIP_FORWARDED | HDOP_STRIP_X_FORWARDED_FOR |
+                   HDOP_STRIP_X_FORWARDED_PROTO | HDOP_STRIP_VIA,
+};
+
 // Appends headers in |headers| to |nv|.  |headers| must be indexed
 // before this call (its element's token field is assigned).  Certain
 // headers, including disallowed headers in HTTP/2 spec and headers
-// which require special handling (i.e. via), are not copied.
+// which require special handling (i.e. via), are not copied.  |flags|
+// is one or more of HeaderBuildOp flags.  They tell function that
+// certain header fields should not be added.
 void copy_headers_to_nva(std::vector<nghttp2_nv> &nva,
-                         const HeaderRefs &headers);
+                         const HeaderRefs &headers, uint32_t flags);
 
 // Just like copy_headers_to_nva(), but this adds
 // NGHTTP2_NV_FLAG_NO_COPY_NAME and NGHTTP2_NV_FLAG_NO_COPY_VALUE.
 void copy_headers_to_nva_nocopy(std::vector<nghttp2_nv> &nva,
-                                const HeaderRefs &headers);
+                                const HeaderRefs &headers, uint32_t flags);
 
 // Appends HTTP/1.1 style header lines to |buf| from headers in
 // |headers|.  |headers| must be indexed before this call (its
 // element's token field is assigned).  Certain headers, which
 // requires special handling (i.e. via and cookie), are not appended.
+// |flags| is one or more of HeaderBuildOp flags.  They tell function
+// that certain header fields should not be added.
 void build_http1_headers_from_headers(DefaultMemchunks *buf,
-                                      const HeaderRefs &headers);
+                                      const HeaderRefs &headers,
+                                      uint32_t flags);
 
 // Return positive window_size_increment if WINDOW_UPDATE should be
 // sent for the stream |stream_id|. If |stream_id| == 0, this function

src/http2_test.cc

@@ -150,11 +150,33 @@ auto headers = HeaderRefs{
     {StringRef::from_lit("zulu"), StringRef::from_lit("12")}};
 } // namespace
 
+namespace {
+auto headers2 = HeaderRefs{
+    {StringRef::from_lit("x-forwarded-for"), StringRef::from_lit("xff1"), false,
+     http2::HD_X_FORWARDED_FOR},
+    {StringRef::from_lit("x-forwarded-for"), StringRef::from_lit("xff2"), false,
+     http2::HD_X_FORWARDED_FOR},
+    {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("xfp1"),
+     false, http2::HD_X_FORWARDED_PROTO},
+    {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("xfp2"),
+     false, http2::HD_X_FORWARDED_PROTO},
+    {StringRef::from_lit("forwarded"), StringRef::from_lit("fwd1"), false,
+     http2::HD_FORWARDED},
+    {StringRef::from_lit("forwarded"), StringRef::from_lit("fwd2"), false,
+     http2::HD_FORWARDED},
+    {StringRef::from_lit("via"), StringRef::from_lit("via1"), false,
+     http2::HD_VIA},
+    {StringRef::from_lit("via"), StringRef::from_lit("via2"), false,
+     http2::HD_VIA},
+};
+} // namespace
+
 void test_http2_copy_headers_to_nva(void) {
   auto ans = std::vector<int>{0, 1, 4, 5, 6, 7, 12};
   std::vector<nghttp2_nv> nva;
 
-  http2::copy_headers_to_nva_nocopy(nva, headers);
+  http2::copy_headers_to_nva_nocopy(nva, headers,
+                                    http2::HDOP_STRIP_X_FORWARDED_FOR);
   CU_ASSERT(7 == nva.size());
   for (size_t i = 0; i < ans.size(); ++i) {
     check_nv(headers[ans[i]], &nva[i]);
@@ -169,7 +191,7 @@ void test_http2_copy_headers_to_nva(void) {
   }
 
   nva.clear();
-  http2::copy_headers_to_nva(nva, headers);
+  http2::copy_headers_to_nva(nva, headers, http2::HDOP_STRIP_X_FORWARDED_FOR);
   CU_ASSERT(7 == nva.size());
   for (size_t i = 0; i < ans.size(); ++i) {
     check_nv(headers[ans[i]], &nva[i]);
@@ -180,12 +202,27 @@ void test_http2_copy_headers_to_nva(void) {
       CU_ASSERT(NGHTTP2_NV_FLAG_NONE == nva[i].flags);
     }
   }
+
+  nva.clear();
+
+  auto ans2 = std::vector<int>{0, 2, 4, 6};
+  http2::copy_headers_to_nva(nva, headers2, http2::HDOP_NONE);
+  CU_ASSERT(ans2.size() == nva.size());
+  for (size_t i = 0; i < ans2.size(); ++i) {
+    check_nv(headers2[ans2[i]], &nva[i]);
+  }
+
+  nva.clear();
+
+  http2::copy_headers_to_nva(nva, headers2, http2::HDOP_STRIP_ALL);
+  CU_ASSERT(nva.empty());
 }
 
 void test_http2_build_http1_headers_from_headers(void) {
   MemchunkPool pool;
   DefaultMemchunks buf(&pool);
-  http2::build_http1_headers_from_headers(&buf, headers);
+  http2::build_http1_headers_from_headers(&buf, headers,
+                                          http2::HDOP_STRIP_X_FORWARDED_FOR);
   auto hdrs = std::string(buf.head->pos, buf.head->last);
   CU_ASSERT("Alpha: 0\r\n"
             "Bravo: 1\r\n"
@@ -196,6 +233,21 @@ void test_http2_build_http1_headers_from_headers(void) {
             "Te: 8\r\n"
             "Te: 9\r\n"
             "Zulu: 12\r\n" == hdrs);
+
+  buf.reset();
+
+  http2::build_http1_headers_from_headers(&buf, headers2, http2::HDOP_NONE);
+  hdrs = std::string(buf.head->pos, buf.head->last);
+  CU_ASSERT("X-Forwarded-For: xff1\r\n"
+            "X-Forwarded-Proto: xfp1\r\n"
+            "Forwarded: fwd1\r\n"
+            "Via: via1\r\n" == hdrs);
+
+  buf.reset();
+
+  http2::build_http1_headers_from_headers(&buf, headers2,
+                                          http2::HDOP_STRIP_ALL);
+  CU_ASSERT(0 == buf.rleft());
 }
 
 void test_http2_lws(void) {

src/shrpx_http2_downstream_connection.cc

@@ -203,7 +203,7 @@ ssize_t http2_data_read_callback(nghttp2_session *session, int32_t stream_id,
       nva.reserve(trailers.size());
       // We cannot use nocopy version, since nva may be touched after
       // Downstream object is deleted.
-      http2::copy_headers_to_nva(nva, trailers);
+      http2::copy_headers_to_nva(nva, trailers, http2::HDOP_STRIP_ALL);
       if (!nva.empty()) {
         rv = nghttp2_submit_trailer(session, stream_id, nva.data(), nva.size());
         if (rv != 0) {
@@ -310,7 +310,16 @@ int Http2DownstreamConnection::push_request_headers() {
     nva.push_back(http2::make_nv_ls_nocopy(":authority", authority));
   }
 
-  http2::copy_headers_to_nva_nocopy(nva, req.fs.headers());
+  auto &fwdconf = httpconf.forwarded;
+  auto &xffconf = httpconf.xff;
+  auto &xfpconf = httpconf.xfp;
+
+  uint32_t build_flags =
+      (fwdconf.strip_incoming ? http2::HDOP_STRIP_FORWARDED : 0) |
+      (xffconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_FOR : 0) |
+      (xfpconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_PROTO : 0);
+
+  http2::copy_headers_to_nva_nocopy(nva, req.fs.headers(), build_flags);
 
   if (!http2conf.no_cookie_crumbling) {
     downstream_->crumble_request_cookie(nva);
@@ -319,8 +328,6 @@ int Http2DownstreamConnection::push_request_headers() {
   auto upstream = downstream_->get_upstream();
   auto handler = upstream->get_client_handler();
 
-  auto &fwdconf = httpconf.forwarded;
-
   auto fwd =
       fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED);
 
@@ -351,8 +358,6 @@ int Http2DownstreamConnection::push_request_headers() {
     nva.push_back(http2::make_nv_ls_nocopy("forwarded", fwd->value));
   }
 
-  auto &xffconf = httpconf.xff;
-
   auto xff = xffconf.strip_incoming ? nullptr
                                     : req.fs.header(http2::HD_X_FORWARDED_FOR);
 
@@ -371,7 +376,6 @@ int Http2DownstreamConnection::push_request_headers() {
   }
 
   if (!config->http2_proxy && req.method != HTTP_CONNECT) {
-    auto &xfpconf = httpconf.xfp;
     auto xfp = xfpconf.strip_incoming
                    ? nullptr
                    : req.fs.header(http2::HD_X_FORWARDED_PROTO);

src/shrpx_http2_upstream.cc

@@ -1418,7 +1418,7 @@ ssize_t downstream_data_read_callback(nghttp2_session *session,
       if (!trailers.empty()) {
         std::vector<nghttp2_nv> nva;
         nva.reserve(trailers.size());
-        http2::copy_headers_to_nva_nocopy(nva, trailers);
+        http2::copy_headers_to_nva_nocopy(nva, trailers, http2::HDOP_STRIP_ALL);
         if (!nva.empty()) {
           rv = nghttp2_submit_trailer(session, stream_id, nva.data(),
                                       nva.size());
@@ -1667,7 +1667,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) {
   nva.push_back(http2::make_nv_ls_nocopy(":status", response_status));
 
   if (downstream->get_non_final_response()) {
-    http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers());
+    http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers(),
+                                      http2::HDOP_STRIP_ALL);
 
     if (LOG_ENABLED(INFO)) {
       log_response_headers(downstream, nva);
@@ -1687,7 +1688,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) {
     return 0;
   }
 
-  http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers());
+  http2::copy_headers_to_nva_nocopy(
+      nva, resp.fs.headers(), http2::HDOP_STRIP_ALL & ~http2::HDOP_STRIP_VIA);
 
   if (!config->http2_proxy && !httpconf.no_server_rewrite) {
     nva.push_back(http2::make_nv_ls_nocopy("server", httpconf.server_name));

src/shrpx_http_downstream_connection.cc

@@ -538,7 +538,16 @@ int HttpDownstreamConnection::push_request_headers() {
   buf->append(authority);
   buf->append("\r\n");
 
-  http2::build_http1_headers_from_headers(buf, req.fs.headers());
+  auto &fwdconf = httpconf.forwarded;
+  auto &xffconf = httpconf.xff;
+  auto &xfpconf = httpconf.xfp;
+
+  uint32_t build_flags =
+      (fwdconf.strip_incoming ? http2::HDOP_STRIP_FORWARDED : 0) |
+      (xffconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_FOR : 0) |
+      (xfpconf.strip_incoming ? http2::HDOP_STRIP_X_FORWARDED_PROTO : 0);
+
+  http2::build_http1_headers_from_headers(buf, req.fs.headers(), build_flags);
 
   auto cookie = downstream_->assemble_request_cookie();
   if (!cookie.empty()) {
@@ -577,8 +586,6 @@ int HttpDownstreamConnection::push_request_headers() {
   auto upstream = downstream_->get_upstream();
   auto handler = upstream->get_client_handler();
 
-  auto &fwdconf = httpconf.forwarded;
-
   auto fwd =
       fwdconf.strip_incoming ? nullptr : req.fs.header(http2::HD_FORWARDED);
 
@@ -611,8 +618,6 @@ int HttpDownstreamConnection::push_request_headers() {
     buf->append("\r\n");
   }
 
-  auto &xffconf = httpconf.xff;
-
   auto xff = xffconf.strip_incoming ? nullptr
                                     : req.fs.header(http2::HD_X_FORWARDED_FOR);
 
@@ -630,7 +635,6 @@ int HttpDownstreamConnection::push_request_headers() {
     buf->append("\r\n");
   }
   if (!config->http2_proxy && !connect_method) {
-    auto &xfpconf = httpconf.xfp;
     auto xfp = xfpconf.strip_incoming
                    ? nullptr
                    : req.fs.header(http2::HD_X_FORWARDED_PROTO);
@@ -740,7 +744,8 @@ int HttpDownstreamConnection::end_upload_data() {
     output->append("0\r\n\r\n");
   } else {
     output->append("0\r\n");
-    http2::build_http1_headers_from_headers(output, trailers);
+    http2::build_http1_headers_from_headers(output, trailers,
+                                            http2::HDOP_STRIP_ALL);
     output->append("\r\n");
   }
 

src/shrpx_https_upstream.cc

@@ -1059,9 +1059,10 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
         get_client_handler()->get_upstream_scheme());
   }
 
-  http2::build_http1_headers_from_headers(buf, resp.fs.headers());
-
   if (downstream->get_non_final_response()) {
+    http2::build_http1_headers_from_headers(buf, resp.fs.headers(),
+                                            http2::HDOP_STRIP_ALL);
+
     buf->append("\r\n");
 
     if (LOG_ENABLED(INFO)) {
@@ -1073,6 +1074,9 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
     return 0;
   }
 
+  http2::build_http1_headers_from_headers(
+      buf, resp.fs.headers(), http2::HDOP_STRIP_ALL & ~http2::HDOP_STRIP_VIA);
+
   auto worker = handler_->get_worker();
 
   // after graceful shutdown commenced, add connection: close header
@@ -1205,7 +1209,8 @@ int HttpsUpstream::on_downstream_body_complete(Downstream *downstream) {
       output->append("0\r\n\r\n");
     } else {
       output->append("0\r\n");
-      http2::build_http1_headers_from_headers(output, trailers);
+      http2::build_http1_headers_from_headers(output, trailers,
+                                              http2::HDOP_STRIP_ALL);
       output->append("\r\n");
     }
   }