From 408aba6190367d15c34d0437c60c480fe022852d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 26 Oct 2025 21:32:37 +0100 Subject: [PATCH] deps: nghttp2: revert 7784fa979d0b This commit reverts "Make error handling robust". Without this revert, we are getting timeouts, crashes, and different error codes in `parallel/test-http2-*`. Refs: https://github.com/nghttp2/nghttp2/commit/7784fa979d0bcf801a35f1afbb25fb048d815cd7 PR-URL: https://github.com/nodejs/node/pull/59790 Reviewed-By: Antoine du Hamel Reviewed-By: Trivikram Kamat --- deps/nghttp2/lib/nghttp2_int.h | 6 +- deps/nghttp2/lib/nghttp2_session.c | 158 +++++++++++++++-------------- 2 files changed, 81 insertions(+), 83 deletions(-) diff --git a/deps/nghttp2/lib/nghttp2_int.h b/deps/nghttp2/lib/nghttp2_int.h index 5ee6b313a0..b23585ccb2 100644 --- a/deps/nghttp2/lib/nghttp2_int.h +++ b/deps/nghttp2/lib/nghttp2_int.h @@ -52,11 +52,7 @@ typedef enum { * Unlike NGHTTP2_ERR_IGN_HTTP_HEADER, this does not invoke * nghttp2_on_invalid_header_callback. */ - NGHTTP2_ERR_REMOVE_HTTP_HEADER = -106, - /* - * Cancel pushed stream. - */ - NGHTTP2_ERR_PUSH_CANCEL = -107, + NGHTTP2_ERR_REMOVE_HTTP_HEADER = -106 } nghttp2_internal_error; #endif /* NGHTTP2_INT_H */ diff --git a/deps/nghttp2/lib/nghttp2_session.c b/deps/nghttp2/lib/nghttp2_session.c index a7ab61d29a..a53c7420b3 100644 --- a/deps/nghttp2/lib/nghttp2_session.c +++ b/deps/nghttp2/lib/nghttp2_session.c @@ -3272,9 +3272,7 @@ static int session_call_on_invalid_header(nghttp2_session *session, session, frame, nv->name->base, nv->name->len, nv->value->base, nv->value->len, nv->flags, session->user_data); } else { - /* If both callbacks are not set, the invalid field nv is - ignored. */ - return 0; + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } if (rv == NGHTTP2_ERR_PAUSE || rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { @@ -3359,10 +3357,6 @@ static uint32_t get_error_code_from_lib_error_code(int lib_error_code) { case NGHTTP2_ERR_HTTP_HEADER: case NGHTTP2_ERR_HTTP_MESSAGING: return NGHTTP2_PROTOCOL_ERROR; - case NGHTTP2_ERR_INTERNAL: - return NGHTTP2_INTERNAL_ERROR; - case NGHTTP2_ERR_PUSH_CANCEL: - return NGHTTP2_CANCEL; default: return NGHTTP2_INTERNAL_ERROR; } @@ -3414,7 +3408,7 @@ static int session_handle_invalid_stream2(nghttp2_session *session, if (rv != 0) { return rv; } - if (frame && session->callbacks.on_invalid_frame_recv_callback) { + if (session->callbacks.on_invalid_frame_recv_callback) { if (session->callbacks.on_invalid_frame_recv_callback( session, frame, lib_error_code, session->user_data) != 0) { return NGHTTP2_ERR_CALLBACK_FAILURE; @@ -3569,29 +3563,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, rv2 = session_call_on_invalid_header(session, frame, &nv); if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n", - frame->hd.type, frame->hd.stream_id, (int)nv.name->len, - nv.name->base, (int)nv.value->len, nv.value->base); - - rv = session_call_error_callback( - session, NGHTTP2_ERR_HTTP_HEADER, - "Invalid HTTP header field was received: frame type: " - "%u, stream: %d, name: [%.*s], value: [%.*s]", - frame->hd.type, frame->hd.stream_id, (int)nv.name->len, - nv.name->base, (int)nv.value->len, nv.value->base); - - if (nghttp2_is_fatal(rv)) { - return rv; - } - - rv = session_handle_invalid_stream2( - session, subject_stream->stream_id, frame, - NGHTTP2_ERR_HTTP_HEADER); - if (nghttp2_is_fatal(rv)) { - return rv; - } - - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + rv = NGHTTP2_ERR_HTTP_HEADER; } else { if (rv2 != 0) { return rv2; @@ -3631,8 +3603,13 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, return rv; } - return nghttp2_session_terminate_session(session, - NGHTTP2_PROTOCOL_ERROR); + rv = + session_handle_invalid_stream2(session, subject_stream->stream_id, + frame, NGHTTP2_ERR_HTTP_HEADER); + if (nghttp2_is_fatal(rv)) { + return rv; + } + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } } if (rv == 0) { @@ -3745,7 +3722,27 @@ static int session_after_header_block_received(nghttp2_session *session) { } } if (rv != 0) { - return nghttp2_session_terminate_session(session, NGHTTP2_PROTOCOL_ERROR); + int32_t stream_id; + + if (frame->hd.type == NGHTTP2_PUSH_PROMISE) { + stream_id = frame->push_promise.promised_stream_id; + } else { + stream_id = frame->hd.stream_id; + } + + rv = session_handle_invalid_stream2(session, stream_id, frame, + NGHTTP2_ERR_HTTP_MESSAGING); + if (nghttp2_is_fatal(rv)) { + return rv; + } + + if (frame->hd.type == NGHTTP2_HEADERS && + (frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) { + nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); + /* Don't call nghttp2_session_close_stream_if_shut_rdwr + because RST_STREAM has been submitted. */ + } + return 0; } } @@ -4081,7 +4078,8 @@ static int update_remote_initial_window_size_func(void *entry, void *ptr) { rv = nghttp2_stream_update_remote_initial_window_size( stream, arg->new_window_size, arg->old_window_size); if (rv != 0) { - return NGHTTP2_ERR_FLOW_CONTROL; + return nghttp2_session_add_rst_stream(arg->session, stream->stream_id, + NGHTTP2_FLOW_CONTROL_ERROR); } /* If window size gets positive, push deferred DATA frame to @@ -4107,8 +4105,6 @@ static int update_remote_initial_window_size_func(void *entry, void *ptr) { * * NGHTTP2_ERR_NOMEM * Out of memory. - * NGHTTP2_ERR_FLOW_CONTROL - * Window size gets out of range. */ static int session_update_remote_initial_window_size(nghttp2_session *session, @@ -4132,7 +4128,8 @@ static int update_local_initial_window_size_func(void *entry, void *ptr) { rv = nghttp2_stream_update_local_initial_window_size( stream, arg->new_window_size, arg->old_window_size); if (rv != 0) { - return NGHTTP2_ERR_FLOW_CONTROL; + return nghttp2_session_add_rst_stream(arg->session, stream->stream_id, + NGHTTP2_FLOW_CONTROL_ERROR); } if (stream->window_update_queued) { @@ -4166,8 +4163,6 @@ static int update_local_initial_window_size_func(void *entry, void *ptr) { * * NGHTTP2_ERR_NOMEM * Out of memory. - * NGHTTP2_ERR_FLOW_CONTROL - * Window size gets out of range. */ static int session_update_local_initial_window_size(nghttp2_session *session, @@ -4554,9 +4549,9 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session, session->max_incoming_reserved_streams) { /* Currently, client does not retain closed stream, so we don't check NGHTTP2_SHUT_RD condition here. */ - rv = session_handle_invalid_stream2(session, - frame->push_promise.promised_stream_id, - NULL, NGHTTP2_ERR_PUSH_CANCEL); + + rv = nghttp2_session_add_rst_stream( + session, frame->push_promise.promised_stream_id, NGHTTP2_CANCEL); if (rv != 0) { return rv; } @@ -4713,9 +4708,8 @@ static int session_on_stream_window_update_received(nghttp2_session *session, } if (NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < stream->remote_window_size) { - return session_handle_invalid_connection( - session, frame, NGHTTP2_ERR_FLOW_CONTROL, - "WINDOW_UPDATE: window size overflow"); + return session_handle_invalid_stream(session, frame, + NGHTTP2_ERR_FLOW_CONTROL); } stream->remote_window_size += frame->window_update.window_size_increment; @@ -4945,7 +4939,16 @@ int nghttp2_session_on_data_received(nghttp2_session *session, if (session_enforce_http_messaging(session) && (frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) { if (nghttp2_http_on_remote_end_stream(stream) != 0) { - return nghttp2_session_terminate_session(session, NGHTTP2_PROTOCOL_ERROR); + rv = nghttp2_session_add_rst_stream(session, stream->stream_id, + NGHTTP2_PROTOCOL_ERROR); + if (nghttp2_is_fatal(rv)) { + return rv; + } + + nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); + /* Don't call nghttp2_session_close_stream_if_shut_rdwr because + RST_STREAM has been submitted. */ + return 0; } } @@ -5003,8 +5006,8 @@ int nghttp2_session_update_recv_stream_window_size(nghttp2_session *session, rv = adjust_recv_window_size(&stream->recv_window_size, delta_size, stream->local_window_size); if (rv != 0) { - return nghttp2_session_terminate_session(session, - NGHTTP2_FLOW_CONTROL_ERROR); + return nghttp2_session_add_rst_stream(session, stream->stream_id, + NGHTTP2_FLOW_CONTROL_ERROR); } /* We don't have to send WINDOW_UPDATE if the data received is the last chunk in the incoming stream. */ @@ -5587,8 +5590,8 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, } if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - rv = session_handle_invalid_stream2( - session, iframe->frame.hd.stream_id, NULL, NGHTTP2_ERR_INTERNAL); + rv = nghttp2_session_add_rst_stream( + session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6052,8 +6055,8 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, } if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - rv = session_handle_invalid_stream2( - session, iframe->frame.hd.stream_id, NULL, NGHTTP2_ERR_INTERNAL); + rv = nghttp2_session_add_rst_stream( + session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6136,9 +6139,9 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, } if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - rv = session_handle_invalid_stream2( - session, iframe->frame.push_promise.promised_stream_id, NULL, - NGHTTP2_ERR_INTERNAL); + rv = nghttp2_session_add_rst_stream( + session, iframe->frame.push_promise.promised_stream_id, + NGHTTP2_INTERNAL_ERROR); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6316,12 +6319,12 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, iframe->payloadleft -= hd_proclen; /* Use promised stream ID for PUSH_PROMISE */ - rv = session_handle_invalid_stream2( + rv = nghttp2_session_add_rst_stream( session, iframe->frame.hd.type == NGHTTP2_PUSH_PROMISE ? iframe->frame.push_promise.promised_stream_id : iframe->frame.hd.stream_id, - NULL, NGHTTP2_ERR_INTERNAL); + NGHTTP2_INTERNAL_ERROR); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6368,10 +6371,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, if (nghttp2_is_fatal(rv)) { return rv; } - - if (iframe->state == NGHTTP2_IB_IGN_ALL) { - return (nghttp2_ssize)inlen; - } } session_inbound_frame_reset(session); @@ -6597,10 +6596,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, if (nghttp2_is_fatal(rv)) { return rv; } - - if (iframe->state == NGHTTP2_IB_IGN_ALL) { - return (nghttp2_ssize)inlen; - } } busy = 1; @@ -6673,10 +6668,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, return rv; } - if (iframe->state == NGHTTP2_IB_IGN_ALL) { - return (nghttp2_ssize)inlen; - } - data_readlen = inbound_frame_effective_readlen(iframe, iframe->payloadleft, readlen); @@ -6706,13 +6697,28 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, if (data_readlen > 0) { if (session_enforce_http_messaging(session)) { if (nghttp2_http_on_data_chunk(stream, (size_t)data_readlen) != 0) { - rv = nghttp2_session_terminate_session(session, - NGHTTP2_PROTOCOL_ERROR); + if (session->opt_flags & NGHTTP2_OPTMASK_NO_AUTO_WINDOW_UPDATE) { + /* Consume all data for connection immediately here */ + rv = session_update_connection_consumed_size( + session, (size_t)data_readlen); + + if (nghttp2_is_fatal(rv)) { + return rv; + } + + if (iframe->state == NGHTTP2_IB_IGN_DATA) { + return (nghttp2_ssize)inlen; + } + } + + rv = nghttp2_session_add_rst_stream( + session, iframe->frame.hd.stream_id, NGHTTP2_PROTOCOL_ERROR); if (nghttp2_is_fatal(rv)) { return rv; } - - return (nghttp2_ssize)inlen; + busy = 1; + iframe->state = NGHTTP2_IB_IGN_DATA; + break; } } if (session->callbacks.on_data_chunk_recv_callback) { @@ -6739,10 +6745,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session, return rv; } - if (iframe->state == NGHTTP2_IB_IGN_ALL) { - return (nghttp2_ssize)inlen; - } - session_inbound_frame_reset(session); break;