Allow NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE from nghttp2_on_begin_headers_callback
Since application most likely allocates the stream object in nghttp2_on_begin_headers_callback, it is desirable to handle its failure as stream error. But previously it only signals success or fatal error. Submitting RST_STREAM does not prevent nghttp2_on_header_callback from being invoked. This commit improves this situation by allowing NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE from nghttp2_on_begin_headers_callback. If that value is returned, library submits RST_STREAM with error code INTERNAL_ERROR, and nghttp2_on_header_callback and nghttp2_on_frame_recv_callback for that frame are not invoked. Note that for PUSH_PROMISE frame, the stream to be reset is promised stream.
This commit is contained in:
parent
b9f602b9a2
commit
514558afc0
|
@ -1502,11 +1502,24 @@ typedef int (*nghttp2_on_stream_close_callback)(nghttp2_session *session,
|
|||
* trailer headers also has ``frame->headers.cat ==
|
||||
* NGHTTP2_HCAT_HEADERS`` which does not contain any status code.
|
||||
*
|
||||
* The implementation of this function must return 0 if it succeeds or
|
||||
* :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. If nonzero value other than
|
||||
* :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` is returned, it is treated as
|
||||
* if :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` is returned. If
|
||||
* :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` is returned,
|
||||
* Returning :enum:`NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE` will close
|
||||
* the stream (promised stream if frame is PUSH_PROMISE) by issuing
|
||||
* RST_STREAM with :enum:`NGHTTP2_INTERNAL_ERROR`. In this case,
|
||||
* :type:`nghttp2_on_header_callback` and
|
||||
* :type:`nghttp2_on_frame_recv_callback` will not be invoked. If a
|
||||
* different error code is desirable, use
|
||||
* `nghttp2_submit_rst_stream()` with a desired error code and then
|
||||
* return :enum:`NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE`. Again, use
|
||||
* `frame->push_promise.promised_stream_id` as stream_id parameter in
|
||||
* `nghttp2_submit_rst_stream()` if frame is PUSH_PROMISE.
|
||||
*
|
||||
* The implementation of this function must return 0 if it succeeds.
|
||||
* It can return :enum:`NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE` to
|
||||
* reset the stream (promised stream if frame is PUSH_PROMISE). For
|
||||
* critical errors, it must return
|
||||
* :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. If the other value is
|
||||
* returned, it is treated as if :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`
|
||||
* is returned. If :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` is returned,
|
||||
* `nghttp2_session_mem_recv()` function will immediately return
|
||||
* :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`.
|
||||
*
|
||||
|
|
|
@ -3069,6 +3069,9 @@ static int session_call_on_begin_headers(nghttp2_session *session,
|
|||
if (session->callbacks.on_begin_headers_callback) {
|
||||
rv = session->callbacks.on_begin_headers_callback(session, frame,
|
||||
session->user_data);
|
||||
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
|
||||
return rv;
|
||||
}
|
||||
if (rv != 0) {
|
||||
return NGHTTP2_ERR_CALLBACK_FAILURE;
|
||||
}
|
||||
|
@ -5085,6 +5088,16 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
|
|||
|
||||
busy = 1;
|
||||
|
||||
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
|
||||
rv = nghttp2_session_add_rst_stream(
|
||||
session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR);
|
||||
if (nghttp2_is_fatal(rv)) {
|
||||
return rv;
|
||||
}
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
}
|
||||
|
||||
if (rv == NGHTTP2_ERR_IGN_HEADER_BLOCK) {
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
|
@ -5329,6 +5342,16 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
|
|||
|
||||
busy = 1;
|
||||
|
||||
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
|
||||
rv = nghttp2_session_add_rst_stream(
|
||||
session, iframe->frame.hd.stream_id, NGHTTP2_INTERNAL_ERROR);
|
||||
if (nghttp2_is_fatal(rv)) {
|
||||
return rv;
|
||||
}
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
}
|
||||
|
||||
if (rv == NGHTTP2_ERR_IGN_HEADER_BLOCK) {
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
|
@ -5393,6 +5416,17 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
|
|||
|
||||
busy = 1;
|
||||
|
||||
if (rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
|
||||
rv = nghttp2_session_add_rst_stream(
|
||||
session, iframe->frame.push_promise.promised_stream_id,
|
||||
NGHTTP2_INTERNAL_ERROR);
|
||||
if (nghttp2_is_fatal(rv)) {
|
||||
return rv;
|
||||
}
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
}
|
||||
|
||||
if (rv == NGHTTP2_ERR_IGN_HEADER_BLOCK) {
|
||||
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
|
||||
break;
|
||||
|
|
|
@ -262,6 +262,8 @@ int main(int argc _U_, char *argv[] _U_) {
|
|||
test_nghttp2_session_reset_pending_headers) ||
|
||||
!CU_add_test(pSuite, "session_send_data_callback",
|
||||
test_nghttp2_session_send_data_callback) ||
|
||||
!CU_add_test(pSuite, "session_on_begin_headers_temporal_failure",
|
||||
test_nghttp2_session_on_begin_headers_temporal_failure) ||
|
||||
!CU_add_test(pSuite, "http_mandatory_headers",
|
||||
test_nghttp2_http_mandatory_headers) ||
|
||||
!CU_add_test(pSuite, "http_content_length",
|
||||
|
|
|
@ -405,6 +405,12 @@ static int on_begin_headers_callback(nghttp2_session *session _U_,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int temporal_failure_on_begin_headers_callback(
|
||||
nghttp2_session *session, const nghttp2_frame *frame, void *user_data) {
|
||||
on_begin_headers_callback(session, frame, user_data);
|
||||
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
|
||||
}
|
||||
|
||||
static ssize_t defer_data_source_read_callback(nghttp2_session *session _U_,
|
||||
int32_t stream_id _U_,
|
||||
uint8_t *buf _U_, size_t len _U_,
|
||||
|
@ -7033,6 +7039,78 @@ void test_nghttp2_session_send_data_callback(void) {
|
|||
nghttp2_session_del(session);
|
||||
}
|
||||
|
||||
void test_nghttp2_session_on_begin_headers_temporal_failure(void) {
|
||||
nghttp2_session *session;
|
||||
nghttp2_session_callbacks callbacks;
|
||||
my_user_data ud;
|
||||
nghttp2_bufs bufs;
|
||||
nghttp2_mem *mem;
|
||||
ssize_t rv;
|
||||
nghttp2_hd_deflater deflater;
|
||||
nghttp2_outbound_item *item;
|
||||
|
||||
mem = nghttp2_mem_default();
|
||||
frame_pack_bufs_init(&bufs);
|
||||
nghttp2_hd_deflate_init(&deflater, mem);
|
||||
|
||||
memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
|
||||
callbacks.on_begin_headers_callback =
|
||||
temporal_failure_on_begin_headers_callback;
|
||||
callbacks.on_header_callback = on_header_callback;
|
||||
callbacks.on_frame_recv_callback = on_frame_recv_callback;
|
||||
callbacks.send_callback = null_send_callback;
|
||||
nghttp2_session_server_new(&session, &callbacks, &ud);
|
||||
|
||||
rv = pack_headers(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, reqnv,
|
||||
ARRLEN(reqnv), mem);
|
||||
CU_ASSERT(0 == rv);
|
||||
|
||||
ud.header_cb_called = 0;
|
||||
ud.frame_recv_cb_called = 0;
|
||||
rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
|
||||
nghttp2_bufs_len(&bufs));
|
||||
CU_ASSERT(nghttp2_bufs_len(&bufs) == rv);
|
||||
CU_ASSERT(0 == ud.header_cb_called);
|
||||
CU_ASSERT(0 == ud.frame_recv_cb_called);
|
||||
|
||||
item = nghttp2_session_get_next_ob_item(session);
|
||||
CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
|
||||
CU_ASSERT(1 == item->frame.hd.stream_id);
|
||||
CU_ASSERT(NGHTTP2_INTERNAL_ERROR == item->frame.rst_stream.error_code);
|
||||
|
||||
nghttp2_session_del(session);
|
||||
nghttp2_hd_deflate_free(&deflater);
|
||||
|
||||
nghttp2_bufs_reset(&bufs);
|
||||
/* check for PUSH_PROMISE */
|
||||
nghttp2_hd_deflate_init(&deflater, mem);
|
||||
nghttp2_session_client_new(&session, &callbacks, &ud);
|
||||
|
||||
nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE,
|
||||
&pri_spec_default, NGHTTP2_STREAM_OPENING, NULL);
|
||||
|
||||
rv = pack_push_promise(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, 2,
|
||||
reqnv, ARRLEN(reqnv), mem);
|
||||
CU_ASSERT(0 == rv);
|
||||
|
||||
ud.header_cb_called = 0;
|
||||
ud.frame_recv_cb_called = 0;
|
||||
rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
|
||||
nghttp2_bufs_len(&bufs));
|
||||
CU_ASSERT(nghttp2_bufs_len(&bufs) == rv);
|
||||
CU_ASSERT(0 == ud.header_cb_called);
|
||||
CU_ASSERT(0 == ud.frame_recv_cb_called);
|
||||
|
||||
item = nghttp2_session_get_next_ob_item(session);
|
||||
CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
|
||||
CU_ASSERT(2 == item->frame.hd.stream_id);
|
||||
CU_ASSERT(NGHTTP2_INTERNAL_ERROR == item->frame.rst_stream.error_code);
|
||||
|
||||
nghttp2_session_del(session);
|
||||
nghttp2_hd_deflate_free(&deflater);
|
||||
nghttp2_bufs_free(&bufs);
|
||||
}
|
||||
|
||||
static void check_nghttp2_http_recv_headers_fail(
|
||||
nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id,
|
||||
int stream_state, const nghttp2_nv *nva, size_t nvlen) {
|
||||
|
|
|
@ -124,6 +124,7 @@ void test_nghttp2_session_open_idle_stream(void);
|
|||
void test_nghttp2_session_cancel_reserved_remote(void);
|
||||
void test_nghttp2_session_reset_pending_headers(void);
|
||||
void test_nghttp2_session_send_data_callback(void);
|
||||
void test_nghttp2_session_on_begin_headers_temporal_failure(void);
|
||||
void test_nghttp2_http_mandatory_headers(void);
|
||||
void test_nghttp2_http_content_length(void);
|
||||
void test_nghttp2_http_content_length_mismatch(void);
|
||||
|
|
Loading…
Reference in New Issue