From 67569486d1be3d2b54f393416de89b7a7e2dad97 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 12 Mar 2016 18:36:05 +0900 Subject: [PATCH] src: Rewrite http:create_via_header_value --- src/shrpx-unittest.cc | 2 ++ src/shrpx_http.cc | 11 ----------- src/shrpx_http.h | 12 +++++++++++- src/shrpx_http2_downstream_connection.cc | 21 ++++++++++++++++----- src/shrpx_http2_upstream.cc | 21 +++++++++++++++------ src/shrpx_http_downstream_connection.cc | 5 ++++- src/shrpx_http_test.cc | 14 ++++++++++++++ src/shrpx_http_test.h | 1 + src/shrpx_https_upstream.cc | 6 ++++-- src/shrpx_spdy_upstream.cc | 6 ++++-- 10 files changed, 71 insertions(+), 28 deletions(-) diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 725838f8..6a015778 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -123,6 +123,8 @@ int main(int argc, char *argv[]) { shrpx::test_shrpx_worker_match_downstream_addr_group) || !CU_add_test(pSuite, "http_create_forwarded", shrpx::test_shrpx_http_create_forwarded) || + !CU_add_test(pSuite, "http_create_via_header_value", + shrpx::test_shrpx_http_create_via_header_value) || !CU_add_test(pSuite, "util_streq", shrpx::test_util_streq) || !CU_add_test(pSuite, "util_strieq", shrpx::test_util_strieq) || !CU_add_test(pSuite, "util_inp_strlower", diff --git a/src/shrpx_http.cc b/src/shrpx_http.cc index 7bca56dd..071f2841 100644 --- a/src/shrpx_http.cc +++ b/src/shrpx_http.cc @@ -50,17 +50,6 @@ std::string create_error_html(unsigned int status_code) { return res; } -std::string create_via_header_value(int major, int minor) { - std::string hdrs; - hdrs += static_cast(major + '0'); - if (major < 2) { - hdrs += '.'; - hdrs += static_cast(minor + '0'); - } - hdrs += " nghttpx"; - return hdrs; -} - std::string create_forwarded(int params, const StringRef &node_by, const StringRef &node_for, const StringRef &host, const StringRef &proto) { diff --git a/src/shrpx_http.h b/src/shrpx_http.h index 56705ecd..35275baf 100644 --- a/src/shrpx_http.h +++ b/src/shrpx_http.h @@ -31,13 +31,23 @@ #include +#include "util.h" + namespace shrpx { namespace http { std::string create_error_html(unsigned int status_code); -std::string create_via_header_value(int major, int minor); +template +OutputIt create_via_header_value(OutputIt dst, int major, int minor) { + *dst++ = static_cast(major + '0'); + if (major < 2) { + *dst++ = '.'; + *dst++ = static_cast(minor + '0'); + } + return util::copy_lit(dst, " nghttpx"); +} // Returns generated RFC 7239 Forwarded header field value. The // |params| is bitwise-OR of zero or more of shrpx_forwarded_param diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 5e2d5129..2e5fb177 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -266,6 +266,8 @@ int Http2DownstreamConnection::push_request_headers() { const auto &req = downstream_->request(); + auto &balloc = downstream_->get_block_allocator(); + auto &httpconf = get_config()->http; auto &http2conf = get_config()->http2; @@ -403,19 +405,28 @@ int Http2DownstreamConnection::push_request_headers() { nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", req.scheme)); } - std::string via_value; auto via = req.fs.header(http2::HD_VIA); if (httpconf.no_via) { if (via) { nva.push_back(http2::make_nv_ls_nocopy("via", (*via).value)); } } else { + size_t vialen = 16; if (via) { - via_value = (*via).value.str(); - via_value += ", "; + vialen += via->value.size() + 2; } - via_value += http::create_via_header_value(req.http_major, req.http_minor); - nva.push_back(http2::make_nv_ls("via", via_value)); + + auto iov = make_byte_ref(balloc, vialen + 1); + auto p = iov.base; + + if (via) { + p = std::copy(std::begin(via->value), std::end(via->value), p); + p = util::copy_lit(p, ", "); + } + p = http::create_via_header_value(p, req.http_major, req.http_minor); + *p = '\0'; + + nva.push_back(http2::make_nv_ls_nocopy("via", StringRef{iov.base, p})); } auto te = req.fs.header(http2::HD_TE); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index e54b8e90..32905876 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1399,7 +1399,6 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { // field. nva.reserve(resp.fs.headers().size() + 4 + httpconf.add_response_headers.size()); - std::string via_value; auto response_status = http2::stringify_status(resp.http_status); if (response_status.empty()) { @@ -1453,13 +1452,23 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { nva.push_back(http2::make_nv_ls_nocopy("via", (*via).value)); } } else { + // we don't create more than 16 bytes in + // http::create_via_header_value. + size_t len = 16; if (via) { - via_value = (*via).value.str(); - via_value += ", "; + len += via->value.size() + 2; } - via_value += - http::create_via_header_value(resp.http_major, resp.http_minor); - nva.push_back(http2::make_nv_ls("via", via_value)); + + auto iov = make_byte_ref(balloc, len + 1); + auto p = iov.base; + if (via) { + p = std::copy(std::begin(via->value), std::end(via->value), p); + p = util::copy_lit(p, ", "); + } + p = http::create_via_header_value(p, resp.http_major, resp.http_minor); + *p = '\0'; + + nva.push_back(http2::make_nv_ls_nocopy("via", StringRef{iov.base, p})); } for (auto &p : httpconf.add_response_headers) { diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 5f4f9468..490fefe8 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -424,7 +424,10 @@ int HttpDownstreamConnection::push_request_headers() { buf->append((*via).value); buf->append(", "); } - buf->append(http::create_via_header_value(req.http_major, req.http_minor)); + std::array viabuf; + auto end = http::create_via_header_value(viabuf.data(), req.http_major, + req.http_minor); + buf->append(viabuf.data(), end - viabuf.data()); buf->append("\r\n"); } diff --git a/src/shrpx_http_test.cc b/src/shrpx_http_test.cc index 87fbb9c5..bee8d966 100644 --- a/src/shrpx_http_test.cc +++ b/src/shrpx_http_test.cc @@ -72,4 +72,18 @@ void test_shrpx_http_create_forwarded(void) { StringRef::from_lit(""), StringRef::from_lit(""))); } +void test_shrpx_http_create_via_header_value(void) { + std::array buf; + + auto end = http::create_via_header_value(std::begin(buf), 1, 1); + + CU_ASSERT(("1.1 nghttpx" == StringRef{std::begin(buf), end})); + + std::fill(std::begin(buf), std::end(buf), '\0'); + + end = http::create_via_header_value(std::begin(buf), 2, 0); + + CU_ASSERT(("2 nghttpx" == StringRef{std::begin(buf), end})); +} + } // namespace shrpx diff --git a/src/shrpx_http_test.h b/src/shrpx_http_test.h index d8e28a9f..7692c95a 100644 --- a/src/shrpx_http_test.h +++ b/src/shrpx_http_test.h @@ -32,6 +32,7 @@ namespace shrpx { void test_shrpx_http_create_forwarded(void); +void test_shrpx_http_create_via_header_value(void); } // namespace shrpx diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index dd028190..ac99c457 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -1074,8 +1074,10 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) { buf->append((*via).value); buf->append(", "); } - buf->append( - http::create_via_header_value(resp.http_major, resp.http_minor)); + std::array viabuf; + auto end = http::create_via_header_value(viabuf.data(), resp.http_major, + resp.http_minor); + buf->append(viabuf.data(), end - std::begin(viabuf)); buf->append("\r\n"); } diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 4b48c93f..21380afa 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -1075,8 +1075,10 @@ int SpdyUpstream::on_downstream_header_complete(Downstream *downstream) { via_value = via->value.str(); via_value += ", "; } - via_value += - http::create_via_header_value(resp.http_major, resp.http_minor); + std::array viabuf; + auto end = http::create_via_header_value(std::begin(viabuf), + resp.http_major, resp.http_minor); + via_value.append(std::begin(viabuf), end); nv[hdidx++] = "via"; nv[hdidx++] = via_value.c_str(); }