http2: move lots of state data to the 'stream' struct

... from the connection struct. The stream one being the 'struct HTTP'
which is kept in the SessionHandle struct (easy handle).

lookup streams for incoming frames in the stream hash, hashing is based
on the stream id and we get the SessionHandle for the incoming stream
that way.

3 files changed, 117 insertions, 83 deletions

diff --git a/lib/http.c b/lib/http.c
index b1e555cf6..aa1dcbf0e 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -153,12 +153,22 @@ CURLcode Curl_http_setup_conn(struct connectdata *conn)
 {
   /* allocate the HTTP-specific struct for the SessionHandle, only to survive
      during this request */
+  struct HTTP *http;
   DEBUGASSERT(conn->data->req.protop == NULL);
 
-  conn->data->req.protop = calloc(1, sizeof(struct HTTP));
-  if(!conn->data->req.protop)
+  http = calloc(1, sizeof(struct HTTP));
+  if(!http)
     return CURLE_OUT_OF_MEMORY;
 
+  conn->data->req.protop = http;
+
+  http->header_recvbuf = Curl_add_buffer_init();
+  http->nread_header_recvbuf = 0;
+  http->bodystarted = FALSE;
+  http->status_code = -1;
+  http->data = NULL;
+  http->datalen = 0;
+
   return CURLE_OK;
 }
 
diff --git a/lib/http.h b/lib/http.h
index b53b963fa..a64d83f18 100644
--- a/lib/http.h
+++ b/lib/http.h
@@ -153,13 +153,17 @@ struct HTTP {
   void *send_buffer; /* used if the request couldn't be sent in one chunk,
                         points to an allocated send_buffer struct */
 
-  /* for HTTP/2 we store stream-local data here */
+  /*********** for HTTP/2 we store stream-local data here *************/
   int32_t stream_id; /* stream we are interested in */
 
+  bool bodystarted;
   /* We store non-final and final response headers here, per-stream */
   Curl_send_buffer *header_recvbuf;
   size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into
                                   upper layer */
+  int status_code; /* HTTP status code */
+  const uint8_t *data; /* pointer to data chunk, received in on_data_chunk */
+  size_t datalen; /* the number of bytes left in data */
 };
 
 typedef int (*sending)(void); /* Curl_send */
@@ -173,14 +177,10 @@ struct http_conn {
   size_t  binlen; /* length of the binsettings data */
   char *mem;     /* points to a buffer in memory to store */
   size_t len;    /* size of the buffer 'mem' points to */
-  bool bodystarted;
   sending send_underlying; /* underlying send Curl_send callback */
   recving recv_underlying; /* underlying recv Curl_recv callback */
   bool closed; /* TRUE on HTTP2 stream close */
   uint32_t error_code; /* HTTP/2 error code */
-  const uint8_t *data; /* pointer to data chunk, received in
-                          on_data_chunk */
-  size_t datalen; /* the number of bytes left in data */
   char *inbuf; /* buffer to receive data from underlying socket */
   /* We need separate buffer for transmission and reception because we
      may call nghttp2_session_send() after the
@@ -190,7 +190,6 @@ struct http_conn {
   const uint8_t *upload_mem; /* points to a buffer to read from */
   size_t upload_len; /* size of the buffer 'upload_mem' points to */
   size_t upload_left; /* number of bytes left to upload */
-  int status_code; /* HTTP status code */
 
   /* this is a hash of all individual streams (SessionHandle structs) */
   struct curl_hash streamsh;
diff --git a/lib/http2.c b/lib/http2.c
index 6d25efc98..3959a4f8a 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -186,20 +186,38 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
 {
   struct connectdata *conn = (struct connectdata *)userp;
   struct http_conn *c = &conn->proto.httpc;
+  struct SessionHandle *data_s = NULL;
+  struct HTTP *stream = NULL;
   int rv;
   size_t left, ncopy;
+  int32_t stream_id = frame->hd.stream_id;
 
   (void)session;
   (void)frame;
   DEBUGF(infof(conn->data, "on_frame_recv() was called with header %x\n",
                frame->hd.type));
+
+  if(stream_id) {
+    /* get the stream from the hash based on Stream ID, stream ID zero is for
+       connection-oriented stuff */
+    data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id,
+                            sizeof(stream_id));
+    if(!data_s) {
+      /* Receiving a Stream ID not in the hash should not happen, this is an
+         internal error more than anything else! */
+      failf(conn->data, "Received frame on Stream ID: %x not in stream hash!",
+            stream_id);
+      return NGHTTP2_ERR_CALLBACK_FAILURE;
+    }
+    stream = data_s->req.protop;
+  }
+
   switch(frame->hd.type) {
   case NGHTTP2_DATA:
-    /* If body started, then receiving DATA is illegal. */
-    if(!c->bodystarted) {
+    /* If body started on this stream, then receiving DATA is illegal. */
+    if(!stream->bodystarted) {
       rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                     frame->hd.stream_id,
-                                     NGHTTP2_PROTOCOL_ERROR);
+                                     stream_id, NGHTTP2_PROTOCOL_ERROR);
 
       if(nghttp2_is_fatal(rv)) {
         return NGHTTP2_ERR_CALLBACK_FAILURE;
@@ -210,14 +228,13 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
     if(frame->headers.cat == NGHTTP2_HCAT_REQUEST)
       break;
 
-    if(c->bodystarted) {
+    if(stream->bodystarted) {
       /* Only valid HEADERS after body started is trailer header,
          which is not fully supported in this code.  If HEADERS is not
          trailer, then it is a PROTOCOL_ERROR. */
       if((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) {
         rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                       frame->hd.stream_id,
-                                       NGHTTP2_PROTOCOL_ERROR);
+                                       stream_id, NGHTTP2_PROTOCOL_ERROR);
 
         if(nghttp2_is_fatal(rv)) {
           return NGHTTP2_ERR_CALLBACK_FAILURE;
@@ -226,11 +243,10 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
       break;
     }
 
-    if(c->status_code == -1) {
+    if(stream->status_code == -1) {
       /* No :status header field means PROTOCOL_ERROR. */
       rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                     frame->hd.stream_id,
-                                     NGHTTP2_PROTOCOL_ERROR);
+                                     stream_id, NGHTTP2_PROTOCOL_ERROR);
 
       if(nghttp2_is_fatal(rv)) {
         return NGHTTP2_ERR_CALLBACK_FAILURE;
@@ -240,22 +256,19 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
     }
 
     /* Only final status code signals the end of header */
-    if(c->status_code / 100 != 1) {
-      c->bodystarted = TRUE;
-    }
+    if(stream->status_code / 100 != 1)
+      stream->bodystarted = TRUE;
 
-    c->status_code = -1;
+    stream->status_code = -1;
 
-    /* get the stream from the hash based on Stream ID */
-    rv = Curl_hash_pick()
+    Curl_add_buffer(stream->header_recvbuf, "\r\n", 2);
 
-    Curl_add_buffer(c->header_recvbuf, "\r\n", 2);
-
-    left = c->header_recvbuf->size_used - c->nread_header_recvbuf;
+    left = stream->header_recvbuf->size_used - stream->nread_header_recvbuf;
     ncopy = c->len < left ? c->len : left;
 
-    memcpy(c->mem, c->header_recvbuf->buffer + c->nread_header_recvbuf, ncopy);
-    c->nread_header_recvbuf += ncopy;
+    memcpy(c->mem, stream->header_recvbuf->buffer +
+           stream->nread_header_recvbuf, ncopy);
+    stream->nread_header_recvbuf += ncopy;
 
     c->mem += ncopy;
     c->len -= ncopy;
@@ -291,7 +304,8 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
 {
   struct connectdata *conn = (struct connectdata *)userp;
   struct http_conn *c = &conn->proto.httpc;
-  struct HTTP *stream = conn->data->req.protop;
+  struct HTTP *stream;
+  struct SessionHandle *data_s;
   size_t nread;
   (void)session;
   (void)flags;
@@ -299,12 +313,19 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
   DEBUGF(infof(conn->data, "on_data_chunk_recv() "
                "len = %u, stream = %x\n", len, stream_id));
 
-  if(stream_id != stream->stream_id) {
-    DEBUGF(infof(conn->data, "on_data_chunk_recv() "
-                 "got stream %x, expected stream %x\n",
-                 stream_id, stream->stream_id));
-    return 0;
+  DEBUGASSERT(stream_id); /* should never be a zero stream ID here */
+
+  /* get the stream from the hash based on Stream ID */
+  data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id,
+                          sizeof(stream_id));
+  if(!data_s) {
+    /* Receiving a Stream ID not in the hash should not happen, this is an
+       internal error more than anything else! */
+    failf(conn->data, "Received frame on Stream ID: %x not in stream hash!",
+          stream_id);
+    return NGHTTP2_ERR_CALLBACK_FAILURE;
   }
+  stream = data_s->req.protop;
 
   nread = c->len < len ? c->len : len;
   memcpy(c->mem, data, nread);
@@ -315,8 +336,8 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
   DEBUGF(infof(conn->data, "%zu data written\n", nread));
 
   if(nread < len) {
-    c->data = data + nread;
-    c->datalen = len - nread;
+    stream->data = data + nread;
+    stream->datalen = len - nread;
     return NGHTTP2_ERR_PAUSE;
   }
   return 0;
@@ -425,11 +446,12 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
                      void *userp)
 {
   struct connectdata *conn = (struct connectdata *)userp;
-  struct http_conn *c = &conn->proto.httpc;
-  struct HTTP *stream = conn->data->req.protop;
+  struct HTTP *stream;
+  struct SessionHandle *data_s;
   int rv;
   int goodname;
   int goodheader;
+  int32_t stream_id = frame->hd.stream_id;
 
   (void)session;
   (void)frame;
@@ -440,31 +462,36 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
     return 0;
   }
 
-  if(frame->hd.stream_id != stream->stream_id) {
-    DEBUGF(infof(conn->data, "on_header() "
-                 "got stream %x, expected stream %x\n",
-                 frame->hd.stream_id, stream->stream_id));
-    return 0;
+  DEBUGASSERT(stream_id); /* should never be a zero stream ID here */
+
+  /* get the stream from the hash based on Stream ID */
+  data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id,
+                          sizeof(stream_id));
+  if(!data_s) {
+    /* Receiving a Stream ID not in the hash should not happen, this is an
+       internal error more than anything else! */
+    failf(conn->data, "Received frame on Stream ID: %x not in stream hash!",
+          stream_id);
+    return NGHTTP2_ERR_CALLBACK_FAILURE;
   }
+  stream = data_s->req.protop;
 
-  if(c->bodystarted) {
+  if(stream->bodystarted)
     /* Ignore trailer or HEADERS not mapped to HTTP semantics.  The
        consequence is handled in on_frame_recv(). */
     return 0;
-  }
 
   goodname = nghttp2_check_header_name(name, namelen);
   goodheader = nghttp2_check_header_value(value, valuelen);
 
   if(!goodname || !goodheader) {
 
-    infof(conn->data, "Detected bad incoming header %s%s, reset stream!\n",
+    infof(data_s, "Detected bad incoming header %s%s, reset stream!\n",
           goodname?"":"name",
           goodheader?"":"value");
 
     rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                   frame->hd.stream_id,
-                                   NGHTTP2_PROTOCOL_ERROR);
+                                   stream_id, NGHTTP2_PROTOCOL_ERROR);
 
     if(nghttp2_is_fatal(rv)) {
       return NGHTTP2_ERR_CALLBACK_FAILURE;
@@ -477,12 +504,11 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
      memcmp(STATUS, name, namelen) == 0) {
 
     /* :status must appear exactly once. */
-    if(c->status_code != -1 ||
-       (c->status_code = decode_status_code(value, valuelen)) == -1) {
+    if(stream->status_code != -1 ||
+       (stream->status_code = decode_status_code(value, valuelen)) == -1) {
 
       rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                     frame->hd.stream_id,
-                                     NGHTTP2_PROTOCOL_ERROR);
+                                     stream_id, NGHTTP2_PROTOCOL_ERROR);
       if(nghttp2_is_fatal(rv)) {
         return NGHTTP2_ERR_CALLBACK_FAILURE;
       }
@@ -490,9 +516,9 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
       return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
     }
 
-    Curl_add_buffer(c->header_recvbuf, "HTTP/2.0 ", 9);
-    Curl_add_buffer(c->header_recvbuf, value, valuelen);
-    Curl_add_buffer(c->header_recvbuf, "\r\n", 2);
+    Curl_add_buffer(stream->header_recvbuf, "HTTP/2.0 ", 9);
+    Curl_add_buffer(stream->header_recvbuf, value, valuelen);
+    Curl_add_buffer(stream->header_recvbuf, "\r\n", 2);
 
     return 0;
   }
@@ -500,10 +526,9 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
     /* Here we are sure that namelen > 0 because of
        nghttp2_check_header_name().  Pseudo header other than :status
        is illegal. */
-    if(c->status_code == -1 || name[0] == ':') {
+    if(stream->status_code == -1 || name[0] == ':') {
       rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE,
-                                     frame->hd.stream_id,
-                                     NGHTTP2_PROTOCOL_ERROR);
+                                     stream_id, NGHTTP2_PROTOCOL_ERROR);
       if(nghttp2_is_fatal(rv)) {
         return NGHTTP2_ERR_CALLBACK_FAILURE;
       }
@@ -512,12 +537,12 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
     }
 
     /* convert to a HTTP1-style header */
-    Curl_add_buffer(c->header_recvbuf, name, namelen);
-    Curl_add_buffer(c->header_recvbuf, ":", 1);
-    Curl_add_buffer(c->header_recvbuf, value, valuelen);
-    Curl_add_buffer(c->header_recvbuf, "\r\n", 2);
+    Curl_add_buffer(stream->header_recvbuf, name, namelen);
+    Curl_add_buffer(stream->header_recvbuf, ":", 1);
+    Curl_add_buffer(stream->header_recvbuf, value, valuelen);
+    Curl_add_buffer(stream->header_recvbuf, "\r\n", 2);
 
-    DEBUGF(infof(conn->data, "h2 header: %.*s: %.*s\n",
+    DEBUGF(infof(data_s, "h2 header: %.*s: %.*s\n",
                  namelen, name, valuelen, value));
   }
 
@@ -719,28 +744,34 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
   httpc->upload_mem = NULL;
   httpc->upload_len = 0;
 
-  if(httpc->bodystarted &&
-     httpc->nread_header_recvbuf < httpc->header_recvbuf->size_used) {
+  /*
+   * At this point 'stream' is just in the SessionHandle the connection
+   * identifies as its owner at this time.
+   */
+
+  if(stream->bodystarted &&
+     stream->nread_header_recvbuf < stream->header_recvbuf->size_used) {
+    /* If there is body data pending for this stream to return, do that */
     size_t left =
-      httpc->header_recvbuf->size_used - httpc->nread_header_recvbuf;
+      stream->header_recvbuf->size_used - stream->nread_header_recvbuf;
     size_t ncopy = len < left ? len : left;
-    memcpy(mem, httpc->header_recvbuf->buffer + httpc->nread_header_recvbuf,
+    memcpy(mem, stream->header_recvbuf->buffer + stream->nread_header_recvbuf,
            ncopy);
-    httpc->nread_header_recvbuf += ncopy;
+    stream->nread_header_recvbuf += ncopy;
     return ncopy;
   }
 
-  if(httpc->data) {
-    nread = len < httpc->datalen ? len : httpc->datalen;
-    memcpy(mem, httpc->data, nread);
+  if(stream->data) {
+    nread = len < stream->datalen ? len : stream->datalen;
+    memcpy(mem, stream->data, nread);
 
-    httpc->data += nread;
-    httpc->datalen -= nread;
+    stream->data += nread;
+    stream->datalen -= nread;
 
     infof(conn->data, "%zu data bytes written\n", nread);
-    if(httpc->datalen == 0) {
-      httpc->data = NULL;
-      httpc->datalen = 0;
+    if(stream->datalen == 0) {
+      stream->data = NULL;
+      stream->datalen = 0;
     }
     return nread;
   }
@@ -1027,17 +1058,11 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
     return result;
 
   infof(conn->data, "Using HTTP2, server supports multi-use\n");
-  httpc->bodystarted = FALSE;
   httpc->error_code = NGHTTP2_NO_ERROR;
   httpc->closed = FALSE;
-  httpc->header_recvbuf = Curl_add_buffer_init();
-  httpc->nread_header_recvbuf = 0;
-  httpc->data = NULL;
-  httpc->datalen = 0;
   httpc->upload_left = 0;
   httpc->upload_mem = NULL;
   httpc->upload_len = 0;
-  httpc->status_code = -1;
 
   conn->httpversion = 20;
   conn->bundle->server_supports_pipelining = TRUE;