diff --git a/src/shrpx_http.cc b/src/shrpx_http.cc index 071f2841..c6a70e06 100644 --- a/src/shrpx_http.cc +++ b/src/shrpx_http.cc @@ -50,58 +50,75 @@ std::string create_error_html(unsigned int status_code) { return res; } -std::string create_forwarded(int params, const StringRef &node_by, - const StringRef &node_for, const StringRef &host, - const StringRef &proto) { - std::string res; +StringRef create_forwarded(BlockAllocator &balloc, int params, + const StringRef &node_by, const StringRef &node_for, + const StringRef &host, const StringRef &proto) { + size_t len = 0; + if ((params & FORWARDED_BY) && !node_by.empty()) { + len += str_size("by=\"") + node_by.size() + str_size("\";"); + } + if ((params & FORWARDED_FOR) && !node_for.empty()) { + len += str_size("for=\"") + node_for.size() + str_size("\";"); + } + if ((params & FORWARDED_HOST) && !host.empty()) { + len += str_size("host=\"") + host.size() + str_size("\";"); + } + if ((params & FORWARDED_PROTO) && !proto.empty()) { + len += str_size("proto=") + proto.size() + str_size(";"); + } + + auto iov = make_byte_ref(balloc, len + 1); + auto p = iov.base; + if ((params & FORWARDED_BY) && !node_by.empty()) { // This must be quoted-string unless it is obfuscated version // (which starts with "_") or some special value (e.g., // "localhost" for UNIX domain socket), since ':' is not allowed // in token. ':' is used to separate host and port. if (node_by[0] == '_' || node_by[0] == 'l') { - res += "by="; - res += node_by; - res += ";"; + p = util::copy_lit(p, "by="); + p = std::copy(std::begin(node_by), std::end(node_by), p); + p = util::copy_lit(p, ";"); } else { - res += "by=\""; - res += node_by; - res += "\";"; + p = util::copy_lit(p, "by=\""); + p = std::copy(std::begin(node_by), std::end(node_by), p); + p = util::copy_lit(p, "\";"); } } if ((params & FORWARDED_FOR) && !node_for.empty()) { // We only quote IPv6 literal address only, which starts with '['. if (node_for[0] == '[') { - res += "for=\""; - res += node_for; - res += "\";"; + p = util::copy_lit(p, "for=\""); + p = std::copy(std::begin(node_for), std::end(node_for), p); + p = util::copy_lit(p, "\";"); } else { - res += "for="; - res += node_for; - res += ";"; + p = util::copy_lit(p, "for="); + p = std::copy(std::begin(node_for), std::end(node_for), p); + p = util::copy_lit(p, ";"); } } if ((params & FORWARDED_HOST) && !host.empty()) { // Just be quoted to skip checking characters. - res += "host=\""; - res += host; - res += "\";"; + p = util::copy_lit(p, "host=\""); + p = std::copy(std::begin(host), std::end(host), p); + p = util::copy_lit(p, "\";"); } if ((params & FORWARDED_PROTO) && !proto.empty()) { // Scheme production rule only allow characters which are all in // token. - res += "proto="; - res += proto; - res += ";"; + p = util::copy_lit(p, "proto="); + p = std::copy(std::begin(proto), std::end(proto), p); + *p++ = ';'; } - if (res.empty()) { - return res; + if (iov.base == p) { + return StringRef{}; } - res.erase(res.size() - 1); + --p; + *p = '\0'; - return res; + return StringRef{iov.base, p}; } std::string colorizeHeaders(const char *hdrs) { diff --git a/src/shrpx_http.h b/src/shrpx_http.h index 35275baf..856f192a 100644 --- a/src/shrpx_http.h +++ b/src/shrpx_http.h @@ -32,6 +32,7 @@ #include #include "util.h" +#include "allocator.h" namespace shrpx { @@ -52,9 +53,9 @@ OutputIt create_via_header_value(OutputIt dst, int major, int minor) { // Returns generated RFC 7239 Forwarded header field value. The // |params| is bitwise-OR of zero or more of shrpx_forwarded_param // defined in shrpx_config.h. -std::string create_forwarded(int params, const StringRef &node_by, - const StringRef &node_for, const StringRef &host, - const StringRef &proto); +StringRef create_forwarded(BlockAllocator &balloc, int params, + const StringRef &node_by, const StringRef &node_for, + const StringRef &host, const StringRef &proto); // Adds ANSI color codes to HTTP headers |hdrs|. std::string colorizeHeaders(const char *hdrs); diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index c9df0369..73a43a8d 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -347,8 +347,6 @@ int Http2DownstreamConnection::push_request_headers() { auto upstream = downstream_->get_upstream(); auto handler = upstream->get_client_handler(); - std::string forwarded_value; - auto &fwdconf = httpconf.forwarded; auto fwd = @@ -361,25 +359,24 @@ int Http2DownstreamConnection::push_request_headers() { params &= ~FORWARDED_PROTO; } - auto value = http::create_forwarded(params, handler->get_forwarded_by(), - handler->get_forwarded_for(), - req.authority, req.scheme); + auto value = http::create_forwarded( + balloc, params, handler->get_forwarded_by(), + handler->get_forwarded_for(), req.authority, req.scheme); + if (fwd || !value.empty()) { if (fwd) { - forwarded_value = fwd->value.str(); - - if (!value.empty()) { - forwarded_value += ", "; + if (value.empty()) { + value = fwd->value; + } else { + value = concat_string_ref(balloc, fwd->value, + StringRef::from_lit(", "), value); } } - forwarded_value += value; - - nva.push_back(http2::make_nv_ls("forwarded", forwarded_value)); + nva.push_back(http2::make_nv_ls_nocopy("forwarded", value)); } } else if (fwd) { nva.push_back(http2::make_nv_ls_nocopy("forwarded", fwd->value)); - forwarded_value = fwd->value.str(); } auto &xffconf = httpconf.xff; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 490fefe8..893c9efc 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -272,6 +272,8 @@ int HttpDownstreamConnection::push_request_headers() { const auto &downstream_hostport = addr_->hostport; const auto &req = downstream_->request(); + auto &balloc = downstream_->get_block_allocator(); + auto connect_method = req.method == HTTP_CONNECT; auto &httpconf = get_config()->http; @@ -366,9 +368,10 @@ int HttpDownstreamConnection::push_request_headers() { params &= ~FORWARDED_PROTO; } - auto value = http::create_forwarded(params, handler->get_forwarded_by(), - handler->get_forwarded_for(), - req.authority, req.scheme); + auto value = http::create_forwarded( + balloc, params, handler->get_forwarded_by(), + handler->get_forwarded_for(), req.authority, req.scheme); + if (fwd || !value.empty()) { buf->append("Forwarded: "); if (fwd) { diff --git a/src/shrpx_http_test.cc b/src/shrpx_http_test.cc index bee8d966..455397a6 100644 --- a/src/shrpx_http_test.cc +++ b/src/shrpx_http_test.cc @@ -38,38 +38,40 @@ namespace shrpx { void test_shrpx_http_create_forwarded(void) { + BlockAllocator balloc(1024, 1024); + CU_ASSERT("by=\"example.com:3000\";for=\"[::1]\";host=\"www.example.com\";" "proto=https" == - http::create_forwarded(FORWARDED_BY | FORWARDED_FOR | - FORWARDED_HOST | FORWARDED_PROTO, + http::create_forwarded(balloc, FORWARDED_BY | FORWARDED_FOR | + FORWARDED_HOST | FORWARDED_PROTO, StringRef::from_lit("example.com:3000"), StringRef::from_lit("[::1]"), StringRef::from_lit("www.example.com"), StringRef::from_lit("https"))); CU_ASSERT("for=192.168.0.1" == - http::create_forwarded(FORWARDED_FOR, StringRef::from_lit("alpha"), - StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("bravo"), - StringRef::from_lit("charlie"))); + http::create_forwarded( + balloc, FORWARDED_FOR, StringRef::from_lit("alpha"), + StringRef::from_lit("192.168.0.1"), + StringRef::from_lit("bravo"), StringRef::from_lit("charlie"))); CU_ASSERT("by=_hidden;for=\"[::1]\"" == http::create_forwarded( - FORWARDED_BY | FORWARDED_FOR, StringRef::from_lit("_hidden"), - StringRef::from_lit("[::1]"), StringRef::from_lit(""), - StringRef::from_lit(""))); + balloc, FORWARDED_BY | FORWARDED_FOR, + StringRef::from_lit("_hidden"), StringRef::from_lit("[::1]"), + StringRef::from_lit(""), StringRef::from_lit(""))); CU_ASSERT("by=\"[::1]\";for=_hidden" == http::create_forwarded( - FORWARDED_BY | FORWARDED_FOR, StringRef::from_lit("[::1]"), - StringRef::from_lit("_hidden"), StringRef::from_lit(""), - StringRef::from_lit(""))); - - CU_ASSERT("" == - http::create_forwarded( - FORWARDED_BY | FORWARDED_FOR | FORWARDED_HOST | FORWARDED_PROTO, - StringRef::from_lit(""), StringRef::from_lit(""), + balloc, FORWARDED_BY | FORWARDED_FOR, + StringRef::from_lit("[::1]"), StringRef::from_lit("_hidden"), StringRef::from_lit(""), StringRef::from_lit(""))); + + CU_ASSERT("" == http::create_forwarded( + balloc, FORWARDED_BY | FORWARDED_FOR | FORWARDED_HOST | + FORWARDED_PROTO, + StringRef::from_lit(""), StringRef::from_lit(""), + StringRef::from_lit(""), StringRef::from_lit(""))); } void test_shrpx_http_create_via_header_value(void) {