Fix the bug that stream is closed with wrong error code

This commit fixes the bug that stream is closed with wrong error code
(0).  This happens when STREAM or DATA frame with END_STREAM flag set
is received and it violates HTTP messaging rule (i.e., content-length
does not match) and the other side of stream has been closed.  In this
case, nghttp2_on_stream_close_callback should be called with nonzero
error code, but previously it is called with 0 (NO_ERROR).
This commit is contained in:
Tatsuhiro Tsujikawa 2019-10-29 00:58:43 +09:00
parent d08c43951f
commit 6ce4835eea
2 changed files with 111 additions and 15 deletions

View File

@ -3735,7 +3735,6 @@ static int session_end_stream_headers_received(nghttp2_session *session,
static int session_after_header_block_received(nghttp2_session *session) { static int session_after_header_block_received(nghttp2_session *session) {
int rv = 0; int rv = 0;
int call_cb = 1;
nghttp2_frame *frame = &session->iframe.frame; nghttp2_frame *frame = &session->iframe.frame;
nghttp2_stream *stream; nghttp2_stream *stream;
@ -3789,21 +3788,25 @@ static int session_after_header_block_received(nghttp2_session *session) {
stream_id = frame->hd.stream_id; stream_id = frame->hd.stream_id;
} }
call_cb = 0;
rv = session_handle_invalid_stream2(session, stream_id, frame, rv = session_handle_invalid_stream2(session, stream_id, frame,
NGHTTP2_ERR_HTTP_MESSAGING); NGHTTP2_ERR_HTTP_MESSAGING);
if (nghttp2_is_fatal(rv)) { if (nghttp2_is_fatal(rv)) {
return 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;
} }
} }
if (call_cb) { rv = session_call_on_frame_received(session, frame);
rv = session_call_on_frame_received(session, frame); if (nghttp2_is_fatal(rv)) {
if (nghttp2_is_fatal(rv)) { return rv;
return rv;
}
} }
if (frame->hd.type != NGHTTP2_HEADERS) { if (frame->hd.type != NGHTTP2_HEADERS) {
@ -4942,7 +4945,6 @@ static int session_process_extension_frame(nghttp2_session *session) {
int nghttp2_session_on_data_received(nghttp2_session *session, int nghttp2_session_on_data_received(nghttp2_session *session,
nghttp2_frame *frame) { nghttp2_frame *frame) {
int rv = 0; int rv = 0;
int call_cb = 1;
nghttp2_stream *stream; nghttp2_stream *stream;
/* We don't call on_frame_recv_callback if stream has been closed /* We don't call on_frame_recv_callback if stream has been closed
@ -4958,20 +4960,22 @@ int nghttp2_session_on_data_received(nghttp2_session *session,
if (session_enforce_http_messaging(session) && if (session_enforce_http_messaging(session) &&
(frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) { (frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) {
if (nghttp2_http_on_remote_end_stream(stream) != 0) { if (nghttp2_http_on_remote_end_stream(stream) != 0) {
call_cb = 0;
rv = nghttp2_session_add_rst_stream(session, stream->stream_id, rv = nghttp2_session_add_rst_stream(session, stream->stream_id,
NGHTTP2_PROTOCOL_ERROR); NGHTTP2_PROTOCOL_ERROR);
if (nghttp2_is_fatal(rv)) { if (nghttp2_is_fatal(rv)) {
return 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;
} }
} }
if (call_cb) { rv = session_call_on_frame_received(session, frame);
rv = session_call_on_frame_received(session, frame); if (nghttp2_is_fatal(rv)) {
if (nghttp2_is_fatal(rv)) { return rv;
return rv;
}
} }
if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {

View File

@ -11288,6 +11288,8 @@ void test_nghttp2_http_content_length_mismatch(void) {
MAKE_NV(":path", "/"), MAKE_NV(":method", "PUT"), MAKE_NV(":path", "/"), MAKE_NV(":method", "PUT"),
MAKE_NV(":authority", "localhost"), MAKE_NV(":scheme", "https"), MAKE_NV(":authority", "localhost"), MAKE_NV(":scheme", "https"),
MAKE_NV("content-length", "20")}; MAKE_NV("content-length", "20")};
const nghttp2_nv cl_resnv[] = {MAKE_NV(":status", "200"),
MAKE_NV("content-length", "20")};
nghttp2_outbound_item *item; nghttp2_outbound_item *item;
nghttp2_frame_hd hd; nghttp2_frame_hd hd;
@ -11365,7 +11367,97 @@ void test_nghttp2_http_content_length_mismatch(void) {
nghttp2_session_del(session); nghttp2_session_del(session);
/* Check for client */
nghttp2_session_client_new(&session, &callbacks, NULL);
nghttp2_hd_deflate_init(&deflater, mem);
/* header says content-length: 20, but HEADERS has END_STREAM flag set */
nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
CU_ASSERT(0 == nghttp2_session_send(session));
rv = pack_headers(&bufs, &deflater, 1,
NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM,
cl_resnv, ARRLEN(cl_resnv), mem);
CU_ASSERT(0 == rv);
rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
nghttp2_buf_len(&bufs.head->buf));
CU_ASSERT((ssize_t)nghttp2_buf_len(&bufs.head->buf) == rv);
item = nghttp2_session_get_next_ob_item(session);
CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
CU_ASSERT(NULL != nghttp2_session_get_stream(session, 1));
CU_ASSERT(0 == nghttp2_session_send(session));
/* After sending RST_STREAM, stream must be closed */
CU_ASSERT(NULL == nghttp2_session_get_stream(session, 1));
nghttp2_bufs_reset(&bufs);
/* header says content-length: 20, but DATA has 0 byte */
nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
CU_ASSERT(0 == nghttp2_session_send(session));
rv = pack_headers(&bufs, &deflater, 3, NGHTTP2_FLAG_END_HEADERS, cl_resnv,
ARRLEN(cl_resnv), mem);
CU_ASSERT(0 == rv);
nghttp2_frame_hd_init(&hd, 0, NGHTTP2_DATA, NGHTTP2_FLAG_END_STREAM, 3);
nghttp2_frame_pack_frame_hd(bufs.head->buf.last, &hd);
bufs.head->buf.last += NGHTTP2_FRAME_HDLEN;
rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
nghttp2_buf_len(&bufs.head->buf));
CU_ASSERT((ssize_t)nghttp2_buf_len(&bufs.head->buf) == rv);
item = nghttp2_session_get_next_ob_item(session);
CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
CU_ASSERT(NULL != nghttp2_session_get_stream(session, 3));
CU_ASSERT(0 == nghttp2_session_send(session));
/* After sending RST_STREAM, stream must be closed */
CU_ASSERT(NULL == nghttp2_session_get_stream(session, 3));
nghttp2_bufs_reset(&bufs);
/* header says content-length: 20, but DATA has 21 bytes */
nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
CU_ASSERT(0 == nghttp2_session_send(session));
rv = pack_headers(&bufs, &deflater, 5, NGHTTP2_FLAG_END_HEADERS, cl_resnv,
ARRLEN(cl_resnv), mem);
CU_ASSERT(0 == rv);
nghttp2_frame_hd_init(&hd, 21, NGHTTP2_DATA, NGHTTP2_FLAG_END_STREAM, 5);
nghttp2_frame_pack_frame_hd(bufs.head->buf.last, &hd);
bufs.head->buf.last += NGHTTP2_FRAME_HDLEN + 21;
rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
nghttp2_buf_len(&bufs.head->buf));
CU_ASSERT((ssize_t)nghttp2_buf_len(&bufs.head->buf) == rv);
item = nghttp2_session_get_next_ob_item(session);
CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
CU_ASSERT(NULL != nghttp2_session_get_stream(session, 5));
CU_ASSERT(0 == nghttp2_session_send(session));
/* After sending RST_STREAM, stream must be closed */
CU_ASSERT(NULL == nghttp2_session_get_stream(session, 5));
nghttp2_bufs_reset(&bufs);
nghttp2_bufs_free(&bufs); nghttp2_bufs_free(&bufs);
nghttp2_hd_deflate_free(&deflater);
nghttp2_session_del(session);
} }
void test_nghttp2_http_non_final_response(void) { void test_nghttp2_http_non_final_response(void) {