moparisthebest
/
curl
mirror of https://github.com/moparisthebest/curl
1
0
Fork 0
Code Issues Releases Wiki Activity

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
This commit is contained in:
Anders Bakken 2015-08-10 17:26:36 -07:00 committed by Daniel Stenberg
parent c8a656d3c7
commit 5778e6f526
3 changed files with 111 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/http.c
View File

@ -1444,6 +1444,9 @@ CURLcode Curl_http_done(struct connectdata *conn,
{ {
struct SessionHandle *data = conn->data; 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); Curl_unencode_cleanup(conn);
@ -1482,6 +1485,10 @@ CURLcode Curl_http_done(struct connectdata *conn,
free(http->push_headers); free(http->push_headers);
http->push_headers = NULL; 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 #endif
if(HTTPREQ_POST_FORM == data->set.httpreq) { if(HTTPREQ_POST_FORM == data->set.httpreq) {

1
lib/http.h
View File

@ -215,7 +215,6 @@ struct http_conn {
nghttp2_session_mem_recv */ nghttp2_session_mem_recv */
/* this is a hash of all individual streams (SessionHandle structs) */ /* this is a hash of all individual streams (SessionHandle structs) */
struct curl_hash streamsh;
struct h2settings settings; struct h2settings settings;
#else #else
int unused; /* prevent a compiler warning */ int unused; /* prevent a compiler warning */

231
lib/http2.c
View File

@ -88,7 +88,6 @@ static CURLcode http2_disconnect(struct connectdata *conn,
nghttp2_session_del(c->h2); nghttp2_session_del(c->h2);
Curl_safefree(c->inbuf); Curl_safefree(c->inbuf);
Curl_hash_destroy(&c->streamsh);
if(http) { if(http) {
Curl_add_buffer_free(http->header_recvbuf); Curl_add_buffer_free(http->header_recvbuf);
@ -360,15 +359,8 @@ static int push_promise(struct SessionHandle *data,
} }
httpc = &conn->proto.httpc; httpc = &conn->proto.httpc;
/* put the newhandle in the hash with the stream id as key */ nghttp2_session_set_stream_user_data(httpc->h2,
if(!Curl_hash_add(&httpc->streamsh, frame->promised_stream_id, newhandle);
(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;
} }
else { else {
DEBUGF(infof(data, "Got PUSH_PROMISE, ignore it!\n")); 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, static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
void *userp) void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct connectdata *conn = NULL;
struct http_conn *httpc = &conn->proto.httpc; struct http_conn *httpc = NULL;
struct SessionHandle *data_s = NULL; struct SessionHandle *data_s = NULL;
struct HTTP *stream = NULL; struct HTTP *stream = NULL;
static int lastStream = -1;
int rv; int rv;
size_t left, ncopy; size_t left, ncopy;
int32_t stream_id = frame->hd.stream_id; int32_t stream_id = frame->hd.stream_id;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, "on_frame_recv() header %x stream %x\n", if(!stream_id) {
/* stream ID zero is for connection-oriented stuff */
return 0;
}
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)); frame->hd.type, stream_id));
if(stream_id) { conn = data_s->easy_conn;
/* get the stream from the hash based on Stream ID, stream ID zero is for assert(conn);
connection-oriented stuff */ assert(conn->data == data_s);
data_s = Curl_hash_pick(&httpc->streamsh, &stream_id, httpc = &conn->proto.httpc;
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;
}
}
else
/* we do nothing on stream zero */
return 0;
switch(frame->hd.type) { switch(frame->hd.type) {
case NGHTTP2_DATA: case NGHTTP2_DATA:
/* If body started on this stream, then receiving DATA is illegal. */ /* 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, const nghttp2_frame *frame,
int lib_error_code, void *userp) int lib_error_code, void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s = NULL;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, 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", "on_invalid_frame_recv() was called, error=%d:%s\n",
lib_error_code, nghttp2_strerror(lib_error_code))); lib_error_code, nghttp2_strerror(lib_error_code)));
}
return 0; return 0;
} }
@ -526,33 +524,26 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
int32_t stream_id, int32_t stream_id,
const uint8_t *data, size_t len, void *userp) const uint8_t *data, size_t len, void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp;
struct HTTP *stream; struct HTTP *stream;
struct SessionHandle *data_s; struct SessionHandle *data_s;
size_t nread; size_t nread;
(void)session; (void)session;
(void)flags; (void)flags;
(void)data; (void)data;
DEBUGF(infof(conn->data, "on_data_chunk_recv() " (void)userp;
"len = %u, stream %u\n", len, stream_id));
DEBUGASSERT(stream_id); /* should never be a zero stream ID here */ DEBUGASSERT(stream_id); /* should never be a zero stream ID here */
/* get the stream from the hash based on Stream ID */ /* get the stream from the hash based on Stream ID */
data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, data_s = nghttp2_session_get_stream_user_data(session, stream_id);
sizeof(stream_id)); if(!data_s)
if(!data_s) {
/* Receiving a Stream ID not in the hash should not happen, this is an /* Receiving a Stream ID not in the hash should not happen, this is an
internal error more than anything else! */ 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; return NGHTTP2_ERR_CALLBACK_FAILURE;
}
stream = data_s->req.protop; stream = data_s->req.protop;
if(!stream) { if(!stream)
failf(conn->data, "Internal NULL stream! 3\n");
return NGHTTP2_ERR_CALLBACK_FAILURE; return NGHTTP2_ERR_CALLBACK_FAILURE;
}
nread = MIN(stream->len, len); nread = MIN(stream->len, len);
memcpy(&stream->mem[stream->memlen], data, nread); 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" DEBUGF(infof(data_s, "NGHTTP2_ERR_PAUSE - %zu bytes out of buffer"
", stream %u\n", ", stream %u\n",
len - nread, stream_id)); 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 NGHTTP2_ERR_PAUSE;
} }
return 0; return 0;
@ -586,73 +577,75 @@ static int before_frame_send(nghttp2_session *session,
const nghttp2_frame *frame, const nghttp2_frame *frame,
void *userp) void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, "before_frame_send() was called\n")); 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; return 0;
} }
static int on_frame_send(nghttp2_session *session, static int on_frame_send(nghttp2_session *session,
const nghttp2_frame *frame, const nghttp2_frame *frame,
void *userp) void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, "on_frame_send() was called, length = %zd\n", 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)); frame->hd.length));
}
return 0; return 0;
} }
static int on_frame_not_send(nghttp2_session *session, static int on_frame_not_send(nghttp2_session *session,
const nghttp2_frame *frame, const nghttp2_frame *frame,
int lib_error_code, void *userp) int lib_error_code, void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, 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", "on_frame_not_send() was called, lib_error_code = %d\n",
lib_error_code)); lib_error_code));
}
return 0; return 0;
} }
static int on_stream_close(nghttp2_session *session, int32_t stream_id, static int on_stream_close(nghttp2_session *session, int32_t stream_id,
uint32_t error_code, void *userp) uint32_t error_code, void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp;
struct SessionHandle *data_s; struct SessionHandle *data_s;
struct HTTP *stream; struct HTTP *stream;
(void)session; (void)session;
(void)stream_id; (void)stream_id;
DEBUGF(infof(conn->data, "on_stream_close(), error_code = %d, stream %u\n", (void)userp;
error_code, stream_id));
if(stream_id) { if(stream_id) {
/* get the stream from the hash based on Stream ID, stream ID zero is for /* get the stream from the hash based on Stream ID, stream ID zero is for
connection-oriented stuff */ connection-oriented stuff */
data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, data_s = nghttp2_session_get_stream_user_data(session, stream_id);
sizeof(stream_id));
if(!data_s) { if(!data_s) {
/* We could get stream ID not in the hash. For example, if we /* We could get stream ID not in the hash. For example, if we
decided to reject stream (e.g., PUSH_PROMISE). We call infof decided to reject stream (e.g., PUSH_PROMISE). */
as a debugging purpose for now. */
infof(conn->data,
"Received frame on Stream ID: %x not in stream hash!\n",
stream_id);
return 0; return 0;
} }
DEBUGF(infof(data_s, "on_stream_close(), error_code = %d, stream %u\n",
error_code, stream_id));
stream = data_s->req.protop; stream = data_s->req.protop;
if(!stream) { if(!stream)
failf(conn->data, "Internal NULL stream! 4\n");
return NGHTTP2_ERR_CALLBACK_FAILURE; return NGHTTP2_ERR_CALLBACK_FAILURE;
}
stream->error_code = error_code; stream->error_code = error_code;
stream->closed = TRUE; stream->closed = TRUE;
/* remove the entry from the hash as the stream is now gone */ /* remove the entry from the hash as the stream is now gone */
Curl_hash_delete(&conn->proto.httpc.streamsh, nghttp2_session_set_stream_user_data(session, stream_id, 0);
&stream_id, sizeof(stream_id)); DEBUGF(infof(data_s, "Removed stream %u hash!\n", stream_id));
DEBUGF(infof(conn->data, "Removed stream %u hash!\n", stream_id));
} }
return 0; 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, static int on_begin_headers(nghttp2_session *session,
const nghttp2_frame *frame, void *userp) const nghttp2_frame *frame, void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp; struct SessionHandle *data_s = NULL;
(void)session; (void)userp;
(void)frame;
DEBUGF(infof(conn->data, "on_begin_headers() was called\n")); 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; return 0;
} }
@ -701,30 +697,25 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
uint8_t flags, uint8_t flags,
void *userp) void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp;
struct HTTP *stream; struct HTTP *stream;
struct SessionHandle *data_s; struct SessionHandle *data_s;
int32_t stream_id = frame->hd.stream_id; int32_t stream_id = frame->hd.stream_id;
(void)session;
(void)frame;
(void)flags; (void)flags;
(void)userp;
DEBUGASSERT(stream_id); /* should never be a zero stream ID here */ DEBUGASSERT(stream_id); /* should never be a zero stream ID here */
/* get the stream from the hash based on Stream ID */ /* get the stream from the hash based on Stream ID */
data_s = Curl_hash_pick(&conn->proto.httpc.streamsh, &stream_id, data_s = nghttp2_session_get_stream_user_data(session, stream_id);
sizeof(stream_id)); if(!data_s)
if(!data_s) {
/* Receiving a Stream ID not in the hash should not happen, this is an /* Receiving a Stream ID not in the hash should not happen, this is an
internal error more than anything else! */ 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; return NGHTTP2_ERR_CALLBACK_FAILURE;
}
stream = data_s->req.protop; stream = data_s->req.protop;
if(!stream) { if(!stream) {
failf(conn->data, "Internal NULL stream! 5\n"); failf(data_s, "Internal NULL stream! 5\n");
return NGHTTP2_ERR_CALLBACK_FAILURE; return NGHTTP2_ERR_CALLBACK_FAILURE;
} }
@ -805,35 +796,27 @@ static ssize_t data_source_read_callback(nghttp2_session *session,
nghttp2_data_source *source, nghttp2_data_source *source,
void *userp) void *userp)
{ {
struct connectdata *conn = (struct connectdata *)userp;
struct http_conn *c = &conn->proto.httpc;
struct SessionHandle *data_s; struct SessionHandle *data_s;
struct HTTP *stream = NULL; struct HTTP *stream = NULL;
size_t nread; size_t nread;
(void)session;
(void)stream_id;
(void)source; (void)source;
(void)userp;
if(stream_id) { if(stream_id) {
/* get the stream from the hash based on Stream ID, stream ID zero is for /* get the stream from the hash based on Stream ID, stream ID zero is for
connection-oriented stuff */ connection-oriented stuff */
data_s = Curl_hash_pick(&c->streamsh, &stream_id, sizeof(stream_id)); data_s = nghttp2_session_get_stream_user_data(session, stream_id);
if(!data_s) { if(!data_s)
/* Receiving a Stream ID not in the hash should not happen, this is an /* Receiving a Stream ID not in the hash should not happen, this is an
internal error more than anything else! */ 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; return NGHTTP2_ERR_CALLBACK_FAILURE;
}
stream = data_s->req.protop; stream = data_s->req.protop;
if(!stream) { if(!stream)
failf(conn->data, "Internal NULL stream! 6\n");
return NGHTTP2_ERR_CALLBACK_FAILURE; return NGHTTP2_ERR_CALLBACK_FAILURE;
} }
} else
else {
failf(conn->data, "nghttp2 confusion");
return NGHTTP2_ERR_INVALID_ARGUMENT; return NGHTTP2_ERR_INVALID_ARGUMENT;
}
nread = MIN(stream->upload_len, length); nread = MIN(stream->upload_len, length);
if(nread > 0) { if(nread > 0) {
@ -865,11 +848,6 @@ static nghttp2_settings_entry settings[] = {
#define H2_BUFSIZE 32768 #define H2_BUFSIZE 32768
static void freestreamentry(void *freethis)
{
(void)freethis;
}
/* /*
* Initialize nghttp2 for a Curl connection * 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 */ 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) { if(rc) {
failf(conn->data, "Couldn't init stream hash!"); failf(conn->data, "Couldn't init stream hash!");
return CURLE_OUT_OF_MEMORY; /* most likely at least */ 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.read_callback = data_source_read_callback;
data_prd.source.ptr = NULL; data_prd.source.ptr = NULL;
stream_id = nghttp2_submit_request(h2, NULL, nva, nheader, stream_id = nghttp2_submit_request(h2, NULL, nva, nheader,
&data_prd, NULL); &data_prd, conn->data);
break; break;
default: default:
stream_id = nghttp2_submit_request(h2, NULL, nva, nheader, stream_id = nghttp2_submit_request(h2, NULL, nva, nheader,
NULL, NULL); NULL, conn->data);
} }
Curl_safefree(nva); Curl_safefree(nva);
@ -1396,14 +1372,6 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
stream_id, conn->data); stream_id, conn->data);
stream->stream_id = stream_id; 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); rv = nghttp2_session_send(h2);
if(rv != 0) { if(rv != 0) {
@ -1506,12 +1474,9 @@ CURLcode Curl_http2_switched(struct connectdata *conn,
return CURLE_HTTP2; return CURLE_HTTP2;
} }
/* put the SessionHandle in the hash with the stream->stream_id as key */ nghttp2_session_set_stream_user_data(httpc->h2,
if(!Curl_hash_add(&httpc->streamsh, &stream->stream_id, stream->stream_id,
sizeof(stream->stream_id), conn->data)) { conn->data);
failf(conn->data, "Couldn't add stream to hash!");
return CURLE_OUT_OF_MEMORY;
}
} }
else { else {
/* stream ID is unknown at this point */ /* stream ID is unknown at this point */