From: Stefan Eissing <stefan@eissing.org>
Date: Tue, 1 Apr 2025 11:44:24 +0000 (+0200)
Subject: http2: fix stream assignemnt for pushes
X-Git-Tag: curl-8_13_0~7
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1f844dd3f0e66f19377b5158201f5b6eec61507f;p=thirdparty%2Fcurl.git

http2: fix stream assignemnt for pushes

When a PUSH_PROMISE was received, the h2_stream object was assigned
to the wrong `newhandle->mid` and was thereafter not found. This led
to internal confusion, because the nghttp2 stream user_data was not
cleared and an invalid easy handle was use for trace messages,
resulting in a crash.

Reported-by: Viktor Szakats
Fixes #16881
Closes #16905
---

diff --git a/lib/http2.c b/lib/http2.c
index 0b40c9ae06..88fbcceb71 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -648,7 +648,7 @@ static int h2_process_pending_input(struct Curl_cfilter *cf,
 
     rv = nghttp2_session_mem_recv(ctx->h2, (const uint8_t *)buf, blen);
     if(rv < 0) {
-      failf(data, "nghttp2 error %zd: %s", rv, nghttp2_strerror((int)rv));
+      failf(data, "nghttp2 recv error %zd: %s", rv, nghttp2_strerror((int)rv));
       *err = CURLE_HTTP2;
       return -1;
     }
@@ -967,9 +967,6 @@ static int push_promise(struct Curl_cfilter *cf,
       goto fail;
     }
 
-    /* ask the application */
-    CURL_TRC_CF(data, cf, "Got PUSH_PROMISE, ask application");
-
     stream = H2_STREAM_CTX(ctx, data);
     if(!stream) {
       failf(data, "Internal NULL stream");
@@ -984,20 +981,13 @@ static int push_promise(struct Curl_cfilter *cf,
 
     rv = set_transfer_url(newhandle, &heads);
     if(rv) {
+      CURL_TRC_CF(data, cf, "[%d] PUSH_PROMISE, failed to set url -> %d",
+                  frame->promised_stream_id, rv);
       discard_newhandle(cf, newhandle);
       rv = CURL_PUSH_DENY;
       goto fail;
     }
 
-    result = http2_data_setup(cf, newhandle, &newstream);
-    if(result) {
-      failf(data, "error setting up stream: %d", result);
-      discard_newhandle(cf, newhandle);
-      rv = CURL_PUSH_DENY;
-      goto fail;
-    }
-    DEBUGASSERT(stream);
-
     Curl_set_in_callback(data, TRUE);
     rv = data->multi->push_cb(data, newhandle,
                               stream->push_headers_used, &heads,
@@ -1010,16 +1000,15 @@ static int push_promise(struct Curl_cfilter *cf,
     if(rv) {
       DEBUGASSERT((rv > CURL_PUSH_OK) && (rv <= CURL_PUSH_ERROROUT));
       /* denied, kill off the new handle again */
+      CURL_TRC_CF(data, cf, "[%d] PUSH_PROMISE, denied by application -> %d",
+                  frame->promised_stream_id, rv);
       discard_newhandle(cf, newhandle);
       goto fail;
     }
 
-    newstream->id = frame->promised_stream_id;
-    newhandle->req.maxdownload = -1;
-    newhandle->req.size = -1;
-
-    /* approved, add to the multi handle and immediately switch to PERFORM
-       state with the given connection !*/
+    /* approved, add to the multi handle for processing. This
+     * assigns newhandle->mid. For the new `mid` we assign the
+     * h2_stream instance and remember the stream_id already known. */
     rc = Curl_multi_add_perform(data->multi, newhandle, cf->conn);
     if(rc) {
       infof(data, "failed to add handle to multi");
@@ -1028,6 +1017,21 @@ static int push_promise(struct Curl_cfilter *cf,
       goto fail;
     }
 
+    result = http2_data_setup(cf, newhandle, &newstream);
+    if(result) {
+      failf(data, "error setting up stream: %d", result);
+      discard_newhandle(cf, newhandle);
+      rv = CURL_PUSH_DENY;
+      goto fail;
+    }
+
+    DEBUGASSERT(newstream);
+    newstream->id = frame->promised_stream_id;
+    newhandle->req.maxdownload = -1;
+    newhandle->req.size = -1;
+
+    CURL_TRC_CF(data, cf, "promise easy handle added to multi, mid=%"
+                FMT_OFF_T, newhandle->mid);
     rv = nghttp2_session_set_stream_user_data(ctx->h2,
                                               newstream->id,
                                               newhandle);
@@ -1581,7 +1585,7 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
 
   /* get the stream from the hash based on Stream ID */
   data_s = nghttp2_session_get_stream_user_data(session, stream_id);
-  if(!data_s)
+  if(!GOOD_EASY_HANDLE(data_s))
     /* Receiving a Stream ID not in the hash should not happen, this is an
        internal error more than anything else! */
     return NGHTTP2_ERR_CALLBACK_FAILURE;