From acbf38fd3c4febb75a9f86813d8a09405222b04e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 4 Mar 2016 00:26:59 +0900 Subject: [PATCH] src: Refactor using StringRef, simplify function parameters --- src/asio_server_serve_mux.cc | 4 +- src/http2.cc | 67 ++++---- src/http2.h | 28 ++-- src/http2_test.cc | 315 ++++++++++++++++------------------- src/shrpx_http2_upstream.cc | 9 +- 5 files changed, 191 insertions(+), 232 deletions(-) diff --git a/src/asio_server_serve_mux.cc b/src/asio_server_serve_mux.cc index b635ee4d..7765adf0 100644 --- a/src/asio_server_serve_mux.cc +++ b/src/asio_server_serve_mux.cc @@ -77,8 +77,8 @@ bool serve_mux::handle(std::string pattern, request_cb cb) { request_cb serve_mux::handler(request_impl &req) const { auto &path = req.uri().path; if (req.method() != "CONNECT") { - auto clean_path = ::nghttp2::http2::path_join( - nullptr, 0, nullptr, 0, path.c_str(), path.size(), nullptr, 0); + auto clean_path = ::nghttp2::http2::path_join(StringRef{}, StringRef{}, + StringRef{path}, StringRef{}); if (clean_path != path) { auto new_uri = util::percent_encode_path(clean_path); auto &uref = req.uri(); diff --git a/src/http2.cc b/src/http2.cc index 3bad70cc..6bc898b9 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -1185,39 +1185,37 @@ void eat_dir(std::string &path) { } } // namespace -std::string path_join(const char *base_path, size_t base_pathlen, - const char *base_query, size_t base_querylen, - const char *rel_path, size_t rel_pathlen, - const char *rel_query, size_t rel_querylen) { +std::string path_join(const StringRef &base_path, const StringRef &base_query, + const StringRef &rel_path, const StringRef &rel_query) { std::string res; - if (rel_pathlen == 0) { - if (base_pathlen == 0) { + if (rel_path.empty()) { + if (base_path.empty()) { res = "/"; } else { - res.assign(base_path, base_pathlen); + res.assign(std::begin(base_path), std::end(base_path)); } - if (rel_querylen == 0) { - if (base_querylen) { + if (rel_query.empty()) { + if (!base_query.empty()) { res += '?'; - res.append(base_query, base_querylen); + res.append(std::begin(base_query), std::end(base_query)); } return res; } res += '?'; - res.append(rel_query, rel_querylen); + res.append(std::begin(rel_query), std::end(rel_query)); return res; } - auto first = rel_path; - auto last = rel_path + rel_pathlen; + auto first = std::begin(rel_path); + auto last = std::end(rel_path); if (rel_path[0] == '/') { res = "/"; ++first; - } else if (base_pathlen == 0) { + } else if (base_path.empty()) { res = "/"; } else { - res.assign(base_path, base_pathlen); + res.assign(std::begin(base_path), std::end(base_path)); } for (; first != last;) { @@ -1254,9 +1252,9 @@ std::string path_join(const char *base_path, size_t base_pathlen, for (; first != last && *first == '/'; ++first) ; } - if (rel_querylen) { + if (!rel_query.empty()) { res += '?'; - res.append(rel_query, rel_querylen); + res.append(std::begin(rel_query), std::end(rel_query)); } return res; } @@ -1480,15 +1478,14 @@ int get_pure_path_component(const char **base, size_t *baselen, } int construct_push_component(std::string &scheme, std::string &authority, - std::string &path, const char *base, - size_t baselen, const char *uri, size_t len) { + std::string &path, const StringRef &base, + const StringRef &uri) { int rv; - const char *rel, *relq = nullptr; - size_t rellen, relqlen = 0; + StringRef rel, relq; http_parser_url u{}; - rv = http_parser_parse_url(uri, len, 0, &u); + rv = http_parser_parse_url(uri.c_str(), uri.size(), 0, &u); if (rv != 0) { if (uri[0] == '/') { @@ -1496,22 +1493,20 @@ int construct_push_component(std::string &scheme, std::string &authority, } // treat link_url as relative URI. - auto end = std::find(uri, uri + len, '#'); - auto q = std::find(uri, end, '?'); + auto end = std::find(std::begin(uri), std::end(uri), '#'); + auto q = std::find(std::begin(uri), end, '?'); - rel = uri; - rellen = q - uri; + rel = StringRef{std::begin(uri), q}; if (q != end) { - relq = q + 1; - relqlen = end - relq; + relq = StringRef{q + 1, std::end(uri)}; } } else { if (u.field_set & (1 << UF_SCHEMA)) { - http2::copy_url_component(scheme, &u, UF_SCHEMA, uri); + http2::copy_url_component(scheme, &u, UF_SCHEMA, uri.c_str()); } if (u.field_set & (1 << UF_HOST)) { - http2::copy_url_component(authority, &u, UF_HOST, uri); + http2::copy_url_component(authority, &u, UF_HOST, uri.c_str()); if (u.field_set & (1 << UF_PORT)) { authority += ':'; authority += util::utos(u.port); @@ -1520,22 +1515,18 @@ int construct_push_component(std::string &scheme, std::string &authority, if (u.field_set & (1 << UF_PATH)) { auto &f = u.field_data[UF_PATH]; - rel = uri + f.off; - rellen = f.len; + rel = StringRef{uri.c_str() + f.off, f.len}; } else { - rel = "/"; - rellen = 1; + rel = StringRef::from_lit("/"); } if (u.field_set & (1 << UF_QUERY)) { auto &f = u.field_data[UF_QUERY]; - relq = uri + f.off; - relqlen = f.len; + relq = StringRef{uri.c_str() + f.off, f.len}; } } - path = - http2::path_join(base, baselen, nullptr, 0, rel, rellen, relq, relqlen); + path = http2::path_join(base, StringRef{}, rel, relq); return 0; } diff --git a/src/http2.h b/src/http2.h index 2bc93f03..23707b45 100644 --- a/src/http2.h +++ b/src/http2.h @@ -297,16 +297,13 @@ struct LinkHeader { // is ignored during parsing. std::vector parse_link_header(const char *src, size_t len); -// Constructs path by combining base path |base_path| of length -// |base_pathlen| with another path |rel_path| of length -// |rel_pathlen|. The base path and another path can have optional +// Constructs path by combining base path |base_path| with another +// path |rel_path|. The base path and another path can have optional // query component. This function assumes |base_path| is normalized. // In other words, it does not contain ".." or "." path components // and starts with "/" if it is not empty. -std::string path_join(const char *base_path, size_t base_pathlen, - const char *base_query, size_t base_querylen, - const char *rel_path, size_t rel_pathlen, - const char *rel_query, size_t rel_querylen); +std::string path_join(const StringRef &base, const StringRef &base_query, + const StringRef &rel_path, const StringRef &rel_query); // true if response has body, taking into account the request method // and status code. @@ -359,8 +356,7 @@ std::string normalize_path(InputIt first, InputIt last) { } result.append(first, last); } - return path_join(nullptr, 0, nullptr, 0, result.c_str(), result.size(), - nullptr, 0); + return path_join(StringRef{}, StringRef{}, StringRef{result}, StringRef{}); } template @@ -384,14 +380,14 @@ std::string rewrite_clean_path(InputIt first, InputIt last) { int get_pure_path_component(const char **base, size_t *baselen, const std::string &uri); -// Deduces scheme, authority and path from given |uri| of length -// |len|, and stores them in |scheme|, |authority|, and |path| -// respectively. If |uri| is relative path, path resolution is taken -// palce using path given in |base| of length |baselen|. This -// function returns 0 if it succeeds, or -1. +// Deduces scheme, authority and path from given |uri|, and stores +// them in |scheme|, |authority|, and |path| respectively. If |uri| +// is relative path, path resolution takes place using path given in +// |base| of length |baselen|. This function returns 0 if it +// succeeds, or -1. int construct_push_component(std::string &scheme, std::string &authority, - std::string &path, const char *base, - size_t baselen, const char *uri, size_t len); + std::string &path, const StringRef &base, + const StringRef &uri); } // namespace http2 diff --git a/src/http2_test.cc b/src/http2_test.cc index 88bf89e3..e3b19693 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -283,21 +283,21 @@ void test_http2_parse_link_header(void) { constexpr char s[] = "; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // With extra link-param. URI url should be extracted constexpr char s[] = "; rel=preload; as=file"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // With extra link-param. URI url should be extracted constexpr char s[] = "; as=file; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // With extra link-param and quote-string. URI url should be @@ -305,7 +305,7 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; rel=preload; title="foo,bar")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // With extra link-param and quote-string. URI url should be @@ -313,36 +313,37 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; title="foo,bar"; rel=preload)"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // ',' after quote-string - constexpr char s[] = R"(; title="foo,bar", ; rel=preload)"; + constexpr char s[] = R"(; title="foo,bar", ; rel=preload)"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[25], &s[28]) == res[0].uri); + CU_ASSERT("url2" == res[0].uri); + CU_ASSERT(&s[25] == &res[0].uri[0]); } { // Only first URI should be extracted. - constexpr char s[] = "; rel=preload, "; + constexpr char s[] = "; rel=preload, "; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // Both have rel=preload, so both urls should be extracted - constexpr char s[] = "; rel=preload, ; rel=preload"; + constexpr char s[] = "; rel=preload, ; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(2 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); - CU_ASSERT(std::make_pair(&s[21], &s[24]) == res[1].uri); + CU_ASSERT("url" == res[0].uri); + CU_ASSERT("url2" == res[1].uri); } { // Second URI uri should be extracted. - constexpr char s[] = ", ;rel=preload"; + constexpr char s[] = ", ;rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[8], &s[11]) == res[0].uri); + CU_ASSERT("url2" == res[0].uri); } { // Error if input ends with ';' @@ -361,14 +362,14 @@ void test_http2_parse_link_header(void) { constexpr char s[] = ";rel=preload,"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // Multiple repeated ','s between fields is OK - constexpr char s[] = ",,,;rel=preload"; + constexpr char s[] = ",,,;rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[9], &s[12]) == res[0].uri); + CU_ASSERT("url2" == res[0].uri); } { // Error if url is not enclosed by <> @@ -408,25 +409,25 @@ void test_http2_parse_link_header(void) { } { // Without whitespaces - constexpr char s[] = ";as=file;rel=preload,;rel=preload"; + constexpr char s[] = ";as=file;rel=preload,;rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(2 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); - CU_ASSERT(std::make_pair(&s[27], &s[30]) == res[1].uri); + CU_ASSERT("url" == res[0].uri); + CU_ASSERT("url2" == res[1].uri); } { // link-extension may have no value constexpr char s[] = "; as; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // ext-name-star constexpr char s[] = "; foo*=bar; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // '*' is not allowed expect for trailing one @@ -457,7 +458,7 @@ void test_http2_parse_link_header(void) { constexpr char s[] = " ; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[3], &s[6]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload is a prefix of bogus rel parameter value @@ -470,35 +471,35 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; rel="preload")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list followed by another parameter constexpr char s[] = R"(; rel="preload foo")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list following another parameter constexpr char s[] = R"(; rel="foo preload")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list between other parameters constexpr char s[] = R"(; rel="foo preload bar")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list between other parameters constexpr char s[] = R"(; rel="foo preload bar")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // no preload in relation-types list @@ -514,24 +515,24 @@ void test_http2_parse_link_header(void) { } { // preload in relation-types list, followed by another link-value. - constexpr char s[] = R"(; rel="preload", )"; + constexpr char s[] = R"(; rel="preload", )"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list, following another link-value. - constexpr char s[] = R"(, ; rel="preload")"; + constexpr char s[] = R"(, ; rel="preload")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[8], &s[11]) == res[0].uri); + CU_ASSERT("url2" == res[0].uri); } { // preload in relation-types list, followed by another link-param. constexpr char s[] = R"(; rel="preload"; as="font")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list, followed by character other @@ -553,7 +554,7 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; rel="preload",)"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // preload in relation-types list but there is preceding white @@ -574,14 +575,14 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; rel=preload; title="foo\"baz\"bar")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // anchor="" is acceptable constexpr char s[] = R"(; rel=preload; anchor="")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // With anchor="#foo", url should be ignored @@ -599,10 +600,10 @@ void test_http2_parse_link_header(void) { // First url is ignored With anchor="#foo", but url should be // accepted. constexpr char s[] = - R"(; rel=preload; anchor="#foo", ; rel=preload)"; + R"(; rel=preload; anchor="#foo", ; rel=preload)"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[36], &s[39]) == res[0].uri); + CU_ASSERT("url2" == res[0].uri); } { // With loadpolicy="next", url should be ignored @@ -615,16 +616,16 @@ void test_http2_parse_link_header(void) { constexpr char s[] = R"(; rel=preload; loadpolicy="")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // case-insensitive match - constexpr char s[] = R"(; rel=preload; ANCHOR="#foo", ; )" - R"(REL=PRELOAD, ; REL="foo PRELOAD bar")"; + constexpr char s[] = R"(; rel=preload; ANCHOR="#foo", ; )" + R"(REL=PRELOAD, ; REL="foo PRELOAD bar")"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(2 == res.size()); - CU_ASSERT(std::make_pair(&s[36], &s[39]) == res[0].uri); - CU_ASSERT(std::make_pair(&s[42 + 14], &s[42 + 17]) == res[1].uri); + CU_ASSERT("url2" == res[0].uri); + CU_ASSERT("url3" == res[1].uri); } { // nopush at the end of input @@ -649,186 +650,166 @@ void test_http2_parse_link_header(void) { constexpr char s[] = "; nopushyes; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } { // rel=preload twice constexpr char s[] = "; rel=preload; rel=preload"; auto res = http2::parse_link_header(s, str_size(s)); CU_ASSERT(1 == res.size()); - CU_ASSERT(std::make_pair(&s[1], &s[4]) == res[0].uri); + CU_ASSERT("url" == res[0].uri); } } void test_http2_path_join(void) { { - const char base[] = "/"; - const char rel[] = "/"; - CU_ASSERT("/" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/"); + CU_ASSERT("/" == http2::path_join(base, StringRef{}, rel, StringRef{})); } { - const char base[] = "/"; - const char rel[] = "/alpha"; - CU_ASSERT("/alpha" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/alpha"); + CU_ASSERT("/alpha" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel ends with trailing '/' - const char base[] = "/"; - const char rel[] = "/alpha/"; - CU_ASSERT("/alpha/" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/alpha/"); + CU_ASSERT("/alpha/" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel contains multiple components - const char base[] = "/"; - const char rel[] = "/alpha/bravo"; - CU_ASSERT("/alpha/bravo" == http2::path_join(base, sizeof(base) - 1, - nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/alpha/bravo"); + CU_ASSERT("/alpha/bravo" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel is relative - const char base[] = "/"; - const char rel[] = "alpha/bravo"; - CU_ASSERT("/alpha/bravo" == http2::path_join(base, sizeof(base) - 1, - nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("alpha/bravo"); + CU_ASSERT("/alpha/bravo" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel is relative and base ends without /, which means it refers // to file. - const char base[] = "/alpha"; - const char rel[] = "bravo/charlie"; + auto base = StringRef::from_lit("/alpha"); + auto rel = StringRef::from_lit("bravo/charlie"); CU_ASSERT("/bravo/charlie" == - http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel contains repeated '/'s - const char base[] = "/"; - const char rel[] = "/alpha/////bravo/////"; - CU_ASSERT("/alpha/bravo/" == http2::path_join(base, sizeof(base) - 1, - nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/alpha/////bravo/////"); + CU_ASSERT("/alpha/bravo/" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // base ends with '/', so '..' eats 'bravo' - const char base[] = "/alpha/bravo/"; - const char rel[] = "../charlie/delta"; + auto base = StringRef::from_lit("/alpha/bravo/"); + auto rel = StringRef::from_lit("../charlie/delta"); CU_ASSERT("/alpha/charlie/delta" == - http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // base does not end with '/', so '..' eats 'alpha/bravo' - const char base[] = "/alpha/bravo"; - const char rel[] = "../charlie"; - CU_ASSERT("/charlie" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/alpha/bravo"); + auto rel = StringRef::from_lit("../charlie"); + CU_ASSERT("/charlie" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // 'charlie' is eaten by following '..' - const char base[] = "/alpha/bravo/"; - const char rel[] = "../charlie/../delta"; - CU_ASSERT("/alpha/delta" == http2::path_join(base, sizeof(base) - 1, - nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/alpha/bravo/"); + auto rel = StringRef::from_lit("../charlie/../delta"); + CU_ASSERT("/alpha/delta" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // excessive '..' results in '/' - const char base[] = "/alpha/bravo/"; - const char rel[] = "../../../"; - CU_ASSERT("/" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/alpha/bravo/"); + auto rel = StringRef::from_lit("../../../"); + CU_ASSERT("/" == http2::path_join(base, StringRef{}, rel, StringRef{})); } { // excessive '..' and path component - const char base[] = "/alpha/bravo/"; - const char rel[] = "../../../charlie"; - CU_ASSERT("/charlie" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/alpha/bravo/"); + auto rel = StringRef::from_lit("../../../charlie"); + CU_ASSERT("/charlie" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // rel ends with '..' - const char base[] = "/alpha/bravo/"; - const char rel[] = "charlie/.."; - CU_ASSERT("/alpha/bravo/" == http2::path_join(base, sizeof(base) - 1, - nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef::from_lit("/alpha/bravo/"); + auto rel = StringRef::from_lit("charlie/.."); + CU_ASSERT("/alpha/bravo/" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // base empty and rel contains '..' - const char base[] = ""; - const char rel[] = "charlie/.."; - CU_ASSERT("/" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef{}; + auto rel = StringRef::from_lit("charlie/.."); + CU_ASSERT("/" == http2::path_join(base, StringRef{}, rel, StringRef{})); } { // '.' is ignored - const char base[] = "/"; - const char rel[] = "charlie/././././delta"; + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("charlie/././././delta"); CU_ASSERT("/charlie/delta" == - http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, nullptr, 0)); + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // trailing '.' is ignored - const char base[] = "/"; - const char rel[] = "charlie/."; - CU_ASSERT("/charlie/" == http2::path_join(base, sizeof(base) - 1, nullptr, - 0, rel, sizeof(rel) - 1, nullptr, - 0)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("charlie/."); + CU_ASSERT("/charlie/" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // query - const char base[] = "/"; - const char rel[] = "/"; - const char relq[] = "q"; - CU_ASSERT("/?q" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, rel, - sizeof(rel) - 1, relq, - sizeof(relq) - 1)); + auto base = StringRef::from_lit("/"); + auto rel = StringRef::from_lit("/"); + auto relq = StringRef::from_lit("q"); + CU_ASSERT("/?q" == http2::path_join(base, StringRef{}, rel, relq)); } { // empty rel and query - const char base[] = "/alpha"; - const char rel[] = ""; - const char relq[] = "q"; - CU_ASSERT("/alpha?q" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, relq, - sizeof(relq) - 1)); + auto base = StringRef::from_lit("/alpha"); + auto rel = StringRef{}; + auto relq = StringRef::from_lit("q"); + CU_ASSERT("/alpha?q" == http2::path_join(base, StringRef{}, rel, relq)); } { // both rel and query are empty - const char base[] = "/alpha"; - const char baseq[] = "r"; - const char rel[] = ""; - const char relq[] = ""; - CU_ASSERT("/alpha?r" == - http2::path_join(base, sizeof(base) - 1, baseq, sizeof(baseq) - 1, - rel, sizeof(rel) - 1, relq, sizeof(relq) - 1)); + auto base = StringRef::from_lit("/alpha"); + auto baseq = StringRef::from_lit("r"); + auto rel = StringRef{}; + auto relq = StringRef{}; + CU_ASSERT("/alpha?r" == http2::path_join(base, baseq, rel, relq)); } { // empty base - const char base[] = ""; - const char rel[] = "/alpha"; - CU_ASSERT("/alpha" == http2::path_join(base, sizeof(base) - 1, nullptr, 0, - rel, sizeof(rel) - 1, nullptr, 0)); + auto base = StringRef{}; + auto rel = StringRef::from_lit("/alpha"); + CU_ASSERT("/alpha" == + http2::path_join(base, StringRef{}, rel, StringRef{})); } { // everything is empty - CU_ASSERT("/" == - http2::path_join(nullptr, 0, nullptr, 0, nullptr, 0, nullptr, 0)); + CU_ASSERT("/" == http2::path_join(StringRef{}, StringRef{}, StringRef{}, + StringRef{})); } { // only baseq is not empty - const char base[] = ""; - const char baseq[] = "r"; - const char rel[] = ""; - CU_ASSERT("/?r" == http2::path_join(base, sizeof(base) - 1, baseq, - sizeof(baseq) - 1, rel, sizeof(rel) - 1, - nullptr, 0)); + auto base = StringRef{}; + auto baseq = StringRef::from_lit("r"); + auto rel = StringRef{}; + CU_ASSERT("/?r" == http2::path_join(base, baseq, rel, StringRef{})); } } @@ -914,19 +895,15 @@ void test_http2_get_pure_path_component(void) { } void test_http2_construct_push_component(void) { - const char *base; - size_t baselen; - std::string uri; + StringRef base, uri; std::string scheme, authority, path; - base = "/b/"; - baselen = 3; + base = StringRef::from_lit("/b/"); - uri = "https://example.org/foo"; + uri = StringRef::from_lit("https://example.org/foo"); - CU_ASSERT(0 == http2::construct_push_component(scheme, authority, path, base, - baselen, uri.c_str(), - uri.size())); + CU_ASSERT( + 0 == http2::construct_push_component(scheme, authority, path, base, uri)); CU_ASSERT("https" == scheme); CU_ASSERT("example.org" == authority); CU_ASSERT("/foo" == path); @@ -935,11 +912,10 @@ void test_http2_construct_push_component(void) { authority.clear(); path.clear(); - uri = "/foo/bar?q=a"; + uri = StringRef::from_lit("/foo/bar?q=a"); - CU_ASSERT(0 == http2::construct_push_component(scheme, authority, path, base, - baselen, uri.c_str(), - uri.size())); + CU_ASSERT( + 0 == http2::construct_push_component(scheme, authority, path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/foo/bar?q=a" == path); @@ -948,11 +924,10 @@ void test_http2_construct_push_component(void) { authority.clear(); path.clear(); - uri = "foo/../bar?q=a"; + uri = StringRef::from_lit("foo/../bar?q=a"); - CU_ASSERT(0 == http2::construct_push_component(scheme, authority, path, base, - baselen, uri.c_str(), - uri.size())); + CU_ASSERT( + 0 == http2::construct_push_component(scheme, authority, path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/b/bar?q=a" == path); @@ -961,11 +936,10 @@ void test_http2_construct_push_component(void) { authority.clear(); path.clear(); - uri = ""; + uri = StringRef{}; - CU_ASSERT(0 == http2::construct_push_component(scheme, authority, path, base, - baselen, uri.c_str(), - uri.size())); + CU_ASSERT( + 0 == http2::construct_push_component(scheme, authority, path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/" == path); @@ -974,11 +948,10 @@ void test_http2_construct_push_component(void) { authority.clear(); path.clear(); - uri = "?q=a"; + uri = StringRef::from_lit("?q=a"); - CU_ASSERT(0 == http2::construct_push_component(scheme, authority, path, base, - baselen, uri.c_str(), - uri.size())); + CU_ASSERT( + 0 == http2::construct_push_component(scheme, authority, path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/b/?q=a" == path); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 37edafcc..048cc50b 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1755,9 +1755,8 @@ int Http2Upstream::prepare_push_promise(Downstream *downstream) { const std::string *scheme_ptr, *authority_ptr; std::string scheme, authority, path; - rv = http2::construct_push_component(scheme, authority, path, base, - baselen, link.uri.c_str(), - link.uri.size()); + rv = http2::construct_push_component(scheme, authority, path, + StringRef{base, baselen}, link.uri); if (rv != 0) { continue; } @@ -1872,8 +1871,8 @@ int Http2Upstream::initiate_push(Downstream *downstream, const char *uri, const std::string *scheme_ptr, *authority_ptr; std::string scheme, authority, path; - rv = http2::construct_push_component(scheme, authority, path, base, baselen, - uri, len); + rv = http2::construct_push_component( + scheme, authority, path, StringRef{base, baselen}, StringRef{uri, len}); if (rv != 0) { return -1; }