From baa2272b0a2a0880cc9e7c1694b8318f2c662e15 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Dec 2013 00:32:14 +0900 Subject: [PATCH] src: Concatenate header fields with same name by NULL as delimiter cookie and set-cookie are treated specially and won't be concatenated. --- src/http2.cc | 29 +++++++++-- src/http2.h | 8 +++ src/http2_test.cc | 12 +++++ src/http2_test.h | 1 + src/nghttp.cc | 65 ++++++++++-------------- src/shrpx-unittest.cc | 2 + src/shrpx_downstream.cc | 6 +++ src/shrpx_downstream.h | 4 ++ src/shrpx_http2_downstream_connection.cc | 8 +-- src/shrpx_http2_upstream.cc | 1 + src/util.cc | 1 + 11 files changed, 90 insertions(+), 47 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 2886af95..5289ef5c 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -319,6 +319,26 @@ nghttp2_nv make_nv(const std::string& name, const std::string& value) }; } +std::vector> +concat_norm_headers +(std::vector> headers) +{ + auto res = std::vector>(); + res.reserve(headers.size()); + for(auto& kv : headers) { + if(!res.empty() && res.back().first == kv.first && + kv.first != "cookie" && kv.first != "set-cookie") { + if(!kv.second.empty()) { + res.back().second.append(1, '\0'); + res.back().second += kv.second; + } + } else { + res.push_back(std::move(kv)); + } + } + return res; +} + void copy_norm_headers_to_nva (std::vector& nva, const std::vector>& headers) @@ -407,11 +427,12 @@ void dump_nv(FILE *out, const char **nv) void dump_nv(FILE *out, const nghttp2_nv *nva, size_t nvlen) { - for(size_t i = 0; i < nvlen; ++i) { - auto nv = &nva[i]; - fwrite(nv->name, nv->namelen, 1, out); + // |nva| may have NULL-concatenated header fields + auto v = sort_nva(nva, nvlen); + for(auto& nv : v) { + fwrite(nv.name, nv.namelen, 1, out); fwrite(": ", 2, 1, out); - fwrite(nv->value, nv->valuelen, 1, out); + fwrite(nv.value, nv.valuelen, 1, out); fwrite("\n", 1, 1, out); } fwrite("\n", 1, 1, out); diff --git a/src/http2.h b/src/http2.h index 5a85bb4c..5f0faf35 100644 --- a/src/http2.h +++ b/src/http2.h @@ -113,6 +113,14 @@ bool value_lws(const nghttp2_nv *nv); // and not contain illegal characters. bool non_empty_value(const nghttp2_nv* nv); +// Concatenates field with same value by NULL as delimiter and returns +// new vector containing the resulting header fields. cookie and +// set-cookie header fields won't be concatenated. This function +// assumes that the |headers| is sorted by name. +std::vector> +concat_norm_headers +(std::vector> headers); + // Creates nghttp2_nv using |name| and |value| and returns it. The // returned value only references the data pointer to name.c_str() and // value.c_str(). diff --git a/src/http2_test.cc b/src/http2_test.cc index d5ab3523..9f92ef00 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -164,6 +164,18 @@ auto headers = std::vector> {"zulu", "12"}}; } // namespace +void test_http2_concat_norm_headers(void) +{ + auto hds = headers; + hds.emplace_back("cookie", "foo"); + hds.emplace_back("cookie", "bar"); + hds.emplace_back("set-cookie", "baz"); + hds.emplace_back("set-cookie", "buzz"); + auto res = http2::concat_norm_headers(hds); + CU_ASSERT(14 == res.size()); + CU_ASSERT(std::string("2") + '\0' + std::string("3") == res[2].second); +} + void test_http2_copy_norm_headers_to_nva(void) { std::vector nva; diff --git a/src/http2_test.h b/src/http2_test.h index 12523ee9..d1c87e27 100644 --- a/src/http2_test.h +++ b/src/http2_test.h @@ -32,6 +32,7 @@ void test_http2_check_http2_headers(void); void test_http2_get_unique_header(void); void test_http2_get_header(void); void test_http2_value_lws(void); +void test_http2_concat_norm_headers(void); void test_http2_copy_norm_headers_to_nva(void); void test_http2_build_http1_headers_from_norm_headers(void); void test_http2_check_header_value(void); diff --git a/src/nghttp.cc b/src/nghttp.cc index 128404b8..d1cb18fb 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -926,57 +926,47 @@ int submit_request }; auto path = req->make_reqpath(); auto scheme = get_uri_field(req->uri.c_str(), req->u, UF_SCHEMA); - const char *static_nv[] = { - ":method", req->data_prd ? "POST" : "GET", - ":path", path.c_str(), - ":scheme", scheme.c_str(), - ":authority", client->hostport.c_str(), - "accept", "*/*", - "accept-encoding", "gzip, deflate", - "user-agent", "nghttp2/" NGHTTP2_VERSION - }; + auto build_headers = std::vector> + {{":method", req->data_prd ? "POST" : "GET"}, + {":path", path}, + {":scheme", scheme}, + {":authority", client->hostport}, + {"accept", "*/*"}, + {"accept-encoding", "gzip, deflate"}, + {"user-agent", "nghttp2/" NGHTTP2_VERSION}}; - int hardcoded_entry_count = sizeof(static_nv) / sizeof(*static_nv); - int header_count = headers.size(); - int total_entry_count = hardcoded_entry_count + header_count * 2; if(req->data_prd) { - total_entry_count += 2; - } - - auto nv = util::make_unique(total_entry_count + 1); - - memcpy(nv.get(), static_nv, hardcoded_entry_count * sizeof(*static_nv)); - - int pos = hardcoded_entry_count; - - std::string content_length_str; - if(req->data_prd) { - content_length_str = util::utos(req->data_length); - nv[pos++] = "content-length"; - nv[pos++] = content_length_str.c_str(); + build_headers.emplace_back("content-length", util::utos(req->data_length)); } for(auto& kv : headers) { auto key = kv.first.c_str(); - auto value = kv.second.c_str(); if ( util::strieq( key, "accept" ) ) { - nv[POS_ACCEPT*2+1] = value; + build_headers[POS_ACCEPT].second = kv.second; } else if ( util::strieq( key, "user-agent" ) ) { - nv[POS_USERAGENT*2+1] = value; + build_headers[POS_USERAGENT].second = kv.second; } else if ( util::strieq( key, ":authority" ) ) { - nv[POS_AUTHORITY*2+1] = value; + build_headers[POS_AUTHORITY].second = kv.second; } else { - nv[pos] = key; - nv[pos+1] = value; - pos += 2; + build_headers.push_back(kv); } } - nv[pos] = nullptr; - - int rv = nghttp2_submit_request(client->session, req->pri, - nv.get(), req->data_prd, req); + std::stable_sort(std::begin(build_headers), std::end(build_headers), + [](const std::pair& lhs, + const std::pair& rhs) + { + return lhs.first < rhs.first; + }); + build_headers = http2::concat_norm_headers(std::move(build_headers)); + auto nva = std::vector(); + nva.reserve(build_headers.size()); + for(auto& kv : build_headers) { + nva.push_back(http2::make_nv(kv.first, kv.second)); + } + int rv = nghttp2_submit_request2(client->session, req->pri, + nva.data(), nva.size(), req->data_prd, req); if(rv != 0) { std::cerr << "nghttp2_submit_request() returned error: " << nghttp2_strerror(rv) << std::endl; @@ -1744,6 +1734,7 @@ int main(int argc, char **argv) // Note that there is no processing currently to handle multiple // message-header fields with the same field name config.headers.emplace_back(header, value); + util::inp_strlower(config.headers.back().first); break; } case 'a': diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 6ebdaace..db1bb49c 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -78,6 +78,8 @@ int main(int argc, char* argv[]) shrpx::test_http2_get_header) || !CU_add_test(pSuite, "http2_value_lws", shrpx::test_http2_value_lws) || + !CU_add_test(pSuite, "http2_concat_norm_headers", + shrpx::test_http2_concat_norm_headers) || !CU_add_test(pSuite, "http2_copy_norm_headers_to_nva", shrpx::test_http2_copy_norm_headers_to_nva) || !CU_add_test(pSuite, "http2_build_http1_headers_from_norm_headers", diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index ac0a733e..b7d3bb00 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -32,6 +32,7 @@ #include "shrpx_error.h" #include "shrpx_downstream_connection.h" #include "util.h" +#include "http2.h" using namespace nghttp2; @@ -455,6 +456,11 @@ void Downstream::normalize_response_headers() normalize_headers(response_headers_); } +void Downstream::concat_norm_response_headers() +{ + response_headers_ = http2::concat_norm_headers(std::move(response_headers_)); +} + Headers::const_iterator Downstream::get_norm_response_header (const std::string& name) const { diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index bcba617a..25eb6f68 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -140,6 +140,10 @@ public: const Headers& get_response_headers() const; // Makes key lowercase and sort headers by name using < void normalize_response_headers(); + // Concatenates response header fields with same name by NULL as + // delimiter. See http2::concat_norm_headers(). This function must + // be called after calling normalize_response_headers(). + void concat_norm_response_headers(); // Returns iterator pointing to the response header with the name // |name|. If multiple header have |name| as name, return first // occurrence from the beginning. If no such header is found, diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 28a619a2..8dae7175 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -232,7 +232,9 @@ int Http2DownstreamConnection::push_request_headers() downstream_->crumble_request_cookie(); } downstream_->normalize_request_headers(); + downstream_->concat_norm_response_headers(); auto end_headers = std::end(downstream_->get_request_headers()); + // 6 means: // 1. :method // 2. :scheme @@ -330,12 +332,6 @@ int Http2DownstreamConnection::push_request_headers() content_length = true; } - auto expect = downstream_->get_norm_request_header("expect"); - if(expect != end_headers && - !util::strifind((*expect).second.c_str(), "100-continue")) { - nva.push_back(MAKE_NV_LS("expect", (*expect).second)); - } - bool chunked_encoding = false; auto transfer_encoding = downstream_->get_norm_request_header("transfer-encoding"); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index e8e4e09c..c96a600c 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -943,6 +943,7 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) DLOG(INFO, downstream) << "HTTP response header completed"; } downstream->normalize_response_headers(); + downstream->concat_norm_response_headers(); auto end_headers = std::end(downstream->get_response_headers()); size_t nheader = downstream->get_response_headers().size(); auto nva = std::vector(); diff --git a/src/util.cc b/src/util.cc index 0cd97af1..fc67ef0c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -152,6 +152,7 @@ bool endsWith(const std::string& a, const std::string& b) { return endsWith(a.begin(), a.end(), b.begin(), b.end()); } + bool strieq(const char *a, const char *b) { if(!a || !b) {