diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index f9acf880..a02fd2e5 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -176,6 +176,8 @@ int main(int argc, char *argv[]) { !CU_add_test(pSuite, "util_make_hostport", shrpx::test_util_make_hostport) || !CU_add_test(pSuite, "util_strifind", shrpx::test_util_strifind) || + !CU_add_test(pSuite, "util_random_alpha_digit", + shrpx::test_util_random_alpha_digit) || !CU_add_test(pSuite, "gzip_inflate", test_nghttp2_gzip_inflate) || !CU_add_test(pSuite, "buffer_write", nghttp2::test_buffer_write) || !CU_add_test(pSuite, "pool_recycle", nghttp2::test_pool_recycle) || diff --git a/src/shrpx.cc b/src/shrpx.cc index 9d601807..b8c67ccf 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2569,8 +2569,10 @@ int process_options(Config *config, fwdconf.by_obfuscated.empty()) { std::mt19937 gen(rd()); auto &dst = fwdconf.by_obfuscated; - dst = "_"; - dst += util::random_alpha_digit(gen, SHRPX_OBFUSCATED_NODE_LENGTH); + dst.resize(1 + SHRPX_OBFUSCATED_NODE_LENGTH); + auto p = std::begin(dst); + *p++ = '_'; + util::random_alpha_digit(p, std::end(dst), gen); } if (config->http2.upstream.debug.frame_debug) { diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index aa7a8117..43c28649 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -376,17 +376,18 @@ int ClientHandler::upstream_http1_connhd_read() { } ClientHandler::ClientHandler(Worker *worker, int fd, SSL *ssl, - const char *ipaddr, const char *port, int family, - const UpstreamAddr *faddr) - : conn_(worker->get_loop(), fd, ssl, worker->get_mcpool(), + const StringRef &ipaddr, const StringRef &port, + int family, const UpstreamAddr *faddr) + : balloc_(128, 128), + conn_(worker->get_loop(), fd, ssl, worker->get_mcpool(), get_config()->conn.upstream.timeout.write, get_config()->conn.upstream.timeout.read, get_config()->conn.upstream.ratelimit.write, get_config()->conn.upstream.ratelimit.read, writecb, readcb, timeoutcb, this, get_config()->tls.dyn_rec.warmup_threshold, get_config()->tls.dyn_rec.idle_timeout, PROTO_NONE), - ipaddr_(ipaddr), - port_(port), + ipaddr_(make_string_ref(balloc_, ipaddr)), + port_(make_string_ref(balloc_, port)), faddr_(faddr), worker_(worker), left_connhd_len_(NGHTTP2_CLIENT_MAGIC_LEN), @@ -416,13 +417,28 @@ ClientHandler::ClientHandler(Worker *worker, int fd, SSL *ssl, if (fwdconf.params & FORWARDED_FOR) { if (fwdconf.for_node_type == FORWARDED_NODE_OBFUSCATED) { - forwarded_for_ = "_"; - forwarded_for_ += util::random_alpha_digit(worker_->get_randgen(), - SHRPX_OBFUSCATED_NODE_LENGTH); + // 1 for '_' + auto len = SHRPX_OBFUSCATED_NODE_LENGTH + 1; + // 1 for terminating NUL. + auto buf = make_byte_ref(balloc_, len + 1); + auto p = buf.base; + *p++ = '_'; + p = util::random_alpha_digit(p, p + len - 1, worker_->get_randgen()); + *p = '\0'; + + forwarded_for_ = StringRef{buf.base, p}; } else if (family == AF_INET6) { - forwarded_for_ = "["; - forwarded_for_ += ipaddr_; - forwarded_for_ += ']'; + // 2 for '[' and ']' + auto len = 2 + ipaddr_.size(); + // 1 for terminating NUL. + auto buf = make_byte_ref(balloc_, len + 1); + auto p = buf.base; + *p++ = '['; + p = std::copy(std::begin(ipaddr_), std::end(ipaddr_), p); + *p++ = ']'; + *p = '\0'; + + forwarded_for_ = StringRef{buf.base, p}; } else { // family == AF_INET or family == AF_UNIX forwarded_for_ = ipaddr_; @@ -441,7 +457,7 @@ void ClientHandler::setup_upstream_io_callback() { // upgraded to HTTP/2 through HTTP Upgrade or direct HTTP/2 // connection. upstream_ = make_unique(this); - alpn_ = "http/1.1"; + alpn_ = StringRef::from_lit("http/1.1"); read_ = &ClientHandler::read_clear; write_ = &ClientHandler::write_clear; on_read_ = &ClientHandler::upstream_http1_connhd_read; @@ -524,7 +540,7 @@ int ClientHandler::validate_next_proto() { } upstream_ = make_unique(this); - alpn_ = "http/1.1"; + alpn_ = StringRef::from_lit("http/1.1"); // At this point, input buffer is already filled with some bytes. // The read callback is not called until new data come. So consume @@ -555,7 +571,7 @@ int ClientHandler::validate_next_proto() { auto http2_upstream = make_unique(this); upstream_ = std::move(http2_upstream); - alpn_.assign(std::begin(proto), std::end(proto)); + alpn_ = make_string_ref(balloc_, proto); // At this point, input buffer is already filled with some bytes. // The read callback is not called until new data come. So consume @@ -574,16 +590,16 @@ int ClientHandler::validate_next_proto() { switch (spdy_version) { case SPDYLAY_PROTO_SPDY2: - alpn_ = "spdy/2"; + alpn_ = StringRef::from_lit("spdy/2"); break; case SPDYLAY_PROTO_SPDY3: - alpn_ = "spdy/3"; + alpn_ = StringRef::from_lit("spdy/3"); break; case SPDYLAY_PROTO_SPDY3_1: - alpn_ = "spdy/3.1"; + alpn_ = StringRef::from_lit("spdy/3.1"); break; default: - alpn_ = "spdy/unknown"; + alpn_ = StringRef::from_lit("spdy/unknown"); } // At this point, input buffer is already filled with some bytes. @@ -599,7 +615,7 @@ int ClientHandler::validate_next_proto() { if (proto == StringRef::from_lit("http/1.1")) { upstream_ = make_unique(this); - alpn_ = proto.str(); + alpn_ = StringRef::from_lit("http/1.1"); // At this point, input buffer is already filled with some bytes. // The read callback is not called until new data come. So consume @@ -629,7 +645,7 @@ int ClientHandler::on_read() { } int ClientHandler::on_write() { return on_write_(*this); } -const std::string &ClientHandler::get_ipaddr() const { return ipaddr_; } +const StringRef &ClientHandler::get_ipaddr() const { return ipaddr_; } bool ClientHandler::get_should_close_after_write() const { return should_close_after_write_; @@ -974,7 +990,7 @@ ClientHandler::get_downstream_connection(Downstream *downstream) { if (shared_addr->affinity == AFFINITY_IP) { if (!affinity_hash_computed_) { - affinity_hash_ = compute_affinity_from_ip(StringRef{ipaddr_}); + affinity_hash_ = compute_affinity_from_ip(ipaddr_); affinity_hash_computed_ = true; } @@ -1093,7 +1109,7 @@ SSL *ClientHandler::get_ssl() const { return conn_.tls.ssl; } void ClientHandler::direct_http2_upgrade() { upstream_ = make_unique(this); - alpn_ = NGHTTP2_CLEARTEXT_PROTO_VERSION_ID; + alpn_ = StringRef::from_lit(NGHTTP2_CLEARTEXT_PROTO_VERSION_ID); on_read_ = &ClientHandler::upstream_read; write_ = &ClientHandler::write_clear; } @@ -1118,7 +1134,7 @@ int ClientHandler::perform_http2_upgrade(HttpsUpstream *http) { upstream_.release(); // TODO We might get other version id in HTTP2-settings, if we // support aliasing for h2, but we just use library default for now. - alpn_ = NGHTTP2_CLEARTEXT_PROTO_VERSION_ID; + alpn_ = StringRef::from_lit(NGHTTP2_CLEARTEXT_PROTO_VERSION_ID); on_read_ = &ClientHandler::upstream_http2_connhd_read; write_ = &ClientHandler::write_clear; @@ -1199,7 +1215,7 @@ void ClientHandler::write_accesslog(Downstream *downstream) { upstream_accesslog( get_config()->logging.access.format, LogSpec{ - downstream, downstream->get_addr(), StringRef{ipaddr_}, + downstream, downstream->get_addr(), ipaddr_, http2::to_method_string(req.method), req.method == HTTP_CONNECT @@ -1212,15 +1228,14 @@ void ClientHandler::write_accesslog(Downstream *downstream) { : StringRef::from_lit("-") : StringRef(req.path), - StringRef(alpn_), - nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl), + alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl), std::chrono::system_clock::now(), // time_now downstream->get_request_start_time(), // request_start_time std::chrono::high_resolution_clock::now(), // request_end_time req.http_major, req.http_minor, resp.http_status, - downstream->response_sent_body_length, StringRef(port_), faddr_->port, + downstream->response_sent_body_length, port_, faddr_->port, get_config()->pid, }); } @@ -1231,21 +1246,20 @@ void ClientHandler::write_accesslog(int major, int minor, unsigned int status, auto highres_now = std::chrono::high_resolution_clock::now(); nghttp2::ssl::TLSSessionInfo tls_info; - upstream_accesslog(get_config()->logging.access.format, - LogSpec{ - nullptr, nullptr, StringRef(ipaddr_), - StringRef::from_lit("-"), // method - StringRef::from_lit("-"), // path, - StringRef(alpn_), nghttp2::ssl::get_tls_session_info( - &tls_info, conn_.tls.ssl), - time_now, - highres_now, // request_start_time TODO is - // there a better value? - highres_now, // request_end_time - major, minor, // major, minor - status, body_bytes_sent, StringRef(port_), - faddr_->port, get_config()->pid, - }); + upstream_accesslog( + get_config()->logging.access.format, + LogSpec{ + nullptr, nullptr, ipaddr_, + StringRef::from_lit("-"), // method + StringRef::from_lit("-"), // path, + alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl), + time_now, + highres_now, // request_start_time TODO is + // there a better value? + highres_now, // request_end_time + major, minor, // major, minor + status, body_bytes_sent, port_, faddr_->port, get_config()->pid, + }); } ClientHandler::ReadBuf *ClientHandler::get_rb() { return &rb_; } @@ -1474,8 +1488,9 @@ int ClientHandler::proxy_protocol_read() { rb_.drain(end + 2 - rb_.pos); - ipaddr_.assign(src_addr, src_addr + src_addrlen); - port_.assign(src_port, src_port + src_portlen); + ipaddr_ = + make_string_ref(balloc_, StringRef{src_addr, src_addr + src_addrlen}); + port_ = make_string_ref(balloc_, StringRef{src_port, src_port + src_portlen}); if (LOG_ENABLED(INFO)) { CLOG(INFO, this) << "PROXY-protocol-v1: Finished, " << (rb_.pos - first) @@ -1495,16 +1510,16 @@ StringRef ClientHandler::get_forwarded_by() const { return StringRef{faddr_->hostport}; } -StringRef ClientHandler::get_forwarded_for() const { - return StringRef{forwarded_for_}; -} +StringRef ClientHandler::get_forwarded_for() const { return forwarded_for_; } const UpstreamAddr *ClientHandler::get_upstream_addr() const { return faddr_; } Connection *ClientHandler::get_connection() { return &conn_; }; -void ClientHandler::set_tls_sni(const StringRef &sni) { sni_ = sni.str(); } +void ClientHandler::set_tls_sni(const StringRef &sni) { + sni_ = make_string_ref(balloc_, sni); +} -StringRef ClientHandler::get_tls_sni() const { return StringRef{sni_}; } +StringRef ClientHandler::get_tls_sni() const { return sni_; } } // namespace shrpx diff --git a/src/shrpx_client_handler.h b/src/shrpx_client_handler.h index 567972c4..8a27c24b 100644 --- a/src/shrpx_client_handler.h +++ b/src/shrpx_client_handler.h @@ -37,6 +37,7 @@ #include "shrpx_connection.h" #include "buffer.h" #include "memchunk.h" +#include "allocator.h" using namespace nghttp2; @@ -54,8 +55,8 @@ struct DownstreamAddr; class ClientHandler { public: - ClientHandler(Worker *worker, int fd, SSL *ssl, const char *ipaddr, - const char *port, int family, const UpstreamAddr *faddr); + ClientHandler(Worker *worker, int fd, SSL *ssl, const StringRef &ipaddr, + const StringRef &port, int family, const UpstreamAddr *faddr); ~ClientHandler(); int noop(); @@ -90,8 +91,7 @@ public: void reset_upstream_write_timeout(ev_tstamp t); int validate_next_proto(); - const std::string &get_ipaddr() const; - const std::string &get_port() const; + const StringRef &get_ipaddr() const; bool get_should_close_after_write() const; void set_should_close_after_write(bool f); Upstream *get_upstream(); @@ -163,20 +163,21 @@ public: StringRef get_tls_sni() const; private: + BlockAllocator balloc_; Connection conn_; ev_timer reneg_shutdown_timer_; std::unique_ptr upstream_; // IP address of client. If UNIX domain socket is used, this is // "localhost". - std::string ipaddr_; - std::string port_; + StringRef ipaddr_; + StringRef port_; // The ALPN identifier negotiated for this connection. - std::string alpn_; + StringRef alpn_; // The client address used in "for" parameter of Forwarded header // field. - std::string forwarded_for_; + StringRef forwarded_for_; // lowercased TLS SNI which client sent. - std::string sni_; + StringRef sni_; std::function read_, write_; std::function on_read_, on_write_; // Address of frontend listening socket diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 2a85f980..27efbf83 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -357,7 +357,7 @@ int Http2DownstreamConnection::push_request_headers() { if (xffconf.add) { StringRef xff_value; - auto addr = StringRef{upstream->get_client_handler()->get_ipaddr()}; + auto addr = upstream->get_client_handler()->get_ipaddr(); if (xff) { xff_value = concat_string_ref(balloc, xff->value, StringRef::from_lit(", "), addr); diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 5a20271d..c59251be 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -829,16 +829,16 @@ SSL *create_ssl(SSL_CTX *ssl_ctx) { ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr, int addrlen, const UpstreamAddr *faddr) { - char host[NI_MAXHOST]; - char service[NI_MAXSERV]; + std::array host; + std::array service; int rv; if (addr->sa_family == AF_UNIX) { - std::copy_n("localhost", sizeof("localhost"), host); + std::copy_n("localhost", sizeof("localhost"), std::begin(host)); service[0] = '\0'; } else { - rv = getnameinfo(addr, addrlen, host, sizeof(host), service, - sizeof(service), NI_NUMERICHOST | NI_NUMERICSERV); + rv = getnameinfo(addr, addrlen, host.data(), host.size(), service.data(), + service.size(), NI_NUMERICHOST | NI_NUMERICSERV); if (rv != 0) { LOG(ERROR) << "getnameinfo() failed: " << gai_strerror(rv); @@ -867,8 +867,8 @@ ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr, } } - return new ClientHandler(worker, fd, ssl, host, service, addr->sa_family, - faddr); + return new ClientHandler(worker, fd, ssl, StringRef{host.data()}, + StringRef{service.data()}, addr->sa_family, faddr); } bool tls_hostname_match(const StringRef &pattern, const StringRef &hostname) { diff --git a/src/util.h b/src/util.h index 81aa671c..94b8fc35 100644 --- a/src/util.h +++ b/src/util.h @@ -665,16 +665,17 @@ uint64_t get_uint64(const uint8_t *data); int read_mime_types(std::map &res, const char *filename); -template -std::string random_alpha_digit(Generator &gen, size_t len) { - std::string res; - res.reserve(len); +// Fills random alpha and digit byte to the range [|first|, |last|). +// Returns the one beyond the |last|. +template +OutputIt random_alpha_digit(OutputIt first, OutputIt last, Generator &gen) { + constexpr uint8_t s[] = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; std::uniform_int_distribution<> dis(0, 26 * 2 + 10 - 1); - for (; len > 0; --len) { - res += "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"[dis( - gen)]; + for (; first != last; ++first) { + *first = s[dis(gen)]; } - return res; + return first; } template diff --git a/src/util_test.cc b/src/util_test.cc index 313e9203..ac104ace 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -26,6 +26,7 @@ #include #include +#include #include @@ -528,4 +529,19 @@ void test_util_strifind(void) { StringRef::from_lit("http1"))); } +void test_util_random_alpha_digit(void) { + std::random_device rd; + std::mt19937 gen(rd()); + std::array data; + + auto p = util::random_alpha_digit(std::begin(data), std::end(data), gen); + + CU_ASSERT(std::end(data) == p); + + for (auto b : data) { + CU_ASSERT(('A' <= b && b <= 'Z') || ('a' <= b && b <= 'z') || + ('0' <= b && b <= '9')); + } +} + } // namespace shrpx diff --git a/src/util_test.h b/src/util_test.h index c7d7a8e7..6741da9e 100644 --- a/src/util_test.h +++ b/src/util_test.h @@ -62,6 +62,7 @@ void test_util_parse_config_str_list(void); void test_util_make_http_hostport(void); void test_util_make_hostport(void); void test_util_strifind(void); +void test_util_random_alpha_digit(void); } // namespace shrpx