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
This commit is contained in:
Stefan Eissing 2025-04-01 13:44:24 +02:00 committed by Daniel Stenberg
parent fe9c99e377
commit 1f844dd3f0
No known key found for this signature in database
GPG key ID: 5CC908FDB71E12C2

View file

@ -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;