Merge pull request #1 from nghttp2/master

Merge from nghttp2/nghttp2

gennghttpxfun.py

@@ -152,6 +152,8 @@ LOGVARS = [
     "ssl_protocol",
     "ssl_session_id",
     "ssl_session_reused",
+    "backend_host",
+    "backend_port",
 ]
 
 if __name__ == '__main__':

lib/includes/nghttp2/nghttp2.h

@@ -668,6 +668,10 @@ typedef enum {
 /**
  * @macro
  *
+ * .. warning::
+ *
+ *   Deprecated.  The initial max concurrent streams is 0xffffffffu.
+ *
  * Default maximum number of incoming concurrent streams.  Use
  * `nghttp2_submit_settings()` with
  * :enum:`NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS` to change the

lib/nghttp2_hd.c

@@ -532,7 +532,7 @@ static void hd_map_insert(nghttp2_hd_map *map, nghttp2_hd_entry *ent) {
 
 static nghttp2_hd_entry *hd_map_find(nghttp2_hd_map *map, int *exact_match,
                                      const nghttp2_nv *nv, int32_t token,
-                                     uint32_t hash) {
+                                     uint32_t hash, int name_only) {
   nghttp2_hd_entry *p;
   nghttp2_hd_entry *res = NULL;
 
@@ -545,6 +545,9 @@ static nghttp2_hd_entry *hd_map_find(nghttp2_hd_map *map, int *exact_match,
     }
     if (!res) {
       res = p;
+      if (name_only) {
+        break;
+      }
     }
     if (value_eq(&p->nv, nv)) {
       res = p;
@@ -1148,16 +1151,16 @@ static int add_hd_table_incremental(nghttp2_hd_context *context,
 typedef struct {
   ssize_t index;
   /* Nonzero if both name and value are matched. */
-  uint8_t name_value_match;
+  int name_value_match;
 } search_result;
 
 static search_result search_static_table(const nghttp2_nv *nv, int32_t token,
-                                         int indexing_mode) {
+                                         int name_only) {
   search_result res = {token, 0};
   int i;
   nghttp2_hd_static_entry *ent;
 
-  if (indexing_mode == NGHTTP2_HD_NEVER_INDEXING) {
+  if (name_only) {
     return res;
   }
 
@@ -1182,30 +1185,22 @@ static search_result search_hd_table(nghttp2_hd_context *context,
   search_result res = {-1, 0};
   nghttp2_hd_entry *ent;
   int exact_match;
-
+  int name_only = indexing_mode == NGHTTP2_HD_NEVER_INDEXING;
-  if (token >= 0 && token <= NGHTTP2_TOKEN_WWW_AUTHENTICATE) {
-    res = search_static_table(nv, token, indexing_mode);
-    if (res.name_value_match) {
-      return res;
-    }
-  }
 
   exact_match = 0;
-  ent = hd_map_find(map, &exact_match, nv, token, hash);
+  ent = hd_map_find(map, &exact_match, nv, token, hash, name_only);
-  if (ent == NULL) {
+
-    return res;
+  if (!exact_match && token >= 0 && token <= NGHTTP2_TOKEN_WWW_AUTHENTICATE) {
+    return search_static_table(nv, token, name_only);
   }
 
-  if (res.index != -1 && !exact_match) {
+  if (ent == NULL) {
     return res;
   }
 
   res.index =
       (ssize_t)(context->next_seq - 1 - ent->seq + NGHTTP2_STATIC_TABLE_LENGTH);
-
+  res.name_value_match = exact_match;
-  if (exact_match) {
-    res.name_value_match = 1;
-  }
 
   return res;
 }

lib/nghttp2_session.c

@@ -365,7 +365,7 @@ static void session_inbound_frame_reset(nghttp2_session *session) {
 static void init_settings(nghttp2_settings_storage *settings) {
   settings->header_table_size = NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE;
   settings->enable_push = 1;
-  settings->max_concurrent_streams = NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS;
+  settings->max_concurrent_streams = NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS;
   settings->initial_window_size = NGHTTP2_INITIAL_WINDOW_SIZE;
   settings->max_frame_size = NGHTTP2_MAX_FRAME_SIZE_MIN;
   settings->max_header_list_size = UINT32_MAX;
@@ -435,7 +435,7 @@ static int session_new(nghttp2_session **session_ptr,
   (*session_ptr)->remote_last_stream_id = (1u << 31) - 1;
 
   (*session_ptr)->pending_local_max_concurrent_stream =
-      NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS;
+      NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS;
   (*session_ptr)->pending_enable_push = 1;
 
   if (server) {
@@ -772,7 +772,7 @@ int nghttp2_session_reprioritize_stream(
                    stream->stream_id));
 
     nghttp2_stream_dep_remove_subtree(dep_stream);
-    rv = nghttp2_stream_dep_add_subtree(&session->root, dep_stream);
+    rv = nghttp2_stream_dep_add_subtree(stream->dep_prev, dep_stream);
     if (rv != 0) {
       return rv;
     }
@@ -1185,14 +1185,12 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id,
        combined with the current active incoming streams to make
        dependency tree work better. */
     nghttp2_session_keep_closed_stream(session, stream);
-
-    rv = nghttp2_session_adjust_closed_stream(session);
   } else {
     rv = nghttp2_session_destroy_stream(session, stream);
-  }
     if (rv != 0) {
       return rv;
     }
+  }
 
   return 0;
 }
@@ -1285,8 +1283,12 @@ int nghttp2_session_adjust_closed_stream(nghttp2_session *session) {
   size_t num_stream_max;
   int rv;
 
-  num_stream_max = nghttp2_min(session->local_settings.max_concurrent_streams,
+  if (session->local_settings.max_concurrent_streams ==
-                               session->pending_local_max_concurrent_stream);
+      NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS) {
+    num_stream_max = session->pending_local_max_concurrent_stream;
+  } else {
+    num_stream_max = session->local_settings.max_concurrent_streams;
+  }
 
   DEBUGF(fprintf(stderr, "stream: adjusting kept closed streams "
                          "num_closed_streams=%zu, num_incoming_streams=%zu, "
@@ -3777,13 +3779,6 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session,
   assert(session->server);
 
   if (!session_is_new_peer_stream_id(session, frame->hd.stream_id)) {
-    /* The spec says if an endpoint receives a HEADERS with invalid
-       stream ID, it MUST issue connection error with error code
-       PROTOCOL_ERROR.  But we could get trailer HEADERS after we have
-       sent RST_STREAM to this stream and peer have not received it.
-       Then connection error is too harsh.  It means that we only use
-       connection error if stream ID refers idle stream.  Therwise we
-       just ignore HEADERS for now. */
     if (frame->hd.stream_id == 0 ||
         nghttp2_session_is_my_stream_id(session, frame->hd.stream_id)) {
       return session_inflate_handle_invalid_connection(
@@ -3791,8 +3786,67 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session,
           "request HEADERS: invalid stream_id");
     }
 
+    /* RFC 7540 says if an endpoint receives a HEADERS with invalid
+     * stream ID (e.g, numerically smaller than previous), it MUST
+     * issue connection error with error code PROTOCOL_ERROR.  It is a
+     * bit hard to detect this, since we cannot remember all streams
+     * we observed so far.
+     *
+     * You might imagine this is really easy.  But no.  HTTP/2 is
+     * asynchronous protocol, and usually client and server do not
+     * share the complete picture of open/closed stream status.  For
+     * example, after server sends RST_STREAM for a stream, client may
+     * send trailer HEADERS for that stream.  If naive server detects
+     * that, and issued connection error, then it is a bug of server
+     * implementation since client is not wrong if it did not get
+     * RST_STREAM when it issued trailer HEADERS.
+     *
+     * For server session, we remember closed streams as long as the
+     * sum of closed streams and opened streams are under current max
+     * concurrent streams.  We can use these closed streams to detect
+     * the error in some cases.
+     *
+     * If the stream cannot be found in either closed or opened
+     * streams, it is considered to be closed, or it has not exist
+     * (e.g., peer skipped sending the stream).  Actually, it is
+     * impossible to detect which is which, since that information was
+     * lost forever.  For these cases, we send back GOAWAY with
+     * PROTOCOL_ERROR.
+     *
+     * If the stream is found, and we know that it is in half closed
+     * (remote), or closed by peer's explicit action (e.g., received
+     * RST_STREAM from peer, or peer sends HEADERS/DATA frame with
+     * END_STREAM), getting new frame on that stream is clearly error.
+     * In this case, we send GOAWAY with error code STREAM_CLOSED.
+     *
+     * There is one corner case here.  Server can change the max
+     * concurrent streams.  The initial value of max concurrent
+     * streams is unlimited (NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS,
+     * which is UINT32_MAX).  When sending out SETTINGS with
+     * MAX_CONCURRENT_STREAMS, we save its value as pending max
+     * concurrent streams, and use it as a cap to remember closed
+     * stream to save memory.  This means that we might not sure that
+     * stream surely closed or has not exist when it is not found in
+     * closed or opened stream.  To workaround this issue, we ignore
+     * incoming frame if the current max concurrent streams is
+     * NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS, and pending max
+     * concurrent streams is less than that.
+     */
     stream = nghttp2_session_get_stream_raw(session, frame->hd.stream_id);
-    if (stream && (stream->shut_flags & NGHTTP2_SHUT_RD)) {
+
+    if (!stream) {
+      if (session->local_settings.max_concurrent_streams ==
+              NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS &&
+          session->pending_local_max_concurrent_stream <
+              NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS) {
+        return NGHTTP2_ERR_IGN_HEADER_BLOCK;
+      }
+
+      return session_inflate_handle_invalid_connection(
+          session, frame, NGHTTP2_ERR_PROTO, "HEADERS: stream does not exist");
+    }
+
+    if (stream->shut_flags & NGHTTP2_SHUT_RD) {
       return session_inflate_handle_invalid_connection(
           session, frame, NGHTTP2_ERR_STREAM_CLOSED, "HEADERS: stream closed");
     }
@@ -5066,7 +5120,25 @@ static int session_on_data_received_fail_fast(nghttp2_session *session) {
   stream = nghttp2_session_get_stream(session, stream_id);
   if (!stream) {
     stream = nghttp2_session_get_stream_raw(session, stream_id);
-    if (stream && (stream->shut_flags & NGHTTP2_SHUT_RD)) {
+
+    if (!stream) {
+      if (session->server) {
+        if (session->local_settings.max_concurrent_streams ==
+                NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS &&
+            session->pending_local_max_concurrent_stream <
+                NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS) {
+          return NGHTTP2_ERR_IGN_PAYLOAD;
+        }
+
+        failure_reason = "DATA: stream does not exist";
+        error_code = NGHTTP2_PROTOCOL_ERROR;
+        goto fail;
+      }
+
+      return NGHTTP2_ERR_IGN_PAYLOAD;
+    }
+
+    if (stream->shut_flags & NGHTTP2_SHUT_RD) {
       failure_reason = "DATA: stream closed";
       error_code = NGHTTP2_STREAM_CLOSED;
       goto fail;

lib/nghttp2_session.h

@@ -97,6 +97,9 @@ typedef struct {
    these frames in this number, it is considered suspicious. */
 #define NGHTTP2_MAX_OBQ_FLOOD_ITEM 10000
 
+/* The default value of maximum number of concurrent streams. */
+#define NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS 0xffffffffu
+
 /* Internal state when receiving incoming frame */
 typedef enum {
   /* Receiving frame header */

src/shrpx.cc

@@ -2082,6 +2082,10 @@ Logging:
               * $ssl_session_id: session ID for SSL/TLS connection.
               * $ssl_session_reused:  "r"   if  SSL/TLS   session  was
                 reused.  Otherwise, "."
+              * $backend_host:  backend  host   used  to  fulfill  the
+                request.  "-" if backend host is not available.
+              * $backend_port:  backend  port   used  to  fulfill  the
+                request.  "-" if backend host is not available.
 
               The  variable  can  be  enclosed  by  "{"  and  "}"  for
               disambiguation (e.g., ${remote_addr}).

src/shrpx_api_downstream_connection.cc

@@ -301,9 +301,10 @@ void APIDownstreamConnection::on_upstream_change(Upstream *uptream) {}
 
 bool APIDownstreamConnection::poolable() const { return false; }
 
-DownstreamAddrGroup *
+const std::shared_ptr<DownstreamAddrGroup> &
 APIDownstreamConnection::get_downstream_addr_group() const {
-  return nullptr;
+  static std::shared_ptr<DownstreamAddrGroup> s;
+  return s;
 }
 
 DownstreamAddr *APIDownstreamConnection::get_addr() const { return nullptr; }

src/shrpx_api_downstream_connection.h

@@ -54,7 +54,8 @@ public:
   // true if this object is poolable.
   virtual bool poolable() const;
 
-  virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual const std::shared_ptr<DownstreamAddrGroup> &
+  get_downstream_addr_group() const;
   virtual DownstreamAddr *get_addr() const;
 
   int send_reply(unsigned int http_status, int api_status);

src/shrpx_client_handler.cc

@@ -643,7 +643,7 @@ void ClientHandler::pool_downstream_connection(
 
   dconn->set_client_handler(nullptr);
 
-  auto group = dconn->get_downstream_addr_group();
+  auto &group = dconn->get_downstream_addr_group();
 
   if (LOG_ENABLED(INFO)) {
     CLOG(INFO, this) << "Pooling downstream connection DCONN:" << dconn.get()
@@ -1143,7 +1143,8 @@ void ClientHandler::write_accesslog(Downstream *downstream) {
   upstream_accesslog(
       get_config()->logging.access.format,
       LogSpec{
-          downstream, StringRef{ipaddr_}, http2::to_method_string(req.method),
+          downstream, downstream->get_addr(), StringRef{ipaddr_},
+          http2::to_method_string(req.method),
 
           req.method == HTTP_CONNECT
               ? StringRef(req.authority)
@@ -1176,7 +1177,7 @@ void ClientHandler::write_accesslog(int major, int minor, unsigned int status,
 
   upstream_accesslog(get_config()->logging.access.format,
                      LogSpec{
-                         nullptr, StringRef(ipaddr_),
+                         nullptr, nullptr, StringRef(ipaddr_),
                          StringRef::from_lit("-"), // method
                          StringRef::from_lit("-"), // path,
                          StringRef(alpn_), nghttp2::ssl::get_tls_session_info(

src/shrpx_config.cc

@@ -434,6 +434,14 @@ LogFragmentType log_var_lookup_token(const char *name, size_t namelen) {
         return SHRPX_LOGF_SSL_PROTOCOL;
       }
       break;
+    case 't':
+      if (util::strieq_l("backend_hos", name, 11)) {
+        return SHRPX_LOGF_BACKEND_HOST;
+      }
+      if (util::strieq_l("backend_por", name, 11)) {
+        return SHRPX_LOGF_BACKEND_PORT;
+      }
+      break;
     }
     break;
   case 14:

src/shrpx_downstream.cc

@@ -124,6 +124,7 @@ Downstream::Downstream(Upstream *upstream, MemchunkPool *mcpool,
       response_buf_(mcpool),
       upstream_(upstream),
       blocked_link_(nullptr),
+      addr_(nullptr),
       num_retry_(0),
       stream_id_(stream_id),
       assoc_stream_id_(-1),
@@ -943,4 +944,13 @@ void Downstream::add_rcbuf(nghttp2_rcbuf *rcbuf) {
   rcbufs_.push_back(rcbuf);
 }
 
+void Downstream::set_downstream_addr_group(
+    const std::shared_ptr<DownstreamAddrGroup> &group) {
+  group_ = group;
+}
+
+void Downstream::set_addr(const DownstreamAddr *addr) { addr_ = addr; }
+
+const DownstreamAddr *Downstream::get_addr() const { return addr_; }
+
 } // namespace shrpx

src/shrpx_downstream.h

@@ -49,6 +49,8 @@ namespace shrpx {
 class Upstream;
 class DownstreamConnection;
 struct BlockedLink;
+struct DownstreamAddrGroup;
+struct DownstreamAddr;
 
 class FieldStore {
 public:
@@ -382,6 +384,12 @@ public:
 
   void add_rcbuf(nghttp2_rcbuf *rcbuf);
 
+  void
+  set_downstream_addr_group(const std::shared_ptr<DownstreamAddrGroup> &group);
+  void set_addr(const DownstreamAddr *addr);
+
+  const DownstreamAddr *get_addr() const;
+
   enum {
     EVENT_ERROR = 0x1,
     EVENT_TIMEOUT = 0x2,
@@ -429,6 +437,10 @@ private:
 
   // only used by HTTP/2 or SPDY upstream
   BlockedLink *blocked_link_;
+  // The backend address used to fulfill this request.  These are for
+  // logging purpose.
+  std::shared_ptr<DownstreamAddrGroup> group_;
+  const DownstreamAddr *addr_;
   // How many times we tried in backend connection
   size_t num_retry_;
   // The stream ID in frontend connection

src/shrpx_downstream_connection.h

@@ -61,7 +61,8 @@ public:
   // true if this object is poolable.
   virtual bool poolable() const = 0;
 
-  virtual DownstreamAddrGroup *get_downstream_addr_group() const = 0;
+  virtual const std::shared_ptr<DownstreamAddrGroup> &
+  get_downstream_addr_group() const = 0;
   virtual DownstreamAddr *get_addr() const = 0;
 
   void set_client_handler(ClientHandler *client_handler);

src/shrpx_health_monitor_downstream_connection.cc

@@ -95,9 +95,10 @@ void HealthMonitorDownstreamConnection::on_upstream_change(Upstream *uptream) {}
 
 bool HealthMonitorDownstreamConnection::poolable() const { return false; }
 
-DownstreamAddrGroup *
+const std::shared_ptr<DownstreamAddrGroup> &
 HealthMonitorDownstreamConnection::get_downstream_addr_group() const {
-  return nullptr;
+  static std::shared_ptr<DownstreamAddrGroup> s;
+  return s;
 }
 
 DownstreamAddr *HealthMonitorDownstreamConnection::get_addr() const {

src/shrpx_health_monitor_downstream_connection.h

@@ -54,7 +54,8 @@ public:
   // true if this object is poolable.
   virtual bool poolable() const;
 
-  virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual const std::shared_ptr<DownstreamAddrGroup> &
+  get_downstream_addr_group() const;
   virtual DownstreamAddr *get_addr() const;
 };
 

src/shrpx_http2_downstream_connection.cc

@@ -538,7 +538,7 @@ int Http2DownstreamConnection::on_timeout() {
   return submit_rst_stream(downstream_, NGHTTP2_NO_ERROR);
 }
 
-DownstreamAddrGroup *
+const std::shared_ptr<DownstreamAddrGroup> &
 Http2DownstreamConnection::get_downstream_addr_group() const {
   return http2session_->get_downstream_addr_group();
 }

src/shrpx_http2_downstream_connection.h

@@ -64,7 +64,8 @@ public:
   // migrate to another Http2Session object.
   virtual bool poolable() const { return false; }
 
-  virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual const std::shared_ptr<DownstreamAddrGroup> &
+  get_downstream_addr_group() const;
   virtual DownstreamAddr *get_addr() const;
 
   int send();

src/shrpx_http2_session.cc

@@ -976,6 +976,10 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream,
   resp.http_major = 2;
   resp.http_minor = 0;
 
+  downstream->set_downstream_addr_group(
+      http2session->get_downstream_addr_group());
+  downstream->set_addr(http2session->get_addr());
+
   if (LOG_ENABLED(INFO)) {
     std::stringstream ss;
     for (auto &nv : nva) {
@@ -2126,8 +2130,9 @@ bool Http2Session::max_concurrency_reached(size_t extra) const {
                  session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
 }
 
-DownstreamAddrGroup *Http2Session::get_downstream_addr_group() const {
+const std::shared_ptr<DownstreamAddrGroup> &
-  return group_.get();
+Http2Session::get_downstream_addr_group() const {
+  return group_;
 }
 
 void Http2Session::add_to_avail_freelist() {

src/shrpx_http2_session.h

@@ -162,7 +162,7 @@ public:
 
   DownstreamAddr *get_addr() const;
 
-  DownstreamAddrGroup *get_downstream_addr_group() const;
+  const std::shared_ptr<DownstreamAddrGroup> &get_downstream_addr_group() const;
 
   int handle_downstream_push_promise(Downstream *downstream,
                                      int32_t promised_stream_id);

src/shrpx_http_downstream_connection.cc

@@ -565,7 +565,7 @@ int HttpDownstreamConnection::end_upload_data() {
 
 namespace {
 void remove_from_pool(HttpDownstreamConnection *dconn) {
-  auto group = dconn->get_downstream_addr_group();
+  auto &group = dconn->get_downstream_addr_group();
   auto &shared_addr = group->shared_addr;
 
   if (shared_addr->affinity == AFFINITY_NONE) {
@@ -677,6 +677,11 @@ int htp_hdrs_completecb(http_parser *htp) {
     resp.http_minor = 1;
   }
 
+  auto dconn = downstream->get_downstream_connection();
+
+  downstream->set_downstream_addr_group(dconn->get_downstream_addr_group());
+  downstream->set_addr(dconn->get_addr());
+
   if (resp.fs.parse_content_length() != 0) {
     downstream->set_response_state(Downstream::MSG_BAD_HEADER);
     return -1;
@@ -1192,9 +1197,9 @@ int HttpDownstreamConnection::actual_signal_write() {
 
 int HttpDownstreamConnection::noop() { return 0; }
 
-DownstreamAddrGroup *
+const std::shared_ptr<DownstreamAddrGroup> &
 HttpDownstreamConnection::get_downstream_addr_group() const {
-  return group_.get();
+  return group_;
 }
 
 DownstreamAddr *HttpDownstreamConnection::get_addr() const { return addr_; }

src/shrpx_http_downstream_connection.h

@@ -64,7 +64,8 @@ public:
 
   virtual bool poolable() const;
 
-  virtual DownstreamAddrGroup *get_downstream_addr_group() const;
+  virtual const std::shared_ptr<DownstreamAddrGroup> &
+  get_downstream_addr_group() const;
   virtual DownstreamAddr *get_addr() const;
 
   int read_clear();
@@ -88,7 +89,7 @@ private:
   Worker *worker_;
   // nullptr if TLS is not used.
   SSL_CTX *ssl_ctx_;
-  const std::shared_ptr<DownstreamAddrGroup> &group_;
+  std::shared_ptr<DownstreamAddrGroup> group_;
   // Address of remote endpoint
   DownstreamAddr *addr_;
   IOControl ioctrl_;

src/shrpx_log.cc

@@ -45,6 +45,7 @@
 
 #include "shrpx_config.h"
 #include "shrpx_downstream.h"
+#include "shrpx_worker.h"
 #include "util.h"
 #include "template.h"
 
@@ -367,6 +368,21 @@ void upstream_accesslog(const std::vector<LogFragment> &lfv,
       std::tie(p, avail) =
           copy_l(lgsp.tls_info->session_reused ? "r" : ".", avail, p);
       break;
+    case SHRPX_LOGF_BACKEND_HOST:
+      if (!lgsp.downstream_addr) {
+        std::tie(p, avail) = copy_l("-", avail, p);
+        break;
+      }
+      std::tie(p, avail) = copy(lgsp.downstream_addr->host, avail, p);
+      break;
+    case SHRPX_LOGF_BACKEND_PORT:
+      if (!lgsp.downstream_addr) {
+        std::tie(p, avail) = copy_l("-", avail, p);
+        break;
+      }
+      std::tie(p, avail) =
+          copy(util::utos(lgsp.downstream_addr->port), avail, p);
+      break;
     case SHRPX_LOGF_NONE:
       break;
     default:

src/shrpx_log.h

@@ -85,6 +85,7 @@ using namespace nghttp2;
 namespace shrpx {
 
 class Downstream;
+struct DownstreamAddr;
 
 enum SeverityLevel { INFO, NOTICE, WARN, ERROR, FATAL };
 
@@ -131,6 +132,8 @@ enum LogFragmentType {
   SHRPX_LOGF_SSL_PROTOCOL,
   SHRPX_LOGF_SSL_SESSION_ID,
   SHRPX_LOGF_SSL_SESSION_REUSED,
+  SHRPX_LOGF_BACKEND_HOST,
+  SHRPX_LOGF_BACKEND_PORT,
 };
 
 struct LogFragment {
@@ -142,6 +145,7 @@ struct LogFragment {
 
 struct LogSpec {
   Downstream *downstream;
+  const DownstreamAddr *downstream_addr;
   StringRef remote_addr;
   StringRef method;
   StringRef path;

tests/main.c

@@ -308,6 +308,8 @@ int main(int argc _U_, char *argv[] _U_) {
                    test_nghttp2_session_set_local_window_size) ||
       !CU_add_test(pSuite, "session_cancel_from_before_frame_send",
                    test_nghttp2_session_cancel_from_before_frame_send) ||
+      !CU_add_test(pSuite, "session_removed_closed_stream",
+                   test_nghttp2_session_removed_closed_stream) ||
       !CU_add_test(pSuite, "http_mandatory_headers",
                    test_nghttp2_http_mandatory_headers) ||
       !CU_add_test(pSuite, "http_content_length",

tests/nghttp2_session_test.c

@@ -2366,7 +2366,7 @@ void test_nghttp2_session_on_request_headers_received(void) {
 
   nghttp2_frame_headers_free(&frame.headers, mem);
   session->local_settings.max_concurrent_streams =
-      NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS;
+      NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS;
 
   /* Stream ID less than or equal to the previouly received request
      HEADERS is just ignored due to race condition */
@@ -4003,22 +4003,23 @@ void test_nghttp2_session_upgrade2(void) {
 void test_nghttp2_session_reprioritize_stream(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;
-  my_user_data ud;
   nghttp2_stream *stream;
   nghttp2_stream *dep_stream;
   nghttp2_priority_spec pri_spec;
+  int rv;
 
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
   callbacks.send_callback = block_count_send_callback;
 
-  nghttp2_session_server_new(&session, &callbacks, &ud);
+  nghttp2_session_server_new(&session, &callbacks, NULL);
 
   stream = open_recv_stream(session, 1);
 
   nghttp2_priority_spec_init(&pri_spec, 0, 10, 0);
 
-  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(10 == stream->weight);
   CU_ASSERT(&session->root == stream->dep_prev);
 
@@ -4026,8 +4027,9 @@ void test_nghttp2_session_reprioritize_stream(void) {
 
   nghttp2_priority_spec_init(&pri_spec, 3, 99, 0);
 
-  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(99 == stream->weight);
   CU_ASSERT(3 == stream->dep_prev->stream_id);
 
@@ -4040,16 +4042,18 @@ void test_nghttp2_session_reprioritize_stream(void) {
   /* Change weight */
   pri_spec.weight = 128;
 
-  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(128 == stream->weight);
   CU_ASSERT(dep_stream == stream->dep_prev);
 
   /* Change weight again to test short-path case */
   pri_spec.weight = 100;
 
-  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(100 == stream->weight);
   CU_ASSERT(dep_stream == stream->dep_prev);
   CU_ASSERT(100 == dep_stream->sum_dep_weight);
@@ -4058,8 +4062,9 @@ void test_nghttp2_session_reprioritize_stream(void) {
      root.  Then stream 3 depends on it. */
   nghttp2_priority_spec_init(&pri_spec, 1, 1, 0);
 
-  nghttp2_session_reprioritize_stream(session, dep_stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, dep_stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(1 == dep_stream->weight);
   CU_ASSERT(stream == dep_stream->dep_prev);
 
@@ -4069,11 +4074,76 @@ void test_nghttp2_session_reprioritize_stream(void) {
 
   nghttp2_priority_spec_init(&pri_spec, 5, 5, 0);
 
-  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
+  CU_ASSERT(0 == rv);
   CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
 
   nghttp2_session_del(session);
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  /* circular dependency; in case of stream which is not a direct
+     descendant of root.  Use exclusive dependency. */
+  stream = open_recv_stream(session, 1);
+  stream = open_recv_stream_with_dep(session, 3, stream);
+  stream = open_recv_stream_with_dep(session, 5, stream);
+  stream = open_recv_stream_with_dep(session, 7, stream);
+  open_recv_stream_with_dep(session, 9, stream);
+
+  nghttp2_priority_spec_init(&pri_spec, 7, 1, 1);
+
+  stream = nghttp2_session_get_stream(session, 3);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(7 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 7);
+
+  CU_ASSERT(1 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 9);
+
+  CU_ASSERT(3 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 5);
+
+  CU_ASSERT(3 == stream->dep_prev->stream_id);
+
+  nghttp2_session_del(session);
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  /* circular dependency; in case of stream which is not a direct
+     descendant of root.  Without exclusive dependency. */
+  stream = open_recv_stream(session, 1);
+  stream = open_recv_stream_with_dep(session, 3, stream);
+  stream = open_recv_stream_with_dep(session, 5, stream);
+  stream = open_recv_stream_with_dep(session, 7, stream);
+  open_recv_stream_with_dep(session, 9, stream);
+
+  nghttp2_priority_spec_init(&pri_spec, 7, 1, 0);
+
+  stream = nghttp2_session_get_stream(session, 3);
+  rv = nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(7 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 7);
+
+  CU_ASSERT(1 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 9);
+
+  CU_ASSERT(7 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream(session, 5);
+
+  CU_ASSERT(3 == stream->dep_prev->stream_id);
+
+  nghttp2_session_del(session);
 }
 
 void test_nghttp2_session_reprioritize_stream_with_idle_stream_dep(void) {
@@ -4972,7 +5042,7 @@ void test_nghttp2_submit_settings(void) {
             nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv, 7));
 
   /* Make sure that local settings are not changed */
-  CU_ASSERT(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS ==
+  CU_ASSERT(NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS ==
             session->local_settings.max_concurrent_streams);
   CU_ASSERT(NGHTTP2_INITIAL_WINDOW_SIZE ==
             session->local_settings.initial_window_size);
@@ -9645,6 +9715,186 @@ void test_nghttp2_session_cancel_from_before_frame_send(void) {
   nghttp2_session_del(session);
 }
 
+static void
+prepare_session_removed_closed_stream(nghttp2_session *session,
+                                      nghttp2_hd_deflater *deflater) {
+  int rv;
+  nghttp2_settings_entry iv;
+  nghttp2_bufs bufs;
+  nghttp2_mem *mem;
+  ssize_t nread;
+  int i;
+  nghttp2_stream *stream;
+  nghttp2_frame_hd hd;
+
+  mem = nghttp2_mem_default();
+
+  frame_pack_bufs_init(&bufs);
+
+  iv.settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS;
+  iv.value = 2;
+
+  rv = nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, &iv, 1);
+
+  CU_ASSERT(0 == rv);
+
+  rv = nghttp2_session_send(session);
+
+  CU_ASSERT(0 == rv);
+
+  for (i = 1; i <= 3; i += 2) {
+    rv = pack_headers(&bufs, deflater, i,
+                      NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM, reqnv,
+                      ARRLEN(reqnv), mem);
+
+    CU_ASSERT(0 == rv);
+
+    nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                     nghttp2_bufs_len(&bufs));
+
+    CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+
+    nghttp2_bufs_reset(&bufs);
+  }
+
+  nghttp2_session_close_stream(session, 3, NGHTTP2_NO_ERROR);
+
+  rv = pack_headers(&bufs, deflater, 5,
+                    NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM, reqnv,
+                    ARRLEN(reqnv), mem);
+
+  CU_ASSERT(0 == rv);
+
+  /* Receiving stream 5 will erase stream 3 from closed stream list */
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+
+  stream = nghttp2_session_get_stream_raw(session, 3);
+
+  CU_ASSERT(NULL == stream);
+
+  /* Since the current max concurrent streams is
+     NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS, receiving frame on stream
+     3 is ignored. */
+  nghttp2_bufs_reset(&bufs);
+  rv = pack_headers(&bufs, deflater, 3,
+                    NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM,
+                    trailernv, ARRLEN(trailernv), mem);
+
+  CU_ASSERT(0 == rv);
+
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+  CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
+
+  nghttp2_frame_hd_init(&hd, 0, NGHTTP2_DATA, NGHTTP2_FLAG_NONE, 3);
+  nghttp2_bufs_reset(&bufs);
+  nghttp2_frame_pack_frame_hd(bufs.head->buf.last, &hd);
+  bufs.head->buf.last += NGHTTP2_FRAME_HDLEN;
+
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+  CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
+
+  /* Now server receives SETTINGS ACK */
+  nghttp2_frame_hd_init(&hd, 0, NGHTTP2_SETTINGS, NGHTTP2_FLAG_ACK, 0);
+  nghttp2_bufs_reset(&bufs);
+  nghttp2_frame_pack_frame_hd(bufs.head->buf.last, &hd);
+  bufs.head->buf.last += NGHTTP2_FRAME_HDLEN;
+
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+
+  nghttp2_bufs_free(&bufs);
+}
+
+void test_nghttp2_session_removed_closed_stream(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  int rv;
+  nghttp2_hd_deflater deflater;
+  nghttp2_bufs bufs;
+  nghttp2_mem *mem;
+  ssize_t nread;
+  nghttp2_frame_hd hd;
+  nghttp2_outbound_item *item;
+
+  mem = nghttp2_mem_default();
+
+  frame_pack_bufs_init(&bufs);
+
+  memset(&callbacks, 0, sizeof(callbacks));
+
+  callbacks.send_callback = null_send_callback;
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  /* Now local max concurrent streams is still unlimited, pending max
+     concurrent streams is now 2. */
+
+  nghttp2_hd_deflate_init(&deflater, mem);
+
+  prepare_session_removed_closed_stream(session, &deflater);
+
+  /* Now current max concurrent streams is 2, so receiving frame on
+     stream 3 is treated as connection error */
+  nghttp2_bufs_reset(&bufs);
+  rv = pack_headers(&bufs, &deflater, 3,
+                    NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM,
+                    trailernv, ARRLEN(trailernv), mem);
+
+  CU_ASSERT(0 == rv);
+
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+
+  item = nghttp2_session_get_next_ob_item(session);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
+  CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code);
+
+  nghttp2_hd_deflate_free(&deflater);
+  nghttp2_session_del(session);
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+  nghttp2_hd_deflate_init(&deflater, mem);
+  /* Same setup, and then receive DATA instead of HEADERS */
+
+  prepare_session_removed_closed_stream(session, &deflater);
+
+  nghttp2_frame_hd_init(&hd, 0, NGHTTP2_DATA, NGHTTP2_FLAG_NONE, 3);
+  nghttp2_bufs_reset(&bufs);
+  nghttp2_frame_pack_frame_hd(bufs.head->buf.last, &hd);
+  bufs.head->buf.last += NGHTTP2_FRAME_HDLEN;
+
+  nread = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                   nghttp2_bufs_len(&bufs));
+
+  CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == nread);
+
+  item = nghttp2_session_get_next_ob_item(session);
+
+  CU_ASSERT(NULL != item);
+  CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
+  CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code);
+
+  nghttp2_hd_deflate_free(&deflater);
+  nghttp2_session_del(session);
+
+  nghttp2_bufs_free(&bufs);
+}
+
 static void check_nghttp2_http_recv_headers_fail(
     nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id,
     int stream_state, const nghttp2_nv *nva, size_t nvlen) {

tests/nghttp2_session_test.h

@@ -152,6 +152,7 @@ void test_nghttp2_session_repeated_priority_change(void);
 void test_nghttp2_session_repeated_priority_submission(void);
 void test_nghttp2_session_set_local_window_size(void);
 void test_nghttp2_session_cancel_from_before_frame_send(void);
+void test_nghttp2_session_removed_closed_stream(void);
 void test_nghttp2_http_mandatory_headers(void);
 void test_nghttp2_http_content_length(void);
 void test_nghttp2_http_content_length_mismatch(void);