diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 4c6b39c8..0e7b17a1 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -106,8 +106,8 @@ int main(int argc, char *argv[]) { shrpx::test_http2_get_pure_path_component) || !CU_add_test(pSuite, "http2_construct_push_component", shrpx::test_http2_construct_push_component) || - !CU_add_test(pSuite, "downstream_field_store_index_headers", - shrpx::test_downstream_field_store_index_headers) || + !CU_add_test(pSuite, "downstream_field_store_add_header_lower", + shrpx::test_downstream_field_store_add_header_lower) || !CU_add_test(pSuite, "downstream_field_store_header", shrpx::test_downstream_field_store_header) || !CU_add_test(pSuite, "downstream_crumble_request_cookie", diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 0d3b2618..11ec9756 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -330,10 +330,10 @@ void Downstream::crumble_request_cookie(std::vector &nva) { namespace { void add_header(bool &key_prev, size_t &sum, Headers &headers, std::string name, - std::string value, int16_t token) { + std::string value, bool no_index, int16_t token) { key_prev = true; sum += name.size() + value.size(); - headers.emplace_back(std::move(name), std::move(value), false, token); + headers.emplace_back(std::move(name), std::move(value), no_index, token); } } // namespace @@ -411,23 +411,17 @@ const Headers::value_type *FieldStore::header(const StringRef &name) const { return search_header_linear_backwards(headers_, name); } -void FieldStore::add_header_lower(const StringRef &name, - const StringRef &value) { +void FieldStore::add_header_lower(const StringRef &name, const StringRef &value, + bool no_index) { auto low_name = name.str(); util::inp_strlower(low_name); auto token = http2::lookup_token(low_name); shrpx::add_header(header_key_prev_, buffer_size_, headers_, - std::move(low_name), value.str(), token); + std::move(low_name), value.str(), no_index, token); } -void FieldStore::add_header(std::string name, std::string value, - int16_t token) { - buffer_size_ += name.size() + value.size(); - headers_.emplace_back(std::move(name), std::move(value), false, token); -} - -void FieldStore::add_header(const StringRef &name, const StringRef &value, - bool no_index, int16_t token) { +void FieldStore::add_header_token(const StringRef &name, const StringRef &value, + bool no_index, int16_t token) { shrpx::add_header(buffer_size_, headers_, name, value, no_index, token); } @@ -443,20 +437,21 @@ void FieldStore::append_last_header_value(const char *data, size_t len) { void FieldStore::clear_headers() { headers_.clear(); } -void FieldStore::add_trailer(const StringRef &name, const StringRef &value, - bool no_index, int16_t token) { - // Header size limit should be applied to all header and trailer - // fields combined. - shrpx::add_header(buffer_size_, trailers_, name, value, no_index, token); -} - void FieldStore::add_trailer_lower(const StringRef &name, - const StringRef &value) { + const StringRef &value, bool no_index) { auto low_name = name.str(); util::inp_strlower(low_name); auto token = http2::lookup_token(low_name); shrpx::add_header(trailer_key_prev_, buffer_size_, trailers_, - std::move(low_name), value.str(), token); + std::move(low_name), value.str(), no_index, token); +} + +void FieldStore::add_trailer_token(const StringRef &name, + const StringRef &value, bool no_index, + int16_t token) { + // Header size limit should be applied to all header and trailer + // fields combined. + shrpx::add_header(buffer_size_, trailers_, name, value, no_index, token); } void FieldStore::append_last_trailer_key(const char *data, size_t len) { diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 53778e49..8a855b1c 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -79,10 +79,10 @@ public: // such header is found, returns nullptr. const Headers::value_type *header(const StringRef &name) const; - void add_header_lower(const StringRef &name, const StringRef &value); - void add_header(std::string name, std::string value, int16_t token); - void add_header(const StringRef &name, const StringRef &value, bool no_index, - int16_t token); + void add_header_lower(const StringRef &name, const StringRef &value, + bool no_index); + void add_header_token(const StringRef &name, const StringRef &value, + bool no_index, int16_t token); void append_last_header_key(const char *data, size_t len); void append_last_header_value(const char *data, size_t len); @@ -97,9 +97,10 @@ public: // Empties headers. void clear_headers(); - void add_trailer(const StringRef &name, const StringRef &value, bool no_index, - int16_t token); - void add_trailer_lower(const StringRef &name, const StringRef &value); + void add_trailer_lower(const StringRef &name, const StringRef &value, + bool no_index); + void add_trailer_token(const StringRef &name, const StringRef &value, + bool no_index, int16_t token); void append_last_trailer_key(const char *data, size_t len); void append_last_trailer_value(const char *data, size_t len); diff --git a/src/shrpx_downstream_test.cc b/src/shrpx_downstream_test.cc index c7b559a4..5b496f9b 100644 --- a/src/shrpx_downstream_test.cc +++ b/src/shrpx_downstream_test.cc @@ -32,17 +32,24 @@ namespace shrpx { -void test_downstream_field_store_index_headers(void) { +void test_downstream_field_store_add_header_lower(void) { FieldStore fs(0); - fs.add_header("1", "0"); - fs.add_header("2", "1"); - fs.add_header("Charlie", "2"); - fs.add_header("Alpha", "3"); - fs.add_header("Delta", "4"); - fs.add_header("BravO", "5"); - fs.add_header(":method", "6"); - fs.add_header(":authority", "7"); - fs.index_headers(); + fs.add_header_lower(StringRef::from_lit("1"), StringRef::from_lit("0"), + false); + fs.add_header_lower(StringRef::from_lit("2"), StringRef::from_lit("1"), + false); + fs.add_header_lower(StringRef::from_lit("Charlie"), StringRef::from_lit("2"), + false); + fs.add_header_lower(StringRef::from_lit("Alpha"), StringRef::from_lit("3"), + false); + fs.add_header_lower(StringRef::from_lit("Delta"), StringRef::from_lit("4"), + false); + fs.add_header_lower(StringRef::from_lit("BravO"), StringRef::from_lit("5"), + false); + fs.add_header_lower(StringRef::from_lit(":method"), StringRef::from_lit("6"), + false); + fs.add_header_lower(StringRef::from_lit(":authority"), + StringRef::from_lit("7"), false); auto ans = Headers{{"1", "0"}, {"2", "1"}, @@ -57,10 +64,13 @@ void test_downstream_field_store_index_headers(void) { void test_downstream_field_store_header(void) { FieldStore fs(0); - fs.add_header("alpha", "0"); - fs.add_header(":authority", "1"); - fs.add_header("content-length", "2"); - fs.index_headers(); + fs.add_header_token(StringRef::from_lit("alpha"), StringRef::from_lit("0"), + false, -1); + fs.add_header_token(StringRef::from_lit(":authority"), + StringRef::from_lit("1"), false, http2::HD__AUTHORITY); + fs.add_header_token(StringRef::from_lit("content-length"), + StringRef::from_lit("2"), false, + http2::HD_CONTENT_LENGTH); // By token CU_ASSERT(Header(":authority", "1") == *fs.header(http2::HD__AUTHORITY)); @@ -74,14 +84,18 @@ void test_downstream_field_store_header(void) { void test_downstream_crumble_request_cookie(void) { Downstream d(nullptr, nullptr, 0); auto &req = d.request(); - req.fs.add_header(":method", "get"); - req.fs.add_header(":path", "/"); - auto val = "alpha; bravo; ; ;; charlie;;"; - req.fs.add_header( - reinterpret_cast("cookie"), sizeof("cookie") - 1, - reinterpret_cast(val), strlen(val), true, -1); - req.fs.add_header("cookie", ";delta"); - req.fs.add_header("cookie", "echo"); + req.fs.add_header_token(StringRef::from_lit(":method"), + StringRef::from_lit("get"), false, -1); + req.fs.add_header_token(StringRef::from_lit(":path"), + StringRef::from_lit("/"), false, -1); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("alpha; bravo; ; ;; charlie;;"), + true, http2::HD_COOKIE); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit(";delta"), false, + http2::HD_COOKIE); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("echo"), false, http2::HD_COOKIE); std::vector nva; d.crumble_request_cookie(nva); @@ -114,12 +128,22 @@ void test_downstream_crumble_request_cookie(void) { void test_downstream_assemble_request_cookie(void) { Downstream d(nullptr, nullptr, 0); auto &req = d.request(); - req.fs.add_header(":method", "get"); - req.fs.add_header(":path", "/"); - req.fs.add_header("cookie", "alpha"); - req.fs.add_header("cookie", "bravo;"); - req.fs.add_header("cookie", "charlie; "); - req.fs.add_header("cookie", "delta;;"); + req.fs.add_header_token(StringRef::from_lit(":method"), + StringRef::from_lit("get"), false, -1); + req.fs.add_header_token(StringRef::from_lit(":path"), + StringRef::from_lit("/"), false, -1); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("alpha"), false, + http2::HD_COOKIE); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("bravo;"), false, + http2::HD_COOKIE); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("charlie; "), false, + http2::HD_COOKIE); + req.fs.add_header_token(StringRef::from_lit("cookie"), + StringRef::from_lit("delta;;"), false, + http2::HD_COOKIE); CU_ASSERT("alpha; bravo; charlie; delta" == d.assemble_request_cookie()); } @@ -129,8 +153,9 @@ void test_downstream_rewrite_location_response_header(void) { auto &resp = d.response(); d.set_request_downstream_host("localhost2"); req.authority = "localhost:8443"; - resp.fs.add_header("location", "http://localhost2:3000/"); - resp.fs.index_headers(); + resp.fs.add_header_token(StringRef::from_lit("location"), + StringRef::from_lit("http://localhost2:3000/"), + false, http2::HD_LOCATION); d.rewrite_location_response_header("https"); auto location = resp.fs.header(http2::HD_LOCATION); CU_ASSERT("https://localhost:8443/" == (*location).value); diff --git a/src/shrpx_downstream_test.h b/src/shrpx_downstream_test.h index 13b33744..bcae1887 100644 --- a/src/shrpx_downstream_test.h +++ b/src/shrpx_downstream_test.h @@ -31,7 +31,7 @@ namespace shrpx { -void test_downstream_field_store_index_headers(void); +void test_downstream_field_store_add_header_lower(void); void test_downstream_field_store_header(void); void test_downstream_crumble_request_cookie(void); void test_downstream_assemble_request_cookie(void); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 2c96a579..d11e3fd7 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -736,16 +736,17 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, } auto token = http2::lookup_token(name, namelen); + auto no_index = flags & NGHTTP2_NV_FLAG_NO_INDEX; if (trailer) { // just store header fields for trailer part - resp.fs.add_trailer(StringRef{name, namelen}, StringRef{value, valuelen}, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + resp.fs.add_trailer_token(StringRef{name, namelen}, + StringRef{value, valuelen}, no_index, token); return 0; } - resp.fs.add_header(StringRef{name, namelen}, StringRef{value, valuelen}, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + resp.fs.add_header_token(StringRef{name, namelen}, + StringRef{value, valuelen}, no_index, token); return 0; } case NGHTTP2_PUSH_PROMISE: { @@ -778,9 +779,9 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, } auto token = http2::lookup_token(name, namelen); - promised_req.fs.add_header(StringRef{name, namelen}, - StringRef{value, valuelen}, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + promised_req.fs.add_header_token(StringRef{name, namelen}, + StringRef{value, valuelen}, + flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } } @@ -927,8 +928,9 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, // Otherwise, use chunked encoding to keep upstream connection // open. In HTTP2, we are supporsed not to receive // transfer-encoding. - resp.fs.add_header("transfer-encoding", "chunked", - http2::HD_TRANSFER_ENCODING); + resp.fs.add_header_token(StringRef::from_lit("transfer-encoding"), + StringRef::from_lit("chunked"), false, + http2::HD_TRANSFER_ENCODING); downstream->set_chunked_response(true); } } diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 4506081c..0c8795dc 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -202,16 +202,17 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, } auto token = http2::lookup_token(name, namelen); + auto no_index = flags & NGHTTP2_NV_FLAG_NO_INDEX; if (frame->headers.cat == NGHTTP2_HCAT_HEADERS) { // just store header fields for trailer part - req.fs.add_trailer(StringRef{name, namelen}, StringRef{value, valuelen}, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + req.fs.add_trailer_token(StringRef{name, namelen}, + StringRef{value, valuelen}, no_index, token); return 0; } - req.fs.add_header(StringRef{name, namelen}, StringRef{value, valuelen}, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + req.fs.add_header_token(StringRef{name, namelen}, StringRef{value, valuelen}, + no_index, token); return 0; } } // namespace @@ -593,9 +594,9 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, req.path = http2::rewrite_clean_path(nv.value, nv.value + nv.valuelen); break; } - req.fs.add_header(StringRef{nv.name, nv.namelen}, - StringRef{nv.value, nv.valuelen}, - nv.flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + req.fs.add_header_token(StringRef{nv.name, nv.namelen}, + StringRef{nv.value, nv.valuelen}, + nv.flags & NGHTTP2_NV_FLAG_NO_INDEX, token); } promised_downstream->inspect_http2_request(); diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 0af56ce1..c5b3da09 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/src/shrpx_http_downstream_connection.cc @@ -691,7 +691,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { if (ensure_max_header_fields(downstream, httpconf) != 0) { return -1; } - resp.fs.add_header_lower(StringRef{data, len}, StringRef{}); + resp.fs.add_header_lower(StringRef{data, len}, StringRef{}, false); } } else { // trailer part @@ -704,7 +704,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { // wrong place or crash if trailer fields are currently empty. return -1; } - resp.fs.add_trailer_lower(StringRef(data, len), StringRef{}); + resp.fs.add_trailer_lower(StringRef(data, len), StringRef{}, false); } } return 0; diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 7f47b681..c84af459 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -142,7 +142,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { Downstream::HTTP1_REQUEST_HEADER_TOO_LARGE); return -1; } - req.fs.add_header_lower(StringRef{data, len}, StringRef{}); + req.fs.add_header_lower(StringRef{data, len}, StringRef{}, false); } } else { // trailer part @@ -156,7 +156,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { } return -1; } - req.fs.add_trailer_lower(StringRef{data, len}, StringRef{}); + req.fs.add_trailer_lower(StringRef{data, len}, StringRef{}, false); } } return 0; diff --git a/src/shrpx_mruby.cc b/src/shrpx_mruby.cc index bd57c146..507d13b9 100644 --- a/src/shrpx_mruby.cc +++ b/src/shrpx_mruby.cc @@ -94,14 +94,6 @@ int MRubyContext::run_app(Downstream *downstream, int phase) { mrb_->ud = nullptr; - if (data.request_headers_dirty) { - downstream->request().fs.index_headers(); - } - - if (data.response_headers_dirty) { - downstream->response().fs.index_headers(); - } - return rv; } diff --git a/src/shrpx_mruby.h b/src/shrpx_mruby.h index 55a18351..4466cf6f 100644 --- a/src/shrpx_mruby.h +++ b/src/shrpx_mruby.h @@ -67,8 +67,6 @@ enum { struct MRubyAssocData { Downstream *downstream; int phase; - bool request_headers_dirty; - bool response_headers_dirty; }; RProc *compile(mrb_state *mrb, const StringRef &filename); diff --git a/src/shrpx_mruby_module_request.cc b/src/shrpx_mruby_module_request.cc index 0b1ddb15..f585d730 100644 --- a/src/shrpx_mruby_module_request.cc +++ b/src/shrpx_mruby_module_request.cc @@ -216,17 +216,20 @@ mrb_value request_mod_header(mrb_state *mrb, mrb_value self, bool repl) { key = mrb_funcall(mrb, key, "downcase", 0); + auto keyref = + StringRef{RSTRING_PTR(key), static_cast(RSTRING_LEN(key))}; + auto token = http2::lookup_token(keyref.byte(), keyref.size()); + if (repl) { size_t p = 0; auto &headers = req.fs.headers(); for (size_t i = 0; i < headers.size(); ++i) { - auto &hd = headers[i]; - if (util::streq(std::begin(hd.name), hd.name.size(), RSTRING_PTR(key), - RSTRING_LEN(key))) { + auto &kv = headers[i]; + if (kv.name == keyref) { continue; } if (i != p) { - headers[p++] = std::move(hd); + headers[p++] = std::move(kv); } } headers.resize(p); @@ -236,16 +239,18 @@ mrb_value request_mod_header(mrb_state *mrb, mrb_value self, bool repl) { auto n = mrb_ary_len(mrb, values); for (int i = 0; i < n; ++i) { auto value = mrb_ary_entry(values, i); - req.fs.add_header(std::string(RSTRING_PTR(key), RSTRING_LEN(key)), - std::string(RSTRING_PTR(value), RSTRING_LEN(value))); + req.fs.add_header_token( + keyref, StringRef{RSTRING_PTR(value), + static_cast(RSTRING_LEN(value))}, + false, token); } } else if (!mrb_nil_p(values)) { - req.fs.add_header(std::string(RSTRING_PTR(key), RSTRING_LEN(key)), - std::string(RSTRING_PTR(values), RSTRING_LEN(values))); + req.fs.add_header_token(keyref, + StringRef{RSTRING_PTR(values), + static_cast(RSTRING_LEN(values))}, + false, token); } - data->request_headers_dirty = true; - return mrb_nil_value(); } } // namespace diff --git a/src/shrpx_mruby_module_response.cc b/src/shrpx_mruby_module_response.cc index 5b88f879..73dacccc 100644 --- a/src/shrpx_mruby_module_response.cc +++ b/src/shrpx_mruby_module_response.cc @@ -117,17 +117,20 @@ mrb_value response_mod_header(mrb_state *mrb, mrb_value self, bool repl) { key = mrb_funcall(mrb, key, "downcase", 0); + auto keyref = + StringRef{RSTRING_PTR(key), static_cast(RSTRING_LEN(key))}; + auto token = http2::lookup_token(keyref.byte(), keyref.size()); + if (repl) { size_t p = 0; auto &headers = resp.fs.headers(); for (size_t i = 0; i < headers.size(); ++i) { - auto &hd = headers[i]; - if (util::streq(std::begin(hd.name), hd.name.size(), RSTRING_PTR(key), - RSTRING_LEN(key))) { + auto &kv = headers[i]; + if (kv.name == keyref) { continue; } if (i != p) { - headers[p++] = std::move(hd); + headers[p++] = std::move(kv); } } headers.resize(p); @@ -137,16 +140,18 @@ mrb_value response_mod_header(mrb_state *mrb, mrb_value self, bool repl) { auto n = mrb_ary_len(mrb, values); for (int i = 0; i < n; ++i) { auto value = mrb_ary_entry(values, i); - resp.fs.add_header(std::string(RSTRING_PTR(key), RSTRING_LEN(key)), - std::string(RSTRING_PTR(value), RSTRING_LEN(value))); + resp.fs.add_header_token( + keyref, StringRef{RSTRING_PTR(value), + static_cast(RSTRING_LEN(value))}, + false, token); } } else if (!mrb_nil_p(values)) { - resp.fs.add_header(std::string(RSTRING_PTR(key), RSTRING_LEN(key)), - std::string(RSTRING_PTR(values), RSTRING_LEN(values))); + resp.fs.add_header_token( + keyref, StringRef{RSTRING_PTR(values), + static_cast(RSTRING_LEN(values))}, + false, token); } - data->response_headers_dirty = true; - return mrb_nil_value(); } } // namespace @@ -197,11 +202,6 @@ mrb_value response_return(mrb_state *mrb, mrb_value self) { resp.http_status = 200; } - if (data->response_headers_dirty) { - resp.fs.index_headers(); - data->response_headers_dirty = false; - } - if (downstream->expect_response_body() && vallen > 0) { body = reinterpret_cast(val); bodylen = vallen; @@ -211,8 +211,9 @@ mrb_value response_return(mrb_state *mrb, mrb_value self) { if (cl) { cl->value = util::utos(bodylen); } else { - resp.fs.add_header("content-length", util::utos(bodylen), - http2::HD_CONTENT_LENGTH); + resp.fs.add_header_token(StringRef::from_lit("content-length"), + StringRef{util::utos(bodylen)}, false, + http2::HD_CONTENT_LENGTH); } resp.fs.content_length = bodylen; @@ -220,7 +221,9 @@ mrb_value response_return(mrb_state *mrb, mrb_value self) { if (!date) { auto lgconf = log_config(); lgconf->update_tstamp(std::chrono::system_clock::now()); - resp.fs.add_header("date", lgconf->time_http_str, http2::HD_DATE); + resp.fs.add_header_token(StringRef::from_lit("date"), + StringRef{lgconf->time_http_str}, false, + http2::HD_DATE); } auto upstream = downstream->get_upstream(); diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 27d768c2..dc4a5db1 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -188,10 +188,10 @@ void on_ctrl_recv_callback(spdylay_session *session, spdylay_frame_type type, } for (size_t i = 0; nv[i]; i += 2) { - auto name = std::string(nv[i]); - auto value = std::string(nv[i + 1]); - auto token = http2::lookup_token(name); - req.fs.add_header(std::move(name), std::move(value), token); + auto name = StringRef{nv[i]}; + auto value = StringRef{nv[i + 1]}; + auto token = http2::lookup_token(name.byte(), name.size()); + req.fs.add_header_token(name, value, false, token); } if (req.fs.index_headers() != 0) {