Fix bug that NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE causes session failure

Previously returning NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE from
on_header_callback moves input offset badly and it causes header
decompression error on the subsequent frames.  This commit fix this
bug.
This commit is contained in:
Tatsuhiro Tsujikawa 2014-08-24 00:50:55 +09:00
parent 02c5621c61
commit 7bfa276e96
4 changed files with 94 additions and 3 deletions

View File

@ -2656,6 +2656,9 @@ static int inflate_header_block(nghttp2_session *session,
in += proclen; in += proclen;
inlen -= proclen; inlen -= proclen;
*readlen_ptr += proclen; *readlen_ptr += proclen;
DEBUGF(fprintf(stderr, "recv: proclen=%zd\n", proclen));
if(call_header_cb && (inflate_flags & NGHTTP2_HD_INFLATE_EMIT)) { if(call_header_cb && (inflate_flags & NGHTTP2_HD_INFLATE_EMIT)) {
rv = session_call_on_header(session, frame, &nv); rv = session_call_on_header(session, frame, &nv);
/* This handles NGHTTP2_ERR_PAUSE and /* This handles NGHTTP2_ERR_PAUSE and
@ -4866,13 +4869,13 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session,
return in - first; return in - first;
} }
in += readlen;
iframe->payloadleft -= readlen;
if(rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { if(rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
/* The application says no more headers. We decompress the /* The application says no more headers. We decompress the
rest of the header block but not invoke on_header_callback rest of the header block but not invoke on_header_callback
and on_frame_recv_callback. */ and on_frame_recv_callback. */
in += hd_proclen;
iframe->payloadleft -= hd_proclen;
rv = nghttp2_session_add_rst_stream(session, rv = nghttp2_session_add_rst_stream(session,
iframe->frame.hd.stream_id, iframe->frame.hd.stream_id,
NGHTTP2_INTERNAL_ERROR); NGHTTP2_INTERNAL_ERROR);
@ -4883,6 +4886,10 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session,
iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK; iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK;
break; break;
} }
in += readlen;
iframe->payloadleft -= readlen;
if(rv == NGHTTP2_ERR_HEADER_COMP) { if(rv == NGHTTP2_ERR_HEADER_COMP) {
/* GOAWAY is already issued */ /* GOAWAY is already issued */
if(iframe->payloadleft == 0) { if(iframe->payloadleft == 0) {

View File

@ -233,6 +233,8 @@ int main(int argc, char* argv[])
test_nghttp2_session_keep_closed_stream) || test_nghttp2_session_keep_closed_stream) ||
!CU_add_test(pSuite, "session_graceful_shutdown", !CU_add_test(pSuite, "session_graceful_shutdown",
test_nghttp2_session_graceful_shutdown) || test_nghttp2_session_graceful_shutdown) ||
!CU_add_test(pSuite, "session_on_header_temporal_failure",
test_nghttp2_session_on_header_temporal_failure) ||
!CU_add_test(pSuite, "frame_pack_headers", !CU_add_test(pSuite, "frame_pack_headers",
test_nghttp2_frame_pack_headers) || test_nghttp2_frame_pack_headers) ||
!CU_add_test(pSuite, "frame_pack_headers_frame_too_large", !CU_add_test(pSuite, "frame_pack_headers_frame_too_large",

View File

@ -319,6 +319,19 @@ static int pause_on_header_callback(nghttp2_session *session,
return NGHTTP2_ERR_PAUSE; return NGHTTP2_ERR_PAUSE;
} }
static int temporal_failure_on_header_callback
(nghttp2_session *session,
const nghttp2_frame *frame,
const uint8_t *name, size_t namelen,
const uint8_t *value, size_t valuelen,
uint8_t flags,
void *user_data)
{
on_header_callback(session, frame, name, namelen, value, valuelen, flags,
user_data);
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
static int on_begin_headers_callback(nghttp2_session *session, static int on_begin_headers_callback(nghttp2_session *session,
const nghttp2_frame *frame, const nghttp2_frame *frame,
void *user_data) void *user_data)
@ -6074,3 +6087,71 @@ void test_nghttp2_session_graceful_shutdown(void)
nghttp2_session_del(session); nghttp2_session_del(session);
} }
void test_nghttp2_session_on_header_temporal_failure(void)
{
nghttp2_session *session;
nghttp2_session_callbacks callbacks;
my_user_data ud;
nghttp2_bufs bufs;
nghttp2_buf *buf;
nghttp2_hd_deflater deflater;
nghttp2_nv nv[] = {
MAKE_NV("alpha", "bravo"),
MAKE_NV("charlie", "delta")
};
nghttp2_nv *nva;
size_t hdpos;
ssize_t rv;
nghttp2_frame frame;
nghttp2_frame_hd hd;
nghttp2_outbound_item *item;
memset(&callbacks, 0, sizeof(callbacks));
callbacks.on_header_callback = temporal_failure_on_header_callback;
nghttp2_session_server_new(&session, &callbacks, &ud);
frame_pack_bufs_init(&bufs);
nghttp2_hd_deflate_init(&deflater);
nghttp2_nv_array_copy(&nva, nv, 1);
nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_STREAM, 1,
NGHTTP2_HCAT_REQUEST, NULL, nva, 1);
nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater);
nghttp2_frame_headers_free(&frame.headers);
/* We are going to create CONTINUATION. First serialize header
block, and then frame header. */
hdpos = nghttp2_bufs_len(&bufs);
buf = &bufs.head->buf;
buf->last += NGHTTP2_FRAME_HDLEN;
nghttp2_hd_deflate_hd_bufs(&deflater, &bufs, &nv[1], 1);
hd.length = nghttp2_bufs_len(&bufs) - hdpos - NGHTTP2_FRAME_HDLEN;
hd.type = NGHTTP2_CONTINUATION;
hd.flags = NGHTTP2_FLAG_END_HEADERS;
hd.stream_id = 1;
nghttp2_frame_pack_frame_hd(&buf->pos[hdpos], &hd);
rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_bufs_len(&bufs));
CU_ASSERT(rv == nghttp2_bufs_len(&bufs));
item = nghttp2_session_get_next_ob_item(session);
CU_ASSERT(NGHTTP2_RST_STREAM == OB_CTRL_TYPE(item));
/* Make sure no header decompression error occurred */
CU_ASSERT(NGHTTP2_GOAWAY_NONE == session->goaway_flags);
nghttp2_bufs_free(&bufs);
nghttp2_hd_deflate_free(&deflater);
nghttp2_session_del(session);
}

View File

@ -109,5 +109,6 @@ void test_nghttp2_session_stream_attach_data(void);
void test_nghttp2_session_stream_attach_data_subtree(void); void test_nghttp2_session_stream_attach_data_subtree(void);
void test_nghttp2_session_keep_closed_stream(void); void test_nghttp2_session_keep_closed_stream(void);
void test_nghttp2_session_graceful_shutdown(void); void test_nghttp2_session_graceful_shutdown(void);
void test_nghttp2_session_on_header_temporal_failure(void);
#endif /* NGHTTP2_SESSION_TEST_H */ #endif /* NGHTTP2_SESSION_TEST_H */