Skip to content

Commit fb2a02f

Browse files
aduh95nodejs-github-bot
authored andcommitted
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: nghttp2/nghttp2@7784fa9 Refs: #60661 PR-URL: #61136 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 1d54921 commit fb2a02f

File tree

2 files changed

+81
-83
lines changed

2 files changed

+81
-83
lines changed

deps/nghttp2/lib/nghttp2_int.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@ typedef enum {
5252
* Unlike NGHTTP2_ERR_IGN_HTTP_HEADER, this does not invoke
5353
* nghttp2_on_invalid_header_callback.
5454
*/
55-
NGHTTP2_ERR_REMOVE_HTTP_HEADER = -106,
56-
/*
57-
* Cancel pushed stream.
58-
*/
59-
NGHTTP2_ERR_PUSH_CANCEL = -107,
55+
NGHTTP2_ERR_REMOVE_HTTP_HEADER = -106
6056
} nghttp2_internal_error;
6157

6258
#endif /* !defined(NGHTTP2_INT_H) */

deps/nghttp2/lib/nghttp2_session.c

Lines changed: 80 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -3272,9 +3272,7 @@ static int session_call_on_invalid_header(nghttp2_session *session,
32723272
session, frame, nv->name->base, nv->name->len, nv->value->base,
32733273
nv->value->len, nv->flags, session->user_data);
32743274
} else {
3275-
/* If both callbacks are not set, the invalid field nv is
3276-
ignored. */
3277-
return 0;
3275+
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
32783276
}
32793277

32803278
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) {
33593357
case NGHTTP2_ERR_HTTP_HEADER:
33603358
case NGHTTP2_ERR_HTTP_MESSAGING:
33613359
return NGHTTP2_PROTOCOL_ERROR;
3362-
case NGHTTP2_ERR_INTERNAL:
3363-
return NGHTTP2_INTERNAL_ERROR;
3364-
case NGHTTP2_ERR_PUSH_CANCEL:
3365-
return NGHTTP2_CANCEL;
33663360
default:
33673361
return NGHTTP2_INTERNAL_ERROR;
33683362
}
@@ -3414,7 +3408,7 @@ static int session_handle_invalid_stream2(nghttp2_session *session,
34143408
if (rv != 0) {
34153409
return rv;
34163410
}
3417-
if (frame && session->callbacks.on_invalid_frame_recv_callback) {
3411+
if (session->callbacks.on_invalid_frame_recv_callback) {
34183412
if (session->callbacks.on_invalid_frame_recv_callback(
34193413
session, frame, lib_error_code, session->user_data) != 0) {
34203414
return NGHTTP2_ERR_CALLBACK_FAILURE;
@@ -3569,29 +3563,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
35693563

35703564
rv2 = session_call_on_invalid_header(session, frame, &nv);
35713565
if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
3572-
DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n",
3573-
frame->hd.type, frame->hd.stream_id, (int)nv.name->len,
3574-
nv.name->base, (int)nv.value->len, nv.value->base);
3575-
3576-
rv = session_call_error_callback(
3577-
session, NGHTTP2_ERR_HTTP_HEADER,
3578-
"Invalid HTTP header field was received: frame type: "
3579-
"%u, stream: %d, name: [%.*s], value: [%.*s]",
3580-
frame->hd.type, frame->hd.stream_id, (int)nv.name->len,
3581-
nv.name->base, (int)nv.value->len, nv.value->base);
3582-
3583-
if (nghttp2_is_fatal(rv)) {
3584-
return rv;
3585-
}
3586-
3587-
rv = session_handle_invalid_stream2(
3588-
session, subject_stream->stream_id, frame,
3589-
NGHTTP2_ERR_HTTP_HEADER);
3590-
if (nghttp2_is_fatal(rv)) {
3591-
return rv;
3592-
}
3593-
3594-
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
3566+
rv = NGHTTP2_ERR_HTTP_HEADER;
35953567
} else {
35963568
if (rv2 != 0) {
35973569
return rv2;
@@ -3631,8 +3603,13 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
36313603
return rv;
36323604
}
36333605

3634-
return nghttp2_session_terminate_session(session,
3635-
NGHTTP2_PROTOCOL_ERROR);
3606+
rv =
3607+
session_handle_invalid_stream2(session, subject_stream->stream_id,
3608+
frame, NGHTTP2_ERR_HTTP_HEADER);
3609+
if (nghttp2_is_fatal(rv)) {
3610+
return rv;
3611+
}
3612+
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
36363613
}
36373614
}
36383615
if (rv == 0) {
@@ -3745,7 +3722,27 @@ static int session_after_header_block_received(nghttp2_session *session) {
37453722
}
37463723
}
37473724
if (rv != 0) {
3748-
return nghttp2_session_terminate_session(session, NGHTTP2_PROTOCOL_ERROR);
3725+
int32_t stream_id;
3726+
3727+
if (frame->hd.type == NGHTTP2_PUSH_PROMISE) {
3728+
stream_id = frame->push_promise.promised_stream_id;
3729+
} else {
3730+
stream_id = frame->hd.stream_id;
3731+
}
3732+
3733+
rv = session_handle_invalid_stream2(session, stream_id, frame,
3734+
NGHTTP2_ERR_HTTP_MESSAGING);
3735+
if (nghttp2_is_fatal(rv)) {
3736+
return rv;
3737+
}
3738+
3739+
if (frame->hd.type == NGHTTP2_HEADERS &&
3740+
(frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) {
3741+
nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD);
3742+
/* Don't call nghttp2_session_close_stream_if_shut_rdwr
3743+
because RST_STREAM has been submitted. */
3744+
}
3745+
return 0;
37493746
}
37503747
}
37513748

@@ -4081,7 +4078,8 @@ static int update_remote_initial_window_size_func(void *entry, void *ptr) {
40814078
rv = nghttp2_stream_update_remote_initial_window_size(
40824079
stream, arg->new_window_size, arg->old_window_size);
40834080
if (rv != 0) {
4084-
return NGHTTP2_ERR_FLOW_CONTROL;
4081+
return nghttp2_session_add_rst_stream(arg->session, stream->stream_id,
4082+
NGHTTP2_FLOW_CONTROL_ERROR);
40854083
}
40864084

40874085
/* 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) {
41074105
*
41084106
* NGHTTP2_ERR_NOMEM
41094107
* Out of memory.
4110-
* NGHTTP2_ERR_FLOW_CONTROL
4111-
* Window size gets out of range.
41124108
*/
41134109
static int
41144110
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) {
41324128
rv = nghttp2_stream_update_local_initial_window_size(
41334129
stream, arg->new_window_size, arg->old_window_size);
41344130
if (rv != 0) {
4135-
return NGHTTP2_ERR_FLOW_CONTROL;
4131+
return nghttp2_session_add_rst_stream(arg->session, stream->stream_id,
4132+
NGHTTP2_FLOW_CONTROL_ERROR);
41364133
}
41374134

41384135
if (stream->window_update_queued) {
@@ -4166,8 +4163,6 @@ static int update_local_initial_window_size_func(void *entry, void *ptr) {
41664163
*
41674164
* NGHTTP2_ERR_NOMEM
41684165
* Out of memory.
4169-
* NGHTTP2_ERR_FLOW_CONTROL
4170-
* Window size gets out of range.
41714166
*/
41724167
static int
41734168
session_update_local_initial_window_size(nghttp2_session *session,
@@ -4554,9 +4549,9 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session,
45544549
session->max_incoming_reserved_streams) {
45554550
/* Currently, client does not retain closed stream, so we don't
45564551
check NGHTTP2_SHUT_RD condition here. */
4557-
rv = session_handle_invalid_stream2(session,
4558-
frame->push_promise.promised_stream_id,
4559-
NULL, NGHTTP2_ERR_PUSH_CANCEL);
4552+
4553+
rv = nghttp2_session_add_rst_stream(
4554+
session, frame->push_promise.promised_stream_id, NGHTTP2_CANCEL);
45604555
if (rv != 0) {
45614556
return rv;
45624557
}
@@ -4713,9 +4708,8 @@ static int session_on_stream_window_update_received(nghttp2_session *session,
47134708
}
47144709
if (NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment <
47154710
stream->remote_window_size) {
4716-
return session_handle_invalid_connection(
4717-
session, frame, NGHTTP2_ERR_FLOW_CONTROL,
4718-
"WINDOW_UPDATE: window size overflow");
4711+
return session_handle_invalid_stream(session, frame,
4712+
NGHTTP2_ERR_FLOW_CONTROL);
47194713
}
47204714
stream->remote_window_size += frame->window_update.window_size_increment;
47214715

@@ -4945,7 +4939,16 @@ int nghttp2_session_on_data_received(nghttp2_session *session,
49454939
if (session_enforce_http_messaging(session) &&
49464940
(frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) {
49474941
if (nghttp2_http_on_remote_end_stream(stream) != 0) {
4948-
return nghttp2_session_terminate_session(session, NGHTTP2_PROTOCOL_ERROR);
4942+
rv = nghttp2_session_add_rst_stream(session, stream->stream_id,
4943+
NGHTTP2_PROTOCOL_ERROR);
4944+
if (nghttp2_is_fatal(rv)) {
4945+
return rv;
4946+
}
4947+
4948+
nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD);
4949+
/* Don't call nghttp2_session_close_stream_if_shut_rdwr because
4950+
RST_STREAM has been submitted. */
4951+
return 0;
49494952
}
49504953
}
49514954

@@ -5003,8 +5006,8 @@ int nghttp2_session_update_recv_stream_window_size(nghttp2_session *session,
50035006
rv = adjust_recv_window_size(&stream->recv_window_size, delta_size,
50045007
stream->local_window_size);
50055008
if (rv != 0) {
5006-
return nghttp2_session_terminate_session(session,
5007-
NGHTTP2_FLOW_CONTROL_ERROR);
5009+
return nghttp2_session_add_rst_stream(session, stream->stream_id,
5010+
NGHTTP2_FLOW_CONTROL_ERROR);
50085011
}
50095012
/* We don't have to send WINDOW_UPDATE if the data received is the
50105013
last chunk in the incoming stream. */
@@ -5587,8 +5590,8 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
55875590
}
55885591

55895592
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
5590-
rv = session_handle_invalid_stream2(
5591-
session, iframe->frame.hd.stream_id, NULL, NGHTTP2_ERR_INTERNAL);
5593+
rv = nghttp2_session_add_rst_stream(
5594+
session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR);
55925595
if (nghttp2_is_fatal(rv)) {
55935596
return rv;
55945597
}
@@ -6104,8 +6107,8 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
61046107
}
61056108

61066109
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
6107-
rv = session_handle_invalid_stream2(
6108-
session, iframe->frame.hd.stream_id, NULL, NGHTTP2_ERR_INTERNAL);
6110+
rv = nghttp2_session_add_rst_stream(
6111+
session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR);
61096112
if (nghttp2_is_fatal(rv)) {
61106113
return rv;
61116114
}
@@ -6188,9 +6191,9 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
61886191
}
61896192

61906193
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
6191-
rv = session_handle_invalid_stream2(
6192-
session, iframe->frame.push_promise.promised_stream_id, NULL,
6193-
NGHTTP2_ERR_INTERNAL);
6194+
rv = nghttp2_session_add_rst_stream(
6195+
session, iframe->frame.push_promise.promised_stream_id,
6196+
NGHTTP2_INTERNAL_ERROR);
61946197
if (nghttp2_is_fatal(rv)) {
61956198
return rv;
61966199
}
@@ -6368,12 +6371,12 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
63686371
iframe->payloadleft -= hd_proclen;
63696372

63706373
/* Use promised stream ID for PUSH_PROMISE */
6371-
rv = session_handle_invalid_stream2(
6374+
rv = nghttp2_session_add_rst_stream(
63726375
session,
63736376
iframe->frame.hd.type == NGHTTP2_PUSH_PROMISE
63746377
? iframe->frame.push_promise.promised_stream_id
63756378
: iframe->frame.hd.stream_id,
6376-
NULL, NGHTTP2_ERR_INTERNAL);
6379+
NGHTTP2_INTERNAL_ERROR);
63776380
if (nghttp2_is_fatal(rv)) {
63786381
return rv;
63796382
}
@@ -6420,10 +6423,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
64206423
if (nghttp2_is_fatal(rv)) {
64216424
return rv;
64226425
}
6423-
6424-
if (iframe->state == NGHTTP2_IB_IGN_ALL) {
6425-
return (nghttp2_ssize)inlen;
6426-
}
64276426
}
64286427
session_inbound_frame_reset(session);
64296428

@@ -6649,10 +6648,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
66496648
if (nghttp2_is_fatal(rv)) {
66506649
return rv;
66516650
}
6652-
6653-
if (iframe->state == NGHTTP2_IB_IGN_ALL) {
6654-
return (nghttp2_ssize)inlen;
6655-
}
66566651
}
66576652

66586653
busy = 1;
@@ -6725,10 +6720,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
67256720
return rv;
67266721
}
67276722

6728-
if (iframe->state == NGHTTP2_IB_IGN_ALL) {
6729-
return (nghttp2_ssize)inlen;
6730-
}
6731-
67326723
data_readlen =
67336724
inbound_frame_effective_readlen(iframe, iframe->payloadleft, readlen);
67346725

@@ -6758,13 +6749,28 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
67586749
if (data_readlen > 0) {
67596750
if (session_enforce_http_messaging(session)) {
67606751
if (nghttp2_http_on_data_chunk(stream, (size_t)data_readlen) != 0) {
6761-
rv = nghttp2_session_terminate_session(session,
6762-
NGHTTP2_PROTOCOL_ERROR);
6752+
if (session->opt_flags & NGHTTP2_OPTMASK_NO_AUTO_WINDOW_UPDATE) {
6753+
/* Consume all data for connection immediately here */
6754+
rv = session_update_connection_consumed_size(
6755+
session, (size_t)data_readlen);
6756+
6757+
if (nghttp2_is_fatal(rv)) {
6758+
return rv;
6759+
}
6760+
6761+
if (iframe->state == NGHTTP2_IB_IGN_DATA) {
6762+
return (nghttp2_ssize)inlen;
6763+
}
6764+
}
6765+
6766+
rv = nghttp2_session_add_rst_stream(
6767+
session, iframe->frame.hd.stream_id, NGHTTP2_PROTOCOL_ERROR);
67636768
if (nghttp2_is_fatal(rv)) {
67646769
return rv;
67656770
}
6766-
6767-
return (nghttp2_ssize)inlen;
6771+
busy = 1;
6772+
iframe->state = NGHTTP2_IB_IGN_DATA;
6773+
break;
67686774
}
67696775
}
67706776
if (session->callbacks.on_data_chunk_recv_callback) {
@@ -6791,10 +6797,6 @@ nghttp2_ssize nghttp2_session_mem_recv2(nghttp2_session *session,
67916797
return rv;
67926798
}
67936799

6794-
if (iframe->state == NGHTTP2_IB_IGN_ALL) {
6795-
return (nghttp2_ssize)inlen;
6796-
}
6797-
67986800
session_inbound_frame_reset(session);
67996801

68006802
break;

0 commit comments

Comments
 (0)