From 6f1347fc8b19b6419d19a55587651330fe02116b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 20 Feb 2016 19:13:39 +0900 Subject: [PATCH] nghttpx: Tokenize trailer field as well so that we can ditch prohibited headers in HTTP/2 --- src/shrpx_downstream.cc | 31 ++++++++++++++----------- src/shrpx_downstream.h | 4 ++-- src/shrpx_http2_session.cc | 6 ++--- src/shrpx_http2_upstream.cc | 6 ++--- src/shrpx_http_downstream_connection.cc | 4 ++-- src/shrpx_https_upstream.cc | 4 ++-- 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 71a0a2b6..bd76592e 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -330,7 +330,7 @@ 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 = -1) { + std::string value, int16_t token) { key_prev = true; sum += name.size() + value.size(); headers.emplace_back(std::move(name), std::move(value), false, token); @@ -415,11 +415,13 @@ const Headers::value_type *FieldStore::header(const StringRef &name) const { return search_header_linear_backwards(headers_, name); } -void FieldStore::add_header_lower(std::string name, std::string value) { - util::inp_strlower(name); - auto token = http2::lookup_token(name); - shrpx::add_header(header_key_prev_, buffer_size_, headers_, std::move(name), - std::move(value), token); +void FieldStore::add_header_lower(const StringRef &name, + const StringRef &value) { + 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); } void FieldStore::add_header(std::string name, std::string value, @@ -450,16 +452,19 @@ void FieldStore::clear_headers() { headers_.clear(); } void FieldStore::add_trailer(const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen, bool no_index, int16_t token) { - // we never index trailer fields. Header size limit should be - // applied to all header and trailer fields combined. + // Header size limit should be applied to all header and trailer + // fields combined. shrpx::add_header(buffer_size_, trailers_, name, namelen, value, valuelen, - no_index, -1); + no_index, token); } -void FieldStore::add_trailer_lower(std::string name, std::string value) { - util::inp_strlower(name); - shrpx::add_header(trailer_key_prev_, buffer_size_, trailers_, std::move(name), - std::move(value)); +void FieldStore::add_trailer_lower(const StringRef &name, + const StringRef &value) { + 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); } 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 4132e905..03286b83 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -79,7 +79,7 @@ public: // such header is found, returns nullptr. const Headers::value_type *header(const StringRef &name) const; - void add_header_lower(std::string name, std::string value); + 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 uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen, bool no_index, int16_t token); @@ -99,7 +99,7 @@ public: void add_trailer(const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen, bool no_index, int16_t token); - void add_trailer_lower(std::string name, std::string value); + void add_trailer_lower(const StringRef &name, const StringRef &value); 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_http2_session.cc b/src/shrpx_http2_session.cc index f4004b9a..24283172 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -735,15 +735,15 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } + auto token = http2::lookup_token(name, namelen); + if (trailer) { // just store header fields for trailer part resp.fs.add_trailer(name, namelen, value, valuelen, - flags & NGHTTP2_NV_FLAG_NO_INDEX, -1); + flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } - auto token = http2::lookup_token(name, namelen); - resp.fs.add_header(name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 7e909a25..f13ea759 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -201,15 +201,15 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } + auto token = http2::lookup_token(name, namelen); + if (frame->headers.cat == NGHTTP2_HCAT_HEADERS) { // just store header fields for trailer part req.fs.add_trailer(name, namelen, value, valuelen, - flags & NGHTTP2_NV_FLAG_NO_INDEX, -1); + flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } - auto token = http2::lookup_token(name, namelen); - req.fs.add_header(name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 8b96e314..0af56ce1 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(std::string(data, len), ""); + resp.fs.add_header_lower(StringRef{data, len}, StringRef{}); } } 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(std::string(data, len), ""); + resp.fs.add_trailer_lower(StringRef(data, len), StringRef{}); } } return 0; diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 5875ac1b..7f47b681 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(std::string(data, len), ""); + req.fs.add_header_lower(StringRef{data, len}, StringRef{}); } } 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(std::string(data, len), ""); + req.fs.add_trailer_lower(StringRef{data, len}, StringRef{}); } } return 0;