From 434e80dc7bfc20cc744aca941cbc516a06e5b1bd Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 24 Jan 2015 15:31:59 +0900 Subject: [PATCH] nghttpx: Reset stream if TE header field contains other than trailer --- src/http2.cc | 32 +++++++++++++++++++++ src/http2.h | 6 ++++ src/http2_test.cc | 56 +++++++++++++++++++++++++++++++++++++ src/http2_test.h | 1 + src/shrpx-unittest.cc | 2 ++ src/shrpx_http2_upstream.cc | 11 +++++++- 6 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/http2.cc b/src/http2.cc index 9ddf9bfb..0765e8a7 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -664,6 +664,38 @@ const Headers::value_type *get_header(const int *hdidx, int token, return &nva[i]; } +bool check_http2_te(const uint8_t *value, size_t valuelen) { + auto first = value; + auto last = first + valuelen; + for (;;) { + if (first == last) { + return true; + } + auto end_param = std::find(first, last, ','); + auto len = end_param - first; + if (!util::istartsWith(reinterpret_cast(first), len, + "trailer")) { + return false; + } + if (len == sizeof("trailer") - 1) { + goto next; + } + switch (first[sizeof("trailer") - 1]) { + case ' ': + case ';': + goto next; + } + return false; + next: + if (end_param == last) { + return true; + } + first = std::find_if_not(end_param + 1, last, [](uint8_t c) { + return c == ' ' || c == '\t' || c == ','; + }); + } +} + } // namespace http2 } // namespace nghttp2 diff --git a/src/http2.h b/src/http2.h index fed901ec..a3e22d87 100644 --- a/src/http2.h +++ b/src/http2.h @@ -244,6 +244,12 @@ bool http2_mandatory_request_headers_presence(const int *hdidx); const Headers::value_type *get_header(const int *hdidx, int token, const Headers &nva); +// Returns true if TE request header field value |value| of length +// |valuelen| is empty or only contains "trailers". The valid header +// field value is "trailer" and optionally followed by "," or OWS for +// simplicity. +bool check_http2_te(const uint8_t *value, size_t valuelen); + } // namespace http2 } // namespace nghttp2 diff --git a/src/http2_test.cc b/src/http2_test.cc index 9edc1bd1..c63dc3ce 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -290,4 +290,60 @@ void test_http2_mandatory_request_headers_presence(void) { CU_ASSERT(http2::http2_mandatory_request_headers_presence(hdidx)); } +void test_http2_check_http2_te(void) { + { + const uint8_t v[] = "trailer"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "Trailer"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer,"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer,,"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer, ,"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer, , "; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer; q=0.9"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer ; q=0.9"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer ; q=0.9, trailer"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailer ; q=0.9, trailer"; + CU_ASSERT(http2::check_http2_te(v, sizeof(v) - 1)); + } + // failure cases + { + const uint8_t v[] = "trailer; q=0.9, gzip"; + CU_ASSERT(!http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "traile"; + CU_ASSERT(!http2::check_http2_te(v, sizeof(v) - 1)); + } + { + const uint8_t v[] = "trailerr"; + CU_ASSERT(!http2::check_http2_te(v, sizeof(v) - 1)); + } +} + } // namespace shrpx diff --git a/src/http2_test.h b/src/http2_test.h index f8127dc7..fba64e1e 100644 --- a/src/http2_test.h +++ b/src/http2_test.h @@ -39,6 +39,7 @@ void test_http2_lookup_token(void); void test_http2_check_http2_pseudo_header(void); void test_http2_http2_header_allowed(void); void test_http2_mandatory_request_headers_presence(void); +void test_http2_check_http2_te(void); } // namespace shrpx diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 59c900f3..f7218069 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -93,6 +93,8 @@ int main(int argc, char *argv[]) { shrpx::test_http2_http2_header_allowed) || !CU_add_test(pSuite, "http2_mandatory_request_headers_presence", shrpx::test_http2_mandatory_request_headers_presence) || + !CU_add_test(pSuite, "http2_check_http2_te", + shrpx::test_http2_check_http2_te) || !CU_add_test(pSuite, "downstream_index_request_headers", shrpx::test_downstream_index_request_headers) || !CU_add_test(pSuite, "downstream_index_response_headers", diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index e92ce5c3..7bef7cb0 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -207,7 +207,8 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } - if (token == http2::HD_CONTENT_LENGTH) { + switch (token) { + case http2::HD_CONTENT_LENGTH: { auto len = util::parse_uint(value, valuelen); if (len == -1) { if (upstream->error_reply(downstream, 400) != 0) { @@ -222,6 +223,14 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } downstream->set_request_content_length(len); + break; + } + case http2::HD_TE: + if (!http2::check_http2_te(value, valuelen)) { + upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + break; } downstream->add_request_header(name, namelen, value, valuelen,