http2: move the stream error field to the per-transfer storage

Storing a stream error in the per-connection struct was an error that lead to
race conditions as subsequent stream handling could overwrite the error code
before it was used for the stream with the actual problem.

Closes #6910

diff --git a/lib/http.h b/lib/http.h
index 36e2152fe309eb6518fda89088c31ea222ece9cc..851e0a3824eaadd096e0eb8c38775f6b48990117 100644 (file)
--- a/lib/http.h
+++ b/lib/http.h
@@ -211,6 +211,7 @@ struct HTTP {
   char **push_headers;       /* allocated array */
   size_t push_headers_used;  /* number of entries filled in */
   size_t push_headers_alloc; /* number of entries allocated */
+  uint32_t error; /* HTTP/2 stream error code */
 #endif
 #if defined(USE_NGHTTP2) || defined(USE_NGHTTP3)
   bool closed; /* TRUE on HTTP2 stream close */
@@ -281,7 +282,6 @@ struct http_conn {
   /* list of settings that will be sent */
   nghttp2_settings_entry local_settings[3];
   size_t local_settings_num;
-  uint32_t error_code; /* HTTP/2 error code */
 #else
   int unused; /* prevent a compiler warning */
 #endif

diff --git a/lib/http2.c b/lib/http2.c
index 8ceea3681b132dad4b4a053f4b4f551d4587a6f6..9d89b7015440de6906288c4e7ee7c9b935abdf7e 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -58,6 +58,7 @@
 
 #define HTTP2_HUGE_WINDOW_SIZE (32 * 1024 * 1024) /* 32 MB */
 
+#define DEBUG_HTTP2
 #ifdef DEBUG_HTTP2
 #define H2BUGF(x) x
 #else
@@ -288,6 +289,7 @@ void Curl_http2_setup_req(struct Curl_easy *data)
   http->mem = NULL;
   http->len = 0;
   http->memlen = 0;
+  http->error = NGHTTP2_NO_ERROR;
 }
 
 /* called from http_setup_conn */
@@ -295,7 +297,6 @@ void Curl_http2_setup_conn(struct connectdata *conn)
 {
   conn->proto.httpc.settings.max_concurrent_streams =
     DEFAULT_MAX_CONCURRENT_STREAMS;
-  conn->proto.httpc.error_code = NGHTTP2_NO_ERROR;
 }
 
 /*
@@ -877,7 +878,7 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
     httpc = &conn->proto.httpc;
     drain_this(data_s, httpc);
     Curl_expire(data_s, 0, EXPIRE_RUN_NOW);
-    httpc->error_code = error_code;
+    stream->error = error_code;
 
     /* remove the entry from the hash as the stream is now gone */
     rv = nghttp2_session_set_stream_user_data(session, stream_id, 0);
@@ -1397,9 +1398,10 @@ static int h2_process_pending_input(struct Curl_easy *data,
   }
 
   if(should_close_session(httpc)) {
+    struct HTTP *stream = data->req.p.http;
     H2BUGF(infof(data,
                  "h2_process_pending_input: nothing to do in this session\n"));
-    if(httpc->error_code)
+    if(stream->error)
       *err = CURLE_HTTP2;
     else {
       /* not an error per se, but should still close the connection */
@@ -1422,9 +1424,7 @@ CURLcode Curl_http2_done_sending(struct Curl_easy *data,
   if((conn->handler == &Curl_handler_http2_ssl) ||
      (conn->handler == &Curl_handler_http2)) {
     /* make sure this is only attempted for HTTP/2 transfers */
-
     struct HTTP *stream = data->req.p.http;
-
     struct http_conn *httpc = &conn->proto.httpc;
     nghttp2_session *h2 = httpc->h2;
 
@@ -1482,7 +1482,7 @@ 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_REFUSED_STREAM) {
+  if(stream->error == 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 */
@@ -1490,10 +1490,10 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
     *err = CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */
     return -1;
   }
-  else if(httpc->error_code != NGHTTP2_NO_ERROR) {
+  else if(stream->error != NGHTTP2_NO_ERROR) {
     failf(data, "HTTP/2 stream %d was not closed cleanly: %s (err %u)",
-          stream->stream_id, nghttp2_http2_strerror(httpc->error_code),
-          httpc->error_code);
+          stream->stream_id, nghttp2_http2_strerror(stream->error),
+          stream->error);
     *err = CURLE_HTTP2_STREAM;
     return -1;
   }
@@ -1764,7 +1764,7 @@ static ssize_t http2_recv(struct Curl_easy *data, int sockindex,
                    nread));
     }
 
-    if(h2_process_pending_input(data, httpc, err) != 0)
+    if(h2_process_pending_input(data, httpc, err))
       return -1;
   }
   if(stream->memlen) {
@@ -1789,7 +1789,7 @@ static ssize_t http2_recv(struct Curl_easy *data, int sockindex,
     return retlen;
   }
   if(stream->closed)
-    return 0;
+    return http2_handle_stream_close(conn, data, stream, err);
   *err = CURLE_AGAIN;
   H2BUGF(infof(data, "http2_recv returns AGAIN for stream %u\n",
                stream->stream_id));
@@ -2460,10 +2460,10 @@ void Curl_http2_cleanup_dependencies(struct Curl_easy *data)
 
 /* Only call this function for a transfer that already got a HTTP/2
    CURLE_HTTP2_STREAM error! */
-bool Curl_h2_http_1_1_error(struct connectdata *conn)
+bool Curl_h2_http_1_1_error(struct Curl_easy *data)
 {
-  struct http_conn *httpc = &conn->proto.httpc;
-  return (httpc->error_code == NGHTTP2_HTTP_1_1_REQUIRED);
+  struct HTTP *stream = data->req.p.http;
+  return (stream->error == NGHTTP2_HTTP_1_1_REQUIRED);
 }
 
 #else /* !USE_NGHTTP2 */

diff --git a/lib/http2.h b/lib/http2.h
index 114b38271845da73fcff66783dda4baf743e9de6..21e2c086a389783a44bb7529d5d773c077cb6840 100644 (file)
--- a/lib/http2.h
+++ b/lib/http2.h
@@ -62,7 +62,7 @@ void Curl_http2_cleanup_dependencies(struct Curl_easy *data);
 CURLcode Curl_http2_stream_pause(struct Curl_easy *data, bool pause);
 
 /* returns true if the HTTP/2 stream error was HTTP_1_1_REQUIRED */
-bool Curl_h2_http_1_1_error(struct connectdata *conn);
+bool Curl_h2_http_1_1_error(struct Curl_easy *data);
 #else /* USE_NGHTTP2 */
 #define Curl_http2_request_upgrade(x,y) CURLE_UNSUPPORTED_PROTOCOL
 #define Curl_http2_setup(x,y) CURLE_UNSUPPORTED_PROTOCOL

diff --git a/lib/multi.c b/lib/multi.c
index be3e41f8bb5f49bb4609a3b07be3b3fd95f559ca..eac918b9ac7f0c270ebd949d0c83e65d8df64352 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2153,7 +2153,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         }
       }
       else if((CURLE_HTTP2_STREAM == result) &&
-              Curl_h2_http_1_1_error(data->conn)) {
+              Curl_h2_http_1_1_error(data)) {
         CURLcode ret = Curl_retry_request(data, &newurl);
 
         if(!ret) {