nghttpx: Limit the maximum header block size (again)

src/http2.cc

@@ -730,21 +730,6 @@ int check_nv(const uint8_t *name, size_t namelen,
   return 1;
 }
 
-int handle_too_many_headers(nghttp2_session *session, int32_t stream_id)
-{
-  int rv;
-  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id,
-                                 NGHTTP2_INTERNAL_ERROR);
-  if(nghttp2_is_fatal(rv)) {
-    return rv;
-  }
-  rv = nghttp2_session_terminate_session(session, NGHTTP2_INTERNAL_ERROR);
-  if(nghttp2_is_fatal(rv)) {
-    return rv;
-  }
-  return 0;
-}
-
 } // namespace http2
 
 } // namespace nghttp2

src/http2.h

@@ -221,10 +221,6 @@ int check_header_value(const uint8_t* value, size_t len);
 int check_nv(const uint8_t *name, size_t namelen,
              const uint8_t *value, size_t valuelen);
 
-// Handles the situation that incoming headers are too many. It is
-// dealt with by issuing RST_STREAM and GOAWAY.
-int handle_too_many_headers(nghttp2_session *session, int32_t stream_id);
-
 } // namespace http2
 
 } // namespace nghttp2

src/shrpx_downstream.cc

@@ -258,12 +258,14 @@ void Downstream::concat_norm_request_headers()
 void Downstream::add_request_header(std::string name, std::string value)
 {
   request_header_key_prev_ = true;
+  request_headers_sum_ += name.size() + value.size();
   request_headers_.emplace_back(std::move(name), std::move(value));
 }
 
 void Downstream::set_last_request_header_value(std::string value)
 {
   request_header_key_prev_ = false;
+  request_headers_sum_ += value.size();
   Headers::value_type &item = request_headers_.back();
   item.second = std::move(value);
   check_transfer_encoding_chunked(&chunked_request_, item);
@@ -274,6 +276,7 @@ void Downstream::split_add_request_header
 (const uint8_t *name, size_t namelen,
  const uint8_t *value, size_t valuelen)
 {
+  request_headers_sum_ += namelen + valuelen;
   http2::split_add_header(request_headers_, name, namelen, value, valuelen);
 }
 
@@ -285,6 +288,7 @@ bool Downstream::get_request_header_key_prev() const
 void Downstream::append_last_request_header_key(const char *data, size_t len)
 {
   assert(request_header_key_prev_);
+  request_headers_sum_ += len;
   auto& item = request_headers_.back();
   item.first.append(data, len);
 }
@@ -292,10 +296,16 @@ void Downstream::append_last_request_header_key(const char *data, size_t len)
 void Downstream::append_last_request_header_value(const char *data, size_t len)
 {
   assert(!request_header_key_prev_);
+  request_headers_sum_ += len;
   auto& item = request_headers_.back();
   item.second.append(data, len);
 }
 
+size_t Downstream::get_request_headers_sum() const
+{
+  return request_headers_sum_;
+}
+
 void Downstream::set_request_method(std::string method)
 {
   request_method_ = std::move(method);
@@ -512,6 +522,7 @@ void Downstream::rewrite_norm_location_response_header
 void Downstream::add_response_header(std::string name, std::string value)
 {
   response_header_key_prev_ = true;
+  response_headers_sum_ += name.size() + value.size();
   response_headers_.emplace_back(std::move(name), std::move(value));
   check_transfer_encoding_chunked(&chunked_response_,
                                   response_headers_.back());
@@ -520,6 +531,7 @@ void Downstream::add_response_header(std::string name, std::string value)
 void Downstream::set_last_response_header_value(std::string value)
 {
   response_header_key_prev_ = false;
+  response_headers_sum_ += value.size();
   auto& item = response_headers_.back();
   item.second = std::move(value);
   check_transfer_encoding_chunked(&chunked_response_, item);
@@ -529,6 +541,7 @@ void Downstream::split_add_response_header
 (const uint8_t *name, size_t namelen,
  const uint8_t *value, size_t valuelen)
 {
+  response_headers_sum_ += namelen + valuelen;
   http2::split_add_header(response_headers_, name, namelen, value, valuelen);
 }
 
@@ -540,6 +553,7 @@ bool Downstream::get_response_header_key_prev() const
 void Downstream::append_last_response_header_key(const char *data, size_t len)
 {
   assert(response_header_key_prev_);
+  response_headers_sum_ += len;
   auto& item = response_headers_.back();
   item.first.append(data, len);
 }
@@ -548,10 +562,16 @@ void Downstream::append_last_response_header_value(const char *data,
                                                    size_t len)
 {
   assert(!response_header_key_prev_);
+  response_headers_sum_ += len;
   auto& item = response_headers_.back();
   item.second.append(data, len);
 }
 
+size_t Downstream::get_response_headers_sum() const
+{
+  return response_headers_sum_;
+}
+
 unsigned int Downstream::get_response_http_status() const
 {
   return response_http_status_;

src/shrpx_downstream.h

@@ -109,6 +109,8 @@ public:
   void append_last_request_header_key(const char *data, size_t len);
   void append_last_request_header_value(const char *data, size_t len);
 
+  size_t get_request_headers_sum() const;
+
   void set_request_method(std::string method);
   const std::string& get_request_method() const;
   void set_request_path(std::string path);
@@ -175,6 +177,8 @@ public:
   void append_last_response_header_key(const char *data, size_t len);
   void append_last_response_header_value(const char *data, size_t len);
 
+  size_t get_response_headers_sum() const;
+
   unsigned int get_response_http_status() const;
   void set_response_http_status(unsigned int status);
   void set_response_major(int major);
@@ -200,9 +204,8 @@ public:
   // Change the priority of downstream
   int change_priority(int32_t pri);
 
-  // Maximum number of headers per HEADERS frame, including its
+  // Maximum buffer size for header name/value pairs.
-  // following CONTINUATIONs.
+  static const size_t MAX_HEADERS_SUM = 32768;
-  static const size_t MAX_HEADERS = 128;
 private:
   Headers request_headers_;
   Headers response_headers_;
@@ -221,6 +224,9 @@ private:
   // body. nghttp2 library reads data from this in the callback.
   evbuffer *response_body_buf_;
 
+  size_t request_headers_sum_;
+  size_t response_headers_sum_;
+
   int32_t stream_id_;
   int32_t priority_;
   // stream ID in backend connection

src/shrpx_http2_session.cc

@@ -804,7 +804,6 @@ int on_header_callback(nghttp2_session *session,
                        const uint8_t *value, size_t valuelen,
                        void *user_data)
 {
-  int rv;
   if(frame->hd.type != NGHTTP2_HEADERS ||
      frame->headers.cat != NGHTTP2_HCAT_RESPONSE) {
     return 0;
@@ -818,12 +817,12 @@ int on_header_callback(nghttp2_session *session,
   if(!downstream) {
     return 0;
   }
-  if(downstream->get_request_headers().size() >= Downstream::MAX_HEADERS) {
+  if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) {
-    rv = http2::handle_too_many_headers(session, frame->hd.stream_id);
+    if(LOG_ENABLED(INFO)) {
-    if(nghttp2_is_fatal(rv)) {
+      DLOG(INFO, downstream) << "Too large header block size="
-      return rv;
+                             << downstream->get_response_headers_sum();
     }
-    return 0;
+    return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
   }
   if(!http2::check_nv(name, namelen, value, valuelen)) {
     return 0;

src/shrpx_http2_upstream.cc

@@ -205,7 +205,6 @@ int on_header_callback(nghttp2_session *session,
                        const uint8_t *value, size_t valuelen,
                        void *user_data)
 {
-  int rv;
   if(frame->hd.type != NGHTTP2_HEADERS ||
      frame->headers.cat != NGHTTP2_HCAT_REQUEST) {
     return 0;
@@ -215,12 +214,12 @@ int on_header_callback(nghttp2_session *session,
   if(!downstream) {
     return 0;
   }
-  if(downstream->get_request_headers().size() >= Downstream::MAX_HEADERS) {
+  if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) {
-    rv = http2::handle_too_many_headers(session, frame->hd.stream_id);
+    if(LOG_ENABLED(INFO)) {
-    if(nghttp2_is_fatal(rv)) {
+      ULOG(INFO, upstream) << "Too large header block size="
-      return rv;
+                           << downstream->get_request_headers_sum();
     }
-    return 0;
+    return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
   }
   if(!http2::check_nv(name, namelen, value, valuelen)) {
     return 0;

src/shrpx_http_downstream_connection.cc

@@ -431,6 +431,13 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len)
   } else {
     downstream->add_response_header(std::string(data, len), "");
   }
+  if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) {
+    if(LOG_ENABLED(INFO)) {
+      DLOG(INFO, downstream) << "Too large header block size="
+                             << downstream->get_response_headers_sum();
+    }
+    return -1;
+  }
   return 0;
 }
 } // namespace
@@ -444,6 +451,13 @@ int htp_hdr_valcb(http_parser *htp, const char *data, size_t len)
   } else {
     downstream->append_last_response_header_value(data, len);
   }
+  if(downstream->get_response_headers_sum() > Downstream::MAX_HEADERS_SUM) {
+    if(LOG_ENABLED(INFO)) {
+      DLOG(INFO, downstream) << "Too large header block size="
+                             << downstream->get_response_headers_sum();
+    }
+    return -1;
+  }
   return 0;
 }
 } // namespace

src/shrpx_https_upstream.cc

@@ -45,7 +45,6 @@ namespace shrpx {
 
 namespace {
 const size_t OUTBUF_MAX_THRES = 64*1024;
-const size_t SHRPX_HTTPS_MAX_HEADER_LENGTH = 64*1024;
 } // namespace
 
 HttpsUpstream::HttpsUpstream(ClientHandler *handler)
@@ -102,6 +101,13 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len)
   } else {
     downstream->add_request_header(std::string(data, len), "");
   }
+  if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) {
+    if(LOG_ENABLED(INFO)) {
+      ULOG(INFO, upstream) << "Too large header block size="
+                           << downstream->get_request_headers_sum();
+    }
+    return -1;
+  }
   return 0;
 }
 } // namespace
@@ -116,6 +122,13 @@ int htp_hdr_valcb(http_parser *htp, const char *data, size_t len)
   } else {
     downstream->append_last_request_header_value(data, len);
   }
+  if(downstream->get_request_headers_sum() > Downstream::MAX_HEADERS_SUM) {
+    if(LOG_ENABLED(INFO)) {
+      ULOG(INFO, upstream) << "Too large header block size="
+                           << downstream->get_request_headers_sum();
+    }
+    return -1;
+  }
   return 0;
 }
 } // namespace
@@ -321,17 +334,7 @@ int HttpsUpstream::on_read()
   } else if(htperr == HPE_OK) {
     // downstream can be NULL here.
     if(downstream) {
-      if(downstream->get_request_state() == Downstream::INITIAL &&
+      if(downstream->get_output_buffer_full()) {
-         current_header_length_ > SHRPX_HTTPS_MAX_HEADER_LENGTH) {
-        ULOG(WARNING, this) << "Request Header too long:"
-                            << current_header_length_
-                            << " bytes";
-        handler->set_should_close_after_write(true);
-        pause_read(SHRPX_MSG_BLOCK);
-        if(error_reply(400) != 0) {
-          return -1;
-        }
-      } else if(downstream->get_output_buffer_full()) {
         if(LOG_ENABLED(INFO)) {
           ULOG(INFO, this) << "Downstream output buffer is full";
         }