http2: Don't call nghttp2_session_mem_recv while it is paused by a stream

2 files changed, 28 insertions, 3 deletions

diff --git a/lib/http.h b/lib/http.h
index 13fa1d99b..0b4aac343 100644
--- a/lib/http.h
+++ b/lib/http.h
@@ -201,6 +201,8 @@ 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 */
+  int32_t pause_stream_id; /* stream ID which paused
+                              nghttp2_session_mem_recv */
 
   /* 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 78a09af8e..e9eb81180 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -372,6 +372,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
     stream->data = data + nread;
     stream->datalen = len - nread;
     DEBUGF(infof(data_s, "NGHTTP2_ERR_PAUSE - out of buffer\n"));
+    conn->proto.httpc.pause_stream_id = stream_id;
     return NGHTTP2_ERR_PAUSE;
   }
   return 0;
@@ -762,8 +763,12 @@ CURLcode Curl_http2_request_upgrade(Curl_send_buffer *req,
   return result;
 }
 
-static ssize_t http2_handle_stream_close(struct SessionHandle *data,
+static ssize_t http2_handle_stream_close(struct http_conn *httpc,
+                                         struct SessionHandle *data,
                                          struct HTTP *stream, CURLcode *err) {
+  if(httpc->pause_stream_id == stream->stream_id) {
+    httpc->pause_stream_id = 0;
+  }
   /* Reset to FALSE to prevent infinite loop in readwrite_data
    function. */
   stream->closed = FALSE;
@@ -798,7 +803,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
      otherwise, we may be going to read from underlying connection,
      and gets EAGAIN, and we will get stuck there. */
   if(stream->memlen == 0 && stream->closed) {
-    return http2_handle_stream_close(data, stream, err);
+    return http2_handle_stream_close(httpc, data, stream, err);
   }
 
   /* Nullify here because we call nghttp2_session_send() and they
@@ -835,6 +840,10 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
 
     infof(data, "%zu data bytes written\n", nread);
     if(stream->datalen == 0) {
+      DEBUGF(infof(data, "Unpaused by stream %x\n", stream->stream_id));
+      assert(httpc->pause_stream_id == stream->stream_id);
+      httpc->pause_stream_id = 0;
+
       stream->data = NULL;
       stream->datalen = 0;
     }
@@ -858,6 +867,18 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
       stream->mem = mem;
     }
   }
+  else if(httpc->pause_stream_id) {
+    /* If a stream paused nghttp2_session_mem_recv previously, and has
+       not processed all data, it still refers to the buffer in
+       nghttp2_session.  If we call nghttp2_session_mem_recv(), we may
+       overwrite that buffer.  To avoid that situation, just return
+       here with CURLE_AGAIN.  This could be busy loop since data in
+       socket is not read.  But it seems that usually streams are
+       notified with its drain property, and socket is read again
+       quickly. */
+    *err = CURLE_AGAIN;
+    return -1;
+  }
   else {
     char *inbuf;
     /* remember where to store incoming data for this stream and how big the
@@ -939,7 +960,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
   /* If stream is closed, return 0 to signal the http routine to close
      the connection */
   if(stream->closed) {
-    return http2_handle_stream_close(data, stream, err);
+    return http2_handle_stream_close(httpc, data, stream, err);
   }
   *err = CURLE_AGAIN;
   DEBUGF(infof(data, "http2_recv returns -1, AGAIN\n"));
@@ -1169,6 +1190,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
   httpc->inbuflen = 0;
   httpc->nread_inbuf = 0;
 
+  httpc->pause_stream_id = 0;
+
   conn->bits.multiplex = TRUE; /* at least potentially multiplexed */
   conn->httpversion = 20;
   conn->bundle->server_supports_pipelining = TRUE;