nghttpx: Response 502 if HTTP/2 backend receives invalid Content-Length

src/shrpx_downstream.cc

@@ -664,6 +664,10 @@ int64_t Downstream::get_response_sent_bodylen() const {
   return response_sent_bodylen_;
 }
 
+int64_t Downstream::get_response_content_length() const {
+  return response_content_length_;
+}
+
 void Downstream::set_response_content_length(int64_t len) {
   response_content_length_ = len;
 }

src/shrpx_downstream.h

@@ -172,7 +172,10 @@ public:
     STREAM_CLOSED,
     CONNECT_FAIL,
     IDLE,
-    MSG_RESET
+    MSG_RESET,
+    // header contains invalid header field.  We can safely send error
+    // response (502) to a client.
+    MSG_BAD_HEADER,
   };
   void set_request_state(int state);
   int get_request_state() const;
@@ -223,6 +226,7 @@ public:
   int64_t get_response_bodylen() const;
   void add_response_sent_bodylen(size_t amount);
   int64_t get_response_sent_bodylen() const;
+  int64_t get_response_content_length() const;
   void set_response_content_length(int64_t len);
   // Validates that received response body length and content-length
   // matches.

src/shrpx_http2_downstream_connection.cc

@@ -139,6 +139,7 @@ int Http2DownstreamConnection::submit_rst_stream(Downstream *downstream,
       downstream->get_downstream_stream_id() != -1) {
     switch (downstream->get_response_state()) {
     case Downstream::MSG_RESET:
+    case Downstream::MSG_BAD_HEADER:
     case Downstream::MSG_COMPLETE:
       break;
     default:

src/shrpx_http2_session.cc

@@ -654,10 +654,15 @@ int on_stream_close_callback(nghttp2_session *session, int32_t stream_id,
         downstream->get_upstream()->on_downstream_body_complete(downstream);
         downstream->set_response_state(Downstream::MSG_COMPLETE);
       } else if (error_code == NGHTTP2_NO_ERROR) {
-        if (downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+        switch (downstream->get_response_state()) {
+        case Downstream::MSG_COMPLETE:
+        case Downstream::MSG_BAD_HEADER:
+          break;
+        default:
           downstream->set_response_state(Downstream::MSG_RESET);
         }
-      } else {
+      } else if (downstream->get_response_state() !=
+                 Downstream::MSG_BAD_HEADER) {
         downstream->set_response_state(Downstream::MSG_RESET);
       }
       call_downstream_readcb(http2session, downstream);
@@ -727,6 +732,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) {
+      http2session->submit_rst_stream(frame->hd.stream_id,
+                                      NGHTTP2_PROTOCOL_ERROR);
+      downstream->set_response_state(Downstream::MSG_BAD_HEADER);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
+    auto cl = downstream->get_response_content_length();
+    if (cl != -1 && cl != len) {
+      http2session->submit_rst_stream(frame->hd.stream_id,
+                                      NGHTTP2_PROTOCOL_ERROR);
+      downstream->set_response_state(Downstream::MSG_BAD_HEADER);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
+    downstream->set_response_content_length(len);
+  }
+
   downstream->add_response_header(name, namelen, value, valuelen,
                                   flags & NGHTTP2_NV_FLAG_NO_INDEX, token);
   return 0;
@@ -817,27 +840,21 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream,
     return 0;
   }
 
-  auto content_length_hd =
-      downstream->get_response_header(http2::HD_CONTENT_LENGTH);
-
-  if (content_length_hd) {
-    auto content_length = util::parse_uint(content_length_hd->value);
-    if (content_length != -1) {
-      downstream->set_response_content_length(content_length);
-    } else if (downstream->expect_response_body()) {
-      // Here we have response body but Content-Length is not known in
-      // advance.
-      if (downstream->get_request_major() <= 0 ||
-          downstream->get_request_minor() <= 0) {
-        // We simply close connection for pre-HTTP/1.1 in this case.
-        downstream->set_response_connection_close(true);
-      } else if (downstream->get_request_method() != "CONNECT") {
-        // Otherwise, use chunked encoding to keep upstream connection
-        // open.  In HTTP2, we are supporsed not to receive
-        // transfer-encoding.
-        downstream->add_response_header("transfer-encoding", "chunked");
-        downstream->set_chunked_response(true);
-      }
+  if (downstream->get_response_content_length() == -1 &&
+      downstream->expect_response_body()) {
+    // Here we have response body but Content-Length is not known in
+    // advance.
+    if (downstream->get_request_major() <= 0 ||
+        (downstream->get_request_major() <= 1 &&
+         downstream->get_request_minor() <= 0)) {
+      // We simply close connection for pre-HTTP/1.1 in this case.
+      downstream->set_response_connection_close(true);
+    } else if (downstream->get_request_method() != "CONNECT") {
+      // Otherwise, use chunked encoding to keep upstream connection
+      // open.  In HTTP2, we are supporsed not to receive
+      // transfer-encoding.
+      downstream->add_response_header("transfer-encoding", "chunked");
+      downstream->set_chunked_response(true);
     }
   }
 

src/shrpx_http2_upstream.cc

@@ -823,6 +823,13 @@ int Http2Upstream::downstream_read(DownstreamConnection *dconn) {
     downstream->pop_downstream_connection();
     // dconn was deleted
     dconn = nullptr;
+  } else if (downstream->get_response_state() == Downstream::MSG_BAD_HEADER) {
+    if (error_reply(downstream, 502) != 0) {
+      return -1;
+    }
+    downstream->pop_downstream_connection();
+    // dconn was deleted
+    dconn = nullptr;
   } else {
     auto rv = downstream->on_read();
     if (rv == DownstreamConnection::ERR_EOF) {

src/shrpx_https_upstream.cc

@@ -460,6 +460,14 @@ int HttpsUpstream::downstream_read(DownstreamConnection *dconn) {
     return -1;
   }
 
+  if (downstream->get_response_state() == Downstream::MSG_BAD_HEADER) {
+    if (error_reply(502) != 0) {
+      return -1;
+    }
+    downstream->pop_downstream_connection();
+    goto end;
+  }
+
   if (rv == DownstreamConnection::ERR_EOF) {
     return downstream_eof(dconn);
   }
@@ -472,6 +480,7 @@ int HttpsUpstream::downstream_read(DownstreamConnection *dconn) {
     return -1;
   }
 
+end:
   handler_->signal_write();
 
   return 0;

src/shrpx_spdy_upstream.cc

@@ -531,6 +531,13 @@ int SpdyUpstream::downstream_read(DownstreamConnection *dconn) {
                    downstream->get_response_rst_stream_error_code()));
     downstream->pop_downstream_connection();
     dconn = nullptr;
+  } else if (downstream->get_response_state() == Downstream::MSG_BAD_HEADER) {
+    if (error_reply(downstream, 502) != 0) {
+      return -1;
+    }
+    downstream->pop_downstream_connection();
+    // dconn was deleted
+    dconn = nullptr;
   } else {
     auto rv = downstream->on_read();
     if (rv == DownstreamConnection::ERR_EOF) {