From 8004ea972665b2d225dd67364dd1c342c58d2615 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 19 Jan 2015 22:40:37 +0900 Subject: [PATCH] nghttpx: Return 400 if request CL is invalid or multiple CLs --- integration-tests/nghttpx_test.go | 39 +++++++++++++++++++++++++++++++ src/shrpx_downstream.cc | 16 ++++++------- src/shrpx_downstream.h | 2 ++ src/shrpx_http2_upstream.cc | 18 ++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/integration-tests/nghttpx_test.go b/integration-tests/nghttpx_test.go index 2a676985..33212668 100644 --- a/integration-tests/nghttpx_test.go +++ b/integration-tests/nghttpx_test.go @@ -201,3 +201,42 @@ func TestHTTP2ChunkedRequestBody(t *testing.T) { t.Errorf("Error st.http2() = %v", err) } } + +func TestHTTP2DuplicateRequestCL(t *testing.T) { + st := newServerTester(nil, t, noopHandler) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestHTTP2DuplicateRequestCL", + header: []hpack.HeaderField{ + pair("content-length", "1"), + pair("content-length", "2"), + }, + }) + if err != nil { + t.Errorf("Error st.http2() = %v", err) + } + want := 400 + if got := res.status; got != want { + t.Errorf("status: %v; want %v", got, want) + } +} + +func TestHTTP2InvalidRequestCL(t *testing.T) { + st := newServerTester(nil, t, noopHandler) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestHTTP2InvalidRequestCL", + header: []hpack.HeaderField{ + pair("content-length", ""), + }, + }) + if err != nil { + t.Errorf("Error st.http2() = %v", err) + } + want := 400 + if got := res.status; got != want { + t.Errorf("status: %v; want %v", got, want) + } +} diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 0ec001ce..e3adb3c9 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -668,6 +668,14 @@ void Downstream::set_response_content_length(int64_t len) { response_content_length_ = len; } +int64_t Downstream::get_request_content_length() const { + return request_content_length_; +} + +void Downstream::set_request_content_length(int64_t len) { + request_content_length_ = len; +} + bool Downstream::validate_request_bodylen() const { if (request_content_length_ == -1) { return true; @@ -725,14 +733,6 @@ void Downstream::inspect_http2_request() { if (request_method_ == "CONNECT") { upgrade_request_ = true; } - - auto idx = request_hdidx_[http2::HD_CONTENT_LENGTH]; - if (idx != -1) { - auto len = util::parse_uint(request_headers_[idx].value); - if (len != -1) { - request_content_length_ = len; - } - } } void Downstream::inspect_http1_request() { diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 7fbeb372..ecc8c462 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -161,6 +161,8 @@ public: // Validates that received request body length and content-length // matches. bool validate_request_bodylen() const; + int64_t get_request_content_length() const; + void set_request_content_length(int64_t len); bool request_pseudo_header_allowed(int token) const; bool expect_response_body() const; enum { diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 61a8314c..c694224c 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -198,6 +198,24 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } + if (token == http2::HD_CONTENT_LENGTH) { + auto len = util::parse_uint(value, valuelen); + if (len == -1) { + if (upstream->error_reply(downstream, 400) != 0) { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + return 0; + } + auto cl = downstream->get_request_content_length(); + if (cl != -1 && cl != len) { + if (upstream->error_reply(downstream, 400) != 0) { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + return 0; + } + downstream->set_request_content_length(len); + } + downstream->add_request_header(name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0;