From e596385fc0558a2496c1a2b66f7d9a5d412618d3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 5 Dec 2013 21:54:36 +0900 Subject: [PATCH] src: Split NULL-separated values --- src/HttpServer.cc | 8 ++--- src/app_helper.cc | 8 ++--- src/http2.cc | 62 ++++++++++++++++++++++--------------- src/http2.h | 15 +++++---- src/http2_test.cc | 51 ++++++++++++------------------ src/nghttp.cc | 15 ++++----- src/shrpx_http2_session.cc | 14 ++++----- src/shrpx_http2_upstream.cc | 16 +++++----- 8 files changed, 97 insertions(+), 92 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index cc88e7f6..1ff07a91 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -707,12 +707,12 @@ void prepare_response(Request *req, Http2Handler *hd) } // namespace namespace { -void append_nv(Request *req, const std::vector& nva) +void append_nv(Request *req, const std::vector& nva) { - for(auto nv : nva) { + for(auto& nv : nva) { req->headers.push_back(std::make_pair - (std::string(nv->name, nv->name + nv->namelen), - std::string(nv->value, nv->value + nv->valuelen))); + (std::string(nv.name, nv.name + nv.namelen), + std::string(nv.value, nv.value + nv.valuelen))); } } } // namespace diff --git a/src/app_helper.cc b/src/app_helper.cc index daf09adc..859c5a7c 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -45,6 +45,7 @@ #include "app_helper.h" #include "util.h" +#include "http2.h" namespace nghttp2 { @@ -163,13 +164,12 @@ const char* ansi_escend() void print_nv(nghttp2_nv *nva, size_t nvlen) { - size_t i; - for(i = 0; i < nvlen; ++i) { + for(auto& nv : http2::sort_nva(nva, nvlen)) { print_frame_attr_indent(); printf("%s", ansi_esc("\033[1;34m")); - fwrite(nva[i].name, nva[i].namelen, 1, stdout); + fwrite(nv.name, nv.namelen, 1, stdout); printf("%s: ", ansi_escend()); - fwrite(nva[i].value, nva[i].valuelen, 1, stdout); + fwrite(nv.value, nv.valuelen, 1, stdout); printf("\n"); } } diff --git a/src/http2.cc b/src/http2.cc index 107fdebe..2886af95 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -196,69 +196,81 @@ size_t HTTP1_IGN_HDLEN = sizeof(HTTP1_IGN_HD)/sizeof(HTTP1_IGN_HD[0]); } // namespace namespace { -auto nv_name_less = [](const nghttp2_nv *lhs, const nghttp2_nv *rhs) +auto nv_name_less = [](const nghttp2_nv& lhs, const nghttp2_nv& rhs) { - return nghttp2_nv_compare_name(lhs, rhs) < 0; + return nghttp2_nv_compare_name(&lhs, &rhs) < 0; }; } // namespace -bool check_http2_headers(const std::vector& nva) +bool check_http2_headers(const std::vector& nva) { for(size_t i = 0; i < DISALLOWED_HDLEN; ++i) { nghttp2_nv nv = {(uint8_t*)DISALLOWED_HD[i], nullptr, (uint16_t)strlen(DISALLOWED_HD[i]), 0}; - if(std::binary_search(std::begin(nva), std::end(nva), &nv, nv_name_less)) { + if(std::binary_search(std::begin(nva), std::end(nva), nv, nv_name_less)) { return false; } } return true; } -std::vector sort_nva(const nghttp2_nv *nva, size_t nvlen) +std::vector sort_nva(const nghttp2_nv *nva, size_t nvlen) { - auto res = std::vector(); + auto v = std::vector(&nva[0], &nva[nvlen]); + std::sort(std::begin(v), std::end(v), nv_name_less); + auto res = std::vector(); res.reserve(nvlen); for(size_t i = 0; i < nvlen; ++i) { - res.push_back(&nva[i]); + if(v[i].valuelen == 0) { + res.push_back(v[i]); + continue; + } + auto j = v[i].value; + auto end = v[i].value + v[i].valuelen; + for(;;) { + // Skip 0 length value + j = std::find_if_not(j, end, + [](uint8_t c) + { + return c == '\0'; + }); + if(j == end) { + break; + } + auto l = std::find(j, end, '\0'); + res.push_back({v[i].name, j, v[i].namelen, static_cast(l-j)}); + j = l; + } } - std::sort(std::begin(res), std::end(res), - [](const nghttp2_nv *lhs, const nghttp2_nv *rhs) - { - auto rv = nghttp2_nv_compare_name(lhs, rhs); - if(rv == 0) { - return lhs < rhs; - } - return rv < 0; - }); return res; } -const nghttp2_nv* get_unique_header(const std::vector& nva, +const nghttp2_nv* get_unique_header(const std::vector& nva, const char *name) { size_t namelen = strlen(name); nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0}; - auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less); - if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen, + auto i = std::lower_bound(std::begin(nva), std::end(nva), nv, nv_name_less); + if(i != std::end(nva) && util::streq((*i).name, (*i).namelen, (const uint8_t*)name, namelen)) { auto j = i + 1; - if(j == std::end(nva) || !util::streq((*j)->name, (*j)->namelen, + if(j == std::end(nva) || !util::streq((*j).name, (*j).namelen, (const uint8_t*)name, namelen)) { - return *i; + return &(*i); } } return nullptr; } -const nghttp2_nv* get_header(const std::vector& nva, +const nghttp2_nv* get_header(const std::vector& nva, const char *name) { size_t namelen = strlen(name); nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0}; - auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less); - if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen, + auto i = std::lower_bound(std::begin(nva), std::end(nva), nv, nv_name_less); + if(i != std::end(nva) && util::streq((*i).name, (*i).namelen, (const uint8_t*)name, namelen)) { - return *i; + return &(*i); } return nullptr; } diff --git a/src/http2.h b/src/http2.h index 484a1287..5a85bb4c 100644 --- a/src/http2.h +++ b/src/http2.h @@ -80,22 +80,25 @@ bool check_http2_allowed_header(const char *name); // Checks that headers |nva| including |nvlen| entries do not contain // disallowed header fields in HTTP/2.0 spec. This function returns // true if |nva| does not contains such headers. -bool check_http2_headers(const std::vector& nva); +bool check_http2_headers(const std::vector& nva); -// Returns sorted |nva| with |nvlen| elements. This sort is stable -// sort. -std::vector sort_nva(const nghttp2_nv *nva, size_t nvlen); +// Returns sorted |nva| with |nvlen| elements. The headers are sorted +// by name only and not necessarily stable. In addition to the +// sorting, this function splits values concatenated with NULL. The +// ordering of the concatenated values are preserved. The element of +// the returned vector refers to the memory pointed by |nva|. +std::vector sort_nva(const nghttp2_nv *nva, size_t nvlen); // Returns the pointer to the entry in |nva| which has name |name| and // the |name| is uinque in the |nva|. If no such entry exist, returns // nullptr. -const nghttp2_nv* get_unique_header(const std::vector& nva, +const nghttp2_nv* get_unique_header(const std::vector& nva, const char *name); // Returns the poiter to the entry in |nva| which has name |name|. If // more than one entries which have the name |name|, first occurrence // in |nva| is returned. If no such entry exist, returns nullptr. -const nghttp2_nv* get_header(const std::vector& nva, +const nghttp2_nv* get_header(const std::vector& nva, const char *name); // Returns std::string version of nv->name with nv->namelen bytes. diff --git a/src/http2_test.cc b/src/http2_test.cc index 964994d9..d5ab3523 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -24,6 +24,7 @@ */ #include "http2_test.h" +#include #include #include @@ -35,39 +36,38 @@ using namespace nghttp2; #define MAKE_NV(K, V) {(uint8_t*)K, (uint8_t*)V, \ - (uint16_t)strlen(K), (uint16_t)strlen(V)} + (uint16_t)(sizeof(K)-1), (uint16_t)(sizeof(V)-1)} namespace shrpx { namespace { -template -int bstrcmp(cstr1 *a, cstr2 *b) +void check_nv(const std::pair& a, + const nghttp2_nv *b) { - return strcmp((const char*)a, (const char*)b); + CU_ASSERT(a.first.size() == b->namelen); + CU_ASSERT(a.second.size() == b->valuelen); + CU_ASSERT(memcmp(a.first.c_str(), b->name, b->namelen) == 0); + CU_ASSERT(memcmp(a.second.c_str(), b->value, b->valuelen) == 0); } } // namespace void test_http2_sort_nva(void) { + // Last 0 is stripped in MAKE_NV + const uint8_t concatval[] = { '4', 0x00, 0x00, '6', 0x00, '5', 0x00 }; nghttp2_nv nv[] = {MAKE_NV("alpha", "1"), - MAKE_NV("bravo", "9"), - MAKE_NV("bravo", "8"), - MAKE_NV("charlie", "5"), - MAKE_NV("bravo", "3"), - MAKE_NV("bravo", "4")}; + MAKE_NV("charlie", "3"), + MAKE_NV("bravo", "2"), + MAKE_NV("delta", concatval)}; auto nvlen = sizeof(nv)/sizeof(nv[0]); auto nva = http2::sort_nva(nv, nvlen); - CU_ASSERT(nvlen == nva.size()); - CU_ASSERT(0 == bstrcmp("alpha", nva[0]->name)); - CU_ASSERT(0 == bstrcmp("bravo", nva[1]->name)); - CU_ASSERT(0 == bstrcmp("9", nva[1]->value)); - CU_ASSERT(0 == bstrcmp("bravo", nva[2]->name)); - CU_ASSERT(0 == bstrcmp("8", nva[2]->value)); - CU_ASSERT(0 == bstrcmp("bravo", nva[3]->name)); - CU_ASSERT(0 == bstrcmp("3", nva[3]->value)); - CU_ASSERT(0 == bstrcmp("bravo", nva[4]->name)); - CU_ASSERT(0 == bstrcmp("4", nva[4]->value)); - CU_ASSERT(0 == bstrcmp("charlie", nva[5]->name)); + CU_ASSERT(6 == nva.size()); + check_nv({"alpha", "1"}, &nva[0]); + check_nv({"bravo", "2"}, &nva[1]); + check_nv({"charlie", "3"}, &nva[2]); + check_nv({"delta", "4"}, &nva[3]); + check_nv({"delta", "6"}, &nva[4]); + check_nv({"delta", "5"}, &nva[5]); } void test_http2_check_http2_headers(void) @@ -164,17 +164,6 @@ auto headers = std::vector> {"zulu", "12"}}; } // namespace -namespace { -void check_nv(const std::pair& a, - const nghttp2_nv *b) -{ - CU_ASSERT(a.first.size() == b->namelen); - CU_ASSERT(a.second.size() == b->valuelen); - CU_ASSERT(memcmp(a.first.c_str(), b->name, b->namelen) == 0); - CU_ASSERT(memcmp(a.second.c_str(), b->value, b->valuelen) == 0); -} -} // namespace - void test_http2_copy_norm_headers_to_nva(void) { std::vector nva; diff --git a/src/nghttp.cc b/src/nghttp.cc index 7461614e..32135d2a 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -68,6 +68,7 @@ #include "HtmlParser.h" #include "util.h" #include "base64.h" +#include "http2.h" #ifndef O_BINARY # define O_BINARY (0) @@ -1125,14 +1126,14 @@ void check_response_header // Server-pushed stream does not have stream user data return; } + auto nva = http2::sort_nva(frame->headers.nva, frame->headers.nvlen); bool gzip = false; - for(size_t i = 0; i < frame->headers.nvlen; ++i) { - auto nv = &frame->headers.nva[i]; - if(util::strieq("content-encoding", nv->name, nv->namelen)) { - gzip = util::strieq("gzip", nv->value, nv->valuelen) || - util::strieq("deflate", nv->value, nv->valuelen); - } else if(util::strieq(":status", nv->name, nv->namelen)) { - req->status.assign(nv->value, nv->value + nv->valuelen); + for(auto& nv : nva) { + if(util::strieq("content-encoding", nv.name, nv.namelen)) { + gzip = util::strieq("gzip", nv.value, nv.valuelen) || + util::strieq("deflate", nv.value, nv.valuelen); + } else if(util::strieq(":status", nv.name, nv.namelen)) { + req->status.assign(nv.value, nv.value + nv.valuelen); } } if(gzip) { diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index d3fb9f64..e3b88140 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -832,10 +832,10 @@ int on_frame_recv_callback return 0; } - for(auto nv : nva) { - if(nv->namelen > 0 && nv->name[0] != ':') { - downstream->add_response_header(http2::name_to_str(nv), - http2::value_to_str(nv)); + for(auto& nv : nva) { + if(nv.namelen > 0 && nv.name[0] != ':') { + downstream->add_response_header(http2::name_to_str(&nv), + http2::value_to_str(&nv)); } } @@ -879,11 +879,11 @@ int on_frame_recv_callback if(LOG_ENABLED(INFO)) { std::stringstream ss; - for(auto nv : nva) { + for(auto& nv : nva) { ss << TTY_HTTP_HD; - ss.write(reinterpret_cast(nv->name), nv->namelen); + ss.write(reinterpret_cast(nv.name), nv.namelen); ss << TTY_RST << ": "; - ss.write(reinterpret_cast(nv->value), nv->valuelen); + ss.write(reinterpret_cast(nv.value), nv.valuelen); ss << "\n"; } SSLOG(INFO, http2session) << "HTTP response headers. stream_id=" diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index abf07283..e8e4e09c 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -242,11 +242,11 @@ int on_frame_recv_callback if(LOG_ENABLED(INFO)) { std::stringstream ss; - for(auto nv : nva) { + for(auto& nv : nva) { ss << TTY_HTTP_HD; - ss.write(reinterpret_cast(nv->name), nv->namelen); + ss.write(reinterpret_cast(nv.name), nv.namelen); ss << TTY_RST << ": "; - ss.write(reinterpret_cast(nv->value), nv->valuelen); + ss.write(reinterpret_cast(nv.value), nv.valuelen); ss << "\n"; } ULOG(INFO, upstream) << "HTTP request headers. stream_id=" @@ -256,7 +256,7 @@ int on_frame_recv_callback if(get_config()->http2_upstream_dump_request_header) { http2::dump_nv(get_config()->http2_upstream_dump_request_header, - frame->headers.nva, frame->headers.nvlen); + nva.data(), nva.size()); } if(!http2::check_http2_headers(nva)) { @@ -264,10 +264,10 @@ int on_frame_recv_callback return 0; } - for(auto nv : nva) { - if(nv->namelen > 0 && nv->name[0] != ':') { - downstream->add_request_header(http2::name_to_str(nv), - http2::value_to_str(nv)); + for(auto& nv : nva) { + if(nv.namelen > 0 && nv.name[0] != ':') { + downstream->add_request_header(http2::name_to_str(&nv), + http2::value_to_str(&nv)); } }