From 5778e6f526e2399ca7d01e6599381ab83d6b3021 Mon Sep 17 00:00:00 2001 From: Anders Bakken Date: Mon, 10 Aug 2015 17:26:36 -0700 Subject: http2: discard frames with no SessionHandle Return 0 instead of NGHTTP2_ERR_CALLBACK_FAILURE if we can't locate the SessionHandle. Apparently mod_h2 will sometimes send a frame for a stream_id we're finished with. Use nghttp2_session_get_stream_user_data and nghttp2_session_set_stream_user_data to identify SessionHandles instead of a hash. Closes #372 --- lib/http.c | 9 ++- lib/http.h | 1 - lib/http2.c | 237 ++++++++++++++++++++++++++---------------------------------- 3 files changed, 109 insertions(+), 138 deletions(-) diff --git a/lib/http.c b/lib/http.c index 8d5b9a408..9817d72af 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1443,7 +1443,10 @@ CURLcode Curl_http_done(struct connectdata *conn, CURLcode status, bool premature) { struct SessionHandle *data = conn->data; - struct HTTP *http =data->req.protop; + struct HTTP *http = data->req.protop; +#ifdef USE_NGHTTP2 + struct http_conn *httpc = &conn->proto.httpc; +#endif Curl_unencode_cleanup(conn); @@ -1482,6 +1485,10 @@ CURLcode Curl_http_done(struct connectdata *conn, free(http->push_headers); http->push_headers = NULL; } + if(http->stream_id) { + nghttp2_session_set_stream_user_data(httpc->h2, http->stream_id, 0); + http->stream_id = 0; + } #endif if(HTTPREQ_POST_FORM == data->set.httpreq) { diff --git a/lib/http.h b/lib/http.h index 63ea4ace4..fe4f39bc6 100644 --- a/lib/http.h +++ b/lib/http.h @@ -215,7 +215,6 @@ struct http_conn { nghttp2_session_mem_recv */ /* this is a hash of all individual streams (SessionHandle structs) */ - struct curl_hash streamsh; struct h2settings settings; #else int unused; /* prevent a compiler warning */ diff --git a/lib/http2.c b/lib/http2.c index eec0c9fca..0024add8a 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -88,7 +88,6 @@ static CURLcode http2_disconnect(struct connectdata *conn, nghttp2_session_del(c->h2); Curl_safefree(c->inbuf); - Curl_hash_destroy(&c->streamsh); if(http) { Curl_add_buffer_free(http->header_recvbuf); @@ -360,15 +359,8 @@ static int push_promise(struct SessionHandle *data, } httpc = &conn->proto.httpc; - /* put the newhandle in the hash with the stream id as key */ - if(!Curl_hash_add(&httpc->streamsh, - (size_t *)&frame->promised_stream_id, - sizeof(frame->promised_stream_id), newhandle)) { - failf(conn->data, "Couldn't add stream to hash!"); - rv = 1; - } - else - rv = 0; + nghttp2_session_set_stream_user_data(httpc->h2, + frame->promised_stream_id, newhandle); } else { DEBUGF(infof(data, "Got PUSH_PROMISE, ignore it!\n")); @@ -381,41 +373,44 @@ static int push_promise(struct SessionHandle *data, static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - struct http_conn *httpc = &conn->proto.httpc; + struct connectdata *conn = NULL; + struct http_conn *httpc = NULL; struct SessionHandle *data_s = NULL; struct HTTP *stream = NULL; + static int lastStream = -1; int rv; size_t left, ncopy; int32_t stream_id = frame->hd.stream_id; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, "on_frame_recv() header %x stream %x\n", - frame->hd.type, stream_id)); + (void)userp; - if(stream_id) { - /* get the stream from the hash based on Stream ID, stream ID zero is for - connection-oriented stuff */ - data_s = Curl_hash_pick(&httpc->streamsh, &stream_id, - sizeof(stream_id)); - if(!data_s) { - /* Receiving a Stream ID not in the hash should not happen, this is an - internal error more than anything else! */ - failf(conn->data, "Received frame on Stream ID: %x not in stream hash!", - stream_id); - return NGHTTP2_ERR_CALLBACK_FAILURE; - } - stream = data_s->req.protop; - if(!stream) { - failf(conn->data, "Internal NULL stream! 2\n"); - return NGHTTP2_ERR_CALLBACK_FAILURE; - } + if(!stream_id) { + /* stream ID zero is for connection-oriented stuff */ + return 0; } - else - /* we do nothing on stream zero */ + data_s = nghttp2_session_get_stream_user_data(session, + frame->hd.stream_id); + if(lastStream != frame->hd.stream_id) { + lastStream = frame->hd.stream_id; + } + if(!data_s) { + DEBUGF(infof(conn->data, + "No SessionHandle associated with stream: %x\n", + stream_id)); return 0; + } + + stream = data_s->req.protop; + if(!stream) + return NGHTTP2_ERR_CALLBACK_FAILURE; + + DEBUGF(infof(data_s, "on_frame_recv() header %x stream %x\n", + frame->hd.type, stream_id)); + conn = data_s->easy_conn; + assert(conn); + assert(conn->data == data_s); + httpc = &conn->proto.httpc; switch(frame->hd.type) { case NGHTTP2_DATA: /* If body started on this stream, then receiving DATA is illegal. */ @@ -513,12 +508,15 @@ static int on_invalid_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, int lib_error_code, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, - "on_invalid_frame_recv() was called, error=%d:%s\n", - lib_error_code, nghttp2_strerror(lib_error_code))); + struct SessionHandle *data_s = NULL; + (void)userp; + + data_s = nghttp2_session_get_stream_user_data(session, frame->hd.stream_id); + if(data_s) { + DEBUGF(infof(data_s, + "on_invalid_frame_recv() was called, error=%d:%s\n", + lib_error_code, nghttp2_strerror(lib_error_code))); + } return 0; } @@ -526,33 +524,26 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, int32_t stream_id, const uint8_t *data, size_t len, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; struct HTTP *stream; struct SessionHandle *data_s; size_t nread; (void)session; (void)flags; (void)data; - DEBUGF(infof(conn->data, "on_data_chunk_recv() " - "len = %u, stream %u\n", len, stream_id)); + (void)userp; DEBUGASSERT(stream_id); /* should never be a zero stream ID here */ /* get the stream from the hash based on Stream ID */ - data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, - sizeof(stream_id)); - if(!data_s) { + data_s = nghttp2_session_get_stream_user_data(session, stream_id); + if(!data_s) /* Receiving a Stream ID not in the hash should not happen, this is an internal error more than anything else! */ - failf(conn->data, "Received frame on Stream ID: %x not in stream hash!", - stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; - } + stream = data_s->req.protop; - if(!stream) { - failf(conn->data, "Internal NULL stream! 3\n"); + if(!stream) return NGHTTP2_ERR_CALLBACK_FAILURE; - } nread = MIN(stream->len, len); memcpy(&stream->mem[stream->memlen], data, nread); @@ -576,7 +567,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, DEBUGF(infof(data_s, "NGHTTP2_ERR_PAUSE - %zu bytes out of buffer" ", stream %u\n", len - nread, stream_id)); - conn->proto.httpc.pause_stream_id = stream_id; + data_s->easy_conn->proto.httpc.pause_stream_id = stream_id; return NGHTTP2_ERR_PAUSE; } return 0; @@ -586,73 +577,75 @@ static int before_frame_send(nghttp2_session *session, const nghttp2_frame *frame, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, "before_frame_send() was called\n")); + struct SessionHandle *data_s; + (void)userp; + + data_s = nghttp2_session_get_stream_user_data(session, frame->hd.stream_id); + if(data_s) { + DEBUGF(infof(data_s, "before_frame_send() was called\n")); + } + return 0; } static int on_frame_send(nghttp2_session *session, const nghttp2_frame *frame, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, "on_frame_send() was called, length = %zd\n", - frame->hd.length)); + struct SessionHandle *data_s; + (void)userp; + + data_s = nghttp2_session_get_stream_user_data(session, frame->hd.stream_id); + if(data_s) { + DEBUGF(infof(data_s, "on_frame_send() was called, length = %zd\n", + frame->hd.length)); + } return 0; } static int on_frame_not_send(nghttp2_session *session, const nghttp2_frame *frame, int lib_error_code, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, - "on_frame_not_send() was called, lib_error_code = %d\n", - lib_error_code)); + struct SessionHandle *data_s; + (void)userp; + + data_s = nghttp2_session_get_stream_user_data(session, frame->hd.stream_id); + if(data_s) { + DEBUGF(infof(data_s, + "on_frame_not_send() was called, lib_error_code = %d\n", + lib_error_code)); + } return 0; } static int on_stream_close(nghttp2_session *session, int32_t stream_id, uint32_t error_code, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s; struct HTTP *stream; (void)session; (void)stream_id; - DEBUGF(infof(conn->data, "on_stream_close(), error_code = %d, stream %u\n", - error_code, stream_id)); + (void)userp; if(stream_id) { /* get the stream from the hash based on Stream ID, stream ID zero is for connection-oriented stuff */ - data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, - sizeof(stream_id)); + data_s = nghttp2_session_get_stream_user_data(session, stream_id); if(!data_s) { /* We could get stream ID not in the hash. For example, if we - decided to reject stream (e.g., PUSH_PROMISE). We call infof - as a debugging purpose for now. */ - infof(conn->data, - "Received frame on Stream ID: %x not in stream hash!\n", - stream_id); + decided to reject stream (e.g., PUSH_PROMISE). */ return 0; } + DEBUGF(infof(data_s, "on_stream_close(), error_code = %d, stream %u\n", + error_code, stream_id)); stream = data_s->req.protop; - if(!stream) { - failf(conn->data, "Internal NULL stream! 4\n"); + if(!stream) return NGHTTP2_ERR_CALLBACK_FAILURE; - } stream->error_code = error_code; stream->closed = TRUE; /* remove the entry from the hash as the stream is now gone */ - Curl_hash_delete(&conn->proto.httpc.streamsh, - &stream_id, sizeof(stream_id)); - DEBUGF(infof(conn->data, "Removed stream %u hash!\n", stream_id)); + nghttp2_session_set_stream_user_data(session, stream_id, 0); + DEBUGF(infof(data_s, "Removed stream %u hash!\n", stream_id)); } return 0; } @@ -660,10 +653,13 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id, static int on_begin_headers(nghttp2_session *session, const nghttp2_frame *frame, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - (void)session; - (void)frame; - DEBUGF(infof(conn->data, "on_begin_headers() was called\n")); + struct SessionHandle *data_s = NULL; + (void)userp; + + data_s = nghttp2_session_get_stream_user_data(session, frame->hd.stream_id); + if(data_s) { + DEBUGF(infof(data_s, "on_begin_headers() was called\n")); + } return 0; } @@ -701,30 +697,25 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, uint8_t flags, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; struct HTTP *stream; struct SessionHandle *data_s; int32_t stream_id = frame->hd.stream_id; - (void)session; - (void)frame; (void)flags; + (void)userp; DEBUGASSERT(stream_id); /* should never be a zero stream ID here */ /* get the stream from the hash based on Stream ID */ - data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, - sizeof(stream_id)); - if(!data_s) { + data_s = nghttp2_session_get_stream_user_data(session, stream_id); + if(!data_s) /* Receiving a Stream ID not in the hash should not happen, this is an internal error more than anything else! */ - failf(conn->data, "Received frame on Stream ID: %x not in stream hash!", - stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; - } + stream = data_s->req.protop; if(!stream) { - failf(conn->data, "Internal NULL stream! 5\n"); + failf(data_s, "Internal NULL stream! 5\n"); return NGHTTP2_ERR_CALLBACK_FAILURE; } @@ -805,35 +796,27 @@ static ssize_t data_source_read_callback(nghttp2_session *session, nghttp2_data_source *source, void *userp) { - struct connectdata *conn = (struct connectdata *)userp; - struct http_conn *c = &conn->proto.httpc; struct SessionHandle *data_s; struct HTTP *stream = NULL; size_t nread; - (void)session; - (void)stream_id; (void)source; + (void)userp; if(stream_id) { /* get the stream from the hash based on Stream ID, stream ID zero is for connection-oriented stuff */ - data_s = Curl_hash_pick(&c->streamsh, &stream_id, sizeof(stream_id)); - if(!data_s) { + data_s = nghttp2_session_get_stream_user_data(session, stream_id); + if(!data_s) /* Receiving a Stream ID not in the hash should not happen, this is an internal error more than anything else! */ - failf(conn->data, "Asked for data to stream %u not in hash!", stream_id); return NGHTTP2_ERR_CALLBACK_FAILURE; - } + stream = data_s->req.protop; - if(!stream) { - failf(conn->data, "Internal NULL stream! 6\n"); + if(!stream) return NGHTTP2_ERR_CALLBACK_FAILURE; - } } - else { - failf(conn->data, "nghttp2 confusion"); + else return NGHTTP2_ERR_INVALID_ARGUMENT; - } nread = MIN(stream->upload_len, length); if(nread > 0) { @@ -865,11 +848,6 @@ static nghttp2_settings_entry settings[] = { #define H2_BUFSIZE 32768 -static void freestreamentry(void *freethis) -{ - (void)freethis; -} - /* * Initialize nghttp2 for a Curl connection */ @@ -929,8 +907,6 @@ CURLcode Curl_http2_init(struct connectdata *conn) return CURLE_OUT_OF_MEMORY; /* most likely at least */ } - rc = Curl_hash_init(&conn->proto.httpc.streamsh, 7, Curl_hash_str, - Curl_str_key_compare, freestreamentry); if(rc) { failf(conn->data, "Couldn't init stream hash!"); return CURLE_OUT_OF_MEMORY; /* most likely at least */ @@ -1377,11 +1353,11 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, data_prd.read_callback = data_source_read_callback; data_prd.source.ptr = NULL; stream_id = nghttp2_submit_request(h2, NULL, nva, nheader, - &data_prd, NULL); + &data_prd, conn->data); break; default: stream_id = nghttp2_submit_request(h2, NULL, nva, nheader, - NULL, NULL); + NULL, conn->data); } Curl_safefree(nva); @@ -1396,14 +1372,6 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex, stream_id, conn->data); stream->stream_id = stream_id; - /* put the SessionHandle in the hash with the stream_id as key */ - if(!Curl_hash_add(&httpc->streamsh, &stream->stream_id, sizeof(stream_id), - conn->data)) { - failf(conn->data, "Couldn't add stream to hash!"); - *err = CURLE_OUT_OF_MEMORY; - return -1; - } - rv = nghttp2_session_send(h2); if(rv != 0) { @@ -1506,12 +1474,9 @@ CURLcode Curl_http2_switched(struct connectdata *conn, return CURLE_HTTP2; } - /* put the SessionHandle in the hash with the stream->stream_id as key */ - if(!Curl_hash_add(&httpc->streamsh, &stream->stream_id, - sizeof(stream->stream_id), conn->data)) { - failf(conn->data, "Couldn't add stream to hash!"); - return CURLE_OUT_OF_MEMORY; - } + nghttp2_session_set_stream_user_data(httpc->h2, + stream->stream_id, + conn->data); } else { /* stream ID is unknown at this point */ -- cgit v1.2.3