[GNUnet-SVN] [gnurl] 98/178: http2: handle GOAWAY properly

[gnunet]

This is an automated email from the git hooks/post-receive script.

ng0 pushed a commit to branch master
in repository gnurl.

commit d122df5972fc01e39ae28e6bca705237d7e3318a
Author: Daniel Stenberg <address@hidden>
AuthorDate: Thu Apr 19 20:03:30 2018 +0200

    http2: handle GOAWAY properly
    
    When receiving REFUSED_STREAM, mark the connection for close and retry
    streams accordingly on another/fresh connection.
    
    Reported-by: Terry Wu
    Fixes #2416
    Fixes #1618
    Closes #2510
---
 lib/http2.c    | 17 ++++++++++++-----
 lib/multi.c    |  4 +++-
 lib/transfer.c | 17 +++++++++++++++--
 lib/urldata.h  |  2 +-
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/lib/http2.c b/lib/http2.c
index 6ac69b9b5..fe5fdb1b8 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1106,7 +1106,6 @@ void Curl_http2_done(struct connectdata *conn, bool 
premature)
   struct http_conn *httpc = &conn->proto.httpc;
 
   if(http->header_recvbuf) {
-    H2BUGF(infof(data, "free header_recvbuf!!\n"));
     Curl_add_buffer_free(http->header_recvbuf);
     http->header_recvbuf = NULL; /* clear the pointer */
     Curl_add_buffer_free(http->trailer_recvbuf);
@@ -1376,7 +1375,15 @@ static ssize_t http2_handle_stream_close(struct 
connectdata *conn,
 
   /* Reset to FALSE to prevent infinite loop in readwrite_data function. */
   stream->closed = FALSE;
-  if(httpc->error_code != NGHTTP2_NO_ERROR) {
+  if(httpc->error_code == NGHTTP2_REFUSED_STREAM) {
+    H2BUGF(infof(data, "REFUSED_STREAM (%d), try again on a new connection!\n",
+                 stream->stream_id));
+    connclose(conn, "REFUSED_STREAM"); /* don't use this anymore */
+    data->state.refused_stream = TRUE;
+    *err = CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */
+    return -1;
+  }
+  else if(httpc->error_code != NGHTTP2_NO_ERROR) {
     failf(data, "HTTP/2 stream %u was not closed cleanly: %s (err %d)",
           stream->stream_id, Curl_http2_strerror(httpc->error_code),
           httpc->error_code);
@@ -1604,9 +1611,9 @@ static ssize_t http2_recv(struct connectdata *conn, int 
sockindex,
       }
 
       if(nread == 0) {
-        failf(data, "Unexpected EOF");
-        *err = CURLE_RECV_ERROR;
-        return -1;
+        H2BUGF(infof(data, "end of stream\n"));
+        *err = CURLE_OK;
+        return 0;
       }
 
       H2BUGF(infof(data, "nread=%zd\n", nread));
diff --git a/lib/multi.c b/lib/multi.c
index 822ac3b1e..5f32dd2a1 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -546,7 +546,9 @@ static CURLcode multi_done(struct connectdata **connp,
   if(conn->send_pipe.size || conn->recv_pipe.size) {
     /* Stop if pipeline is not empty . */
     data->easy_conn = NULL;
-    DEBUGF(infof(data, "Connection still in use, no more multi_done now!\n"));
+    DEBUGF(infof(data, "Connection still in use %d/%d, "
+                 "no more multi_done now!\n",
+                 conn->send_pipe.size, conn->recv_pipe.size));
     return CURLE_OK;
   }
 
diff --git a/lib/transfer.c b/lib/transfer.c
index b5614f55f..9712a7f7e 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1926,7 +1926,7 @@ CURLcode Curl_retry_request(struct connectdata *conn,
                             char **url)
 {
   struct Curl_easy *data = conn->data;
-
+  bool retry = FALSE;
   *url = NULL;
 
   /* if we're talking upload, we can't do the checks below, unless the protocol
@@ -1939,7 +1939,7 @@ CURLcode Curl_retry_request(struct connectdata *conn,
       conn->bits.reuse &&
       (!data->set.opt_no_body
         || (conn->handler->protocol & PROTO_FAMILY_HTTP)) &&
-      (data->set.rtspreq != RTSPREQ_RECEIVE)) {
+      (data->set.rtspreq != RTSPREQ_RECEIVE))
     /* We got no data, we attempted to re-use a connection. For HTTP this
        can be a retry so we try again regardless if we expected a body.
        For other protocols we only try again only if we expected a body.
@@ -1947,6 +1947,19 @@ CURLcode Curl_retry_request(struct connectdata *conn,
        This might happen if the connection was left alive when we were
        done using it before, but that was closed when we wanted to read from
        it again. Bad luck. Retry the same request on a fresh connect! */
+    retry = TRUE;
+  else if(data->state.refused_stream &&
+          (data->req.bytecount + data->req.headerbytecount == 0) ) {
+    /* This was sent on a refused stream, safe to rerun. A refused stream
+       error can typically only happen on HTTP/2 level if the stream is safe
+       to issue again, but the nghttp2 API can deliver the message to other
+       streams as well, which is why this adds the check the data counters
+       too. */
+    infof(conn->data, "REFUSED_STREAM, retrying a fresh connect\n");
+    data->state.refused_stream = FALSE; /* clear again */
+    retry = TRUE;
+  }
+  if(retry) {
     infof(conn->data, "Connection died, retrying a fresh connect\n");
     *url = strdup(conn->data->change.url);
     if(!*url)
diff --git a/lib/urldata.h b/lib/urldata.h
index d439dc3d6..db8113937 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1226,7 +1226,7 @@ struct UrlState {
   curl_off_t current_speed;  /* the ProgressShow() function sets this,
                                 bytes / second */
   bool this_is_a_follow; /* this is a followed Location: request */
-
+  bool refused_stream; /* this was refused, try again */
   char *first_host; /* host name of the first (not followed) request.
                        if set, this should be the host name that we will
                        sent authorization to, no else. Used to make Location:

-- 
To stop receiving notification emails like this one, please contact
address@hidden