diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 579d4adf..d2e43d57 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1255,14 +1255,9 @@ int hd_on_frame_recv_callback } if(frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - if(!http2::check_http2_request_pseudo_headers_without_sort - (stream->headers)) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } http2::normalize_headers(stream->headers); - if(!http2::check_http2_headers(stream->headers)) { + if(!http2::check_http2_request_headers(stream->headers)) { hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); return 0; } diff --git a/src/http2.cc b/src/http2.cc index 22a73942..a0412e5e 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -255,19 +255,16 @@ bool check_pseudo_headers(const Headers& nva, InputIterator allowed_first, InputIterator allowed_last) { - bool expect_no_pseudo_header = false; // strict checking for pseudo headers. for(auto& hd : nva) { auto c = hd.name.c_str()[0]; - if(c != ':') { - expect_no_pseudo_header = true; + if(c < ':') { continue; } - // Pseudo headers must come before normal headers - if(expect_no_pseudo_header) { - return false; + if(c > ':') { + break; } auto i = allowed_first; @@ -287,14 +284,22 @@ bool check_pseudo_headers(const Headers& nva, } } // namespace -bool check_http2_request_pseudo_headers_without_sort(const Headers& nva) +bool check_http2_request_headers(const Headers& nva) { + if(!check_http2_headers(nva)) { + return false; + } + return check_pseudo_headers(nva, REQUEST_PSEUDO_HD, REQUEST_PSEUDO_HD + REQUEST_PSEUDO_HDLEN); } -bool check_http2_response_pseudo_headers_without_sort(const Headers& nva) +bool check_http2_response_headers(const Headers& nva) { + if(!check_http2_headers(nva)) { + return false; + } + return check_pseudo_headers(nva, RESPONSE_PSEUDO_HD, RESPONSE_PSEUDO_HD + RESPONSE_PSEUDO_HDLEN); } diff --git a/src/http2.h b/src/http2.h index e88ef5ed..7e1a4ff4 100644 --- a/src/http2.h +++ b/src/http2.h @@ -96,15 +96,15 @@ bool check_http2_allowed_header(const char *name); // contains such headers. bool check_http2_headers(const Headers& nva); -// Checks that |nva| only contains pseudo headers allowed in request -// and pseudo headers come before normal headers. Returns true if all +// Calls check_http2_headers() and also checks that |nva| only +// contains pseudo headers allowed in request. Returns true if all // checks passed. -bool check_http2_request_pseudo_headers_without_sort(const Headers& nva); +bool check_http2_request_headers(const Headers& nva); -// Checks that |nva| only contains pseudo headers allowed in response -// and pseudo headers come before normal headers. Returns true if all +// Calls check_http2_headers() and also checks that |nva| only +// contains pseudo headers allowed in response. Returns true if all // checks passed. -bool check_http2_response_pseudo_headers_without_sort(const Headers& nva); +bool check_http2_response_headers(const Headers& nva); bool name_less(const Headers::value_type& lhs, const Headers::value_type& rhs); diff --git a/src/http2_test.cc b/src/http2_test.cc index b2953b09..63d7074a 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -99,20 +99,14 @@ void test_http2_check_http2_headers(void) { ":path", "3" }, { ":scheme", "4" } }; - CU_ASSERT(http2::check_http2_request_pseudo_headers_without_sort(nva4)); - CU_ASSERT(!http2::check_http2_response_pseudo_headers_without_sort(nva4)); + CU_ASSERT(http2::check_http2_request_headers(nva4)); + CU_ASSERT(!http2::check_http2_response_headers(nva4)); auto nva5 = Headers{ { ":status", "1" } }; - CU_ASSERT(!http2::check_http2_request_pseudo_headers_without_sort(nva5)); - CU_ASSERT(http2::check_http2_response_pseudo_headers_without_sort(nva5)); - - auto nva6 = Headers{ - { "content-length", "1"}, - { ":authority", "2" }, - }; - CU_ASSERT(!http2::check_http2_request_pseudo_headers_without_sort(nva6)); + CU_ASSERT(!http2::check_http2_request_headers(nva5)); + CU_ASSERT(http2::check_http2_response_headers(nva5)); } void test_http2_get_unique_header(void) diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 804fcc26..9b983d7f 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -895,22 +895,12 @@ int on_response_headers(Http2Session *http2session, auto upstream = downstream->get_upstream(); - if(!http2::check_http2_response_pseudo_headers_without_sort - (downstream->get_response_headers())) { - - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - downstream->set_response_state(Downstream::MSG_RESET); - call_downstream_readcb(http2session, downstream); - return 0; - } - downstream->normalize_response_headers(); auto& nva = downstream->get_response_headers(); downstream->set_expect_final_response(false); - if(!http2::check_http2_headers(nva)) { + if(!http2::check_http2_response_headers(nva)) { http2session->submit_rst_stream(frame->hd.stream_id, NGHTTP2_PROTOCOL_ERROR); downstream->set_response_state(Downstream::MSG_RESET); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 76526082..b043b7ce 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -285,13 +285,6 @@ int on_request_headers(Http2Upstream *upstream, return 0; } - if(!http2::check_http2_request_pseudo_headers_without_sort - (downstream->get_request_headers())) { - - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - downstream->normalize_request_headers(); auto& nva = downstream->get_request_headers(); @@ -309,7 +302,7 @@ int on_request_headers(Http2Upstream *upstream, http2::dump_nv(get_config()->http2_upstream_dump_request_header, nva); } - if(!http2::check_http2_headers(nva)) { + if(!http2::check_http2_request_headers(nva)) { if(upstream->error_reply(downstream, 400) != 0) { upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); }