aboutsummaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorTatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>2016-02-23 23:33:04 +0900
committerJay Satiro <raysatiro@yahoo.com>2016-04-11 21:43:28 -0400
commit86c633a893f1c0b40383dcc40a73d4bb76356969 (patch)
tree2abfc7f471c4344d777879f8f11eec7f0fde598c /lib
parentb5f82148f5cd3ec7f7a5a6ab760f7c9a15673958 (diff)
http2: Ensure that http2_handle_stream_close is called
This commit ensures that streams which was closed in on_stream_close callback gets passed to http2_handle_stream_close. Previously, this might not happen. To achieve this, we increment drain property to forcibly call recv function for that stream. To more accurately check that we have no pending event before shutting down HTTP/2 session, we sum up drain property into http_conn.drain_total. We only shutdown session if that value is 0. With this commit, when stream was closed before reading response header fields, error code CURLE_HTTP2_STREAM is returned even if HTTP/2 level error is NO_ERROR. This signals the upper layer that stream was closed by error just like TCP connection close in HTTP/1. Ref: https://github.com/curl/curl/issues/659 Ref: https://github.com/curl/curl/pull/663
Diffstat (limited to 'lib')
-rw-r--r--lib/http.h1
-rw-r--r--lib/http2.c57
2 files changed, 39 insertions, 19 deletions
diff --git a/lib/http.h b/lib/http.h
index 5000df448..8aae35206 100644
--- a/lib/http.h
+++ b/lib/http.h
@@ -214,6 +214,7 @@ struct http_conn {
them for both cases. */
int32_t pause_stream_id; /* stream ID which paused
nghttp2_session_mem_recv */
+ int drain_total; /* sum of all stream's UrlState.drain */
/* this is a hash of all individual streams (SessionHandle structs) */
struct h2settings settings;
diff --git a/lib/http2.c b/lib/http2.c
index 206d9592d..5e9adf240 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -507,6 +507,7 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame,
stream->memlen += ncopy;
data_s->state.drain++;
+ httpc->drain_total++;
{
/* get the pointer from userp again since it was re-assigned above */
struct connectdata *conn_s = (struct connectdata *)userp;
@@ -583,6 +584,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags,
stream->memlen += nread;
data_s->state.drain++;
+ conn->proto.httpc.drain_total++;
/* if we receive data for another handle, wake that up */
if(conn->data != data_s)
@@ -665,9 +667,9 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
{
struct SessionHandle *data_s;
struct HTTP *stream;
+ struct connectdata *conn = (struct connectdata *)userp;
(void)session;
(void)stream_id;
- (void)userp;
if(stream_id) {
/* get the stream from the hash based on Stream ID, stream ID zero is for
@@ -686,6 +688,8 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
stream->error_code = error_code;
stream->closed = TRUE;
+ data_s->state.drain++;
+ conn->proto.httpc.drain_total++;
/* remove the entry from the hash as the stream is now gone */
nghttp2_session_set_stream_user_data(session, stream_id, 0);
@@ -845,6 +849,7 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
/* the space character after the status code is mandatory */
Curl_add_buffer(stream->header_recvbuf, " \r\n", 3);
data_s->state.drain++;
+ conn->proto.httpc.drain_total++;
/* if we receive data for another handle, wake that up */
if(conn->data != data_s)
Curl_expire(data_s, 1);
@@ -862,6 +867,7 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame,
Curl_add_buffer(stream->header_recvbuf, value, valuelen);
Curl_add_buffer(stream->header_recvbuf, "\r\n", 2);
data_s->state.drain++;
+ conn->proto.httpc.drain_total++;
/* if we receive data for another handle, wake that up */
if(conn->data != data_s)
Curl_expire(data_s, 1);
@@ -1053,6 +1059,14 @@ CURLcode Curl_http2_request_upgrade(Curl_send_buffer *req,
return result;
}
+/*
+ * Returns nonzero if current HTTP/2 session should be closed.
+ */
+static int should_close_session(struct http_conn *httpc) {
+ return httpc->drain_total == 0 && !nghttp2_session_want_read(httpc->h2) &&
+ !nghttp2_session_want_write(httpc->h2);
+}
+
static int h2_session_send(struct SessionHandle *data,
nghttp2_session *h2);
@@ -1102,9 +1116,7 @@ static int h2_process_pending_input(struct SessionHandle *data,
return -1;
}
- if(httpc->pause_stream_id == 0 &&
- !nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(data,
"h2_process_pending_input: nothing to do in this session\n"));
*err = CURLE_HTTP2;
@@ -1125,12 +1137,17 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
httpc->pause_stream_id = 0;
}
+ httpc->drain_total -= data->state.drain;
+ data->state.drain = 0;
+
if(httpc->pause_stream_id == 0) {
if(h2_process_pending_input(data, httpc, err) != 0) {
return -1;
}
}
+ DEBUGASSERT(data->state.drain == 0);
+
/* Reset to FALSE to prevent infinite loop in readwrite_data
function. */
stream->closed = FALSE;
@@ -1141,6 +1158,14 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
return -1;
}
+ if(!stream->bodystarted) {
+ failf(data, "HTTP/2 stream %u was closed cleanly, but before getting "
+ " all response header fields, teated as error",
+ stream->stream_id);
+ *err = CURLE_HTTP2_STREAM;
+ return -1;
+ }
+
if(stream->trailer_recvbuf && stream->trailer_recvbuf->buffer) {
trailer_pos = stream->trailer_recvbuf->buffer;
trailer_end = trailer_pos + stream->trailer_recvbuf->size_used;
@@ -1228,9 +1253,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
(void)sockindex; /* we always do HTTP2 on sockindex 0 */
- if(httpc->pause_stream_id == 0 &&
- !nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(data,
"http2_recv: nothing to do in this session\n"));
*err = CURLE_HTTP2;
@@ -1399,9 +1422,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
return 0;
}
- if(httpc->pause_stream_id == 0 &&
- !nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(data, "http2_recv: nothing to do in this session\n"));
*err = CURLE_HTTP2;
return -1;
@@ -1419,8 +1440,10 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
DEBUGF(infof(data, "Data returned for PAUSED stream %u\n",
stream->stream_id));
}
- else
+ else if(!stream->closed) {
+ httpc->drain_total -= data->state.drain;
data->state.drain = 0; /* this stream is hereby drained */
+ }
return retlen;
}
@@ -1484,9 +1507,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
stream->upload_mem = NULL;
stream->upload_len = 0;
- if(httpc->pause_stream_id == 0 &&
- !nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(conn->data, "http2_send: nothing to do in this session\n"));
*err = CURLE_HTTP2;
return -1;
@@ -1658,9 +1679,7 @@ static ssize_t http2_send(struct connectdata *conn, int sockindex,
return -1;
}
- if(httpc->pause_stream_id == 0 &&
- !nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(conn->data, "http2_send: nothing to do in this session\n"));
*err = CURLE_HTTP2;
return -1;
@@ -1719,6 +1738,7 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
httpc->nread_inbuf = 0;
httpc->pause_stream_id = 0;
+ httpc->drain_total = 0;
conn->bits.multiplex = TRUE; /* at least potentially multiplexed */
conn->httpversion = 20;
@@ -1826,8 +1846,7 @@ CURLcode Curl_http2_switched(struct connectdata *conn,
return CURLE_HTTP2;
}
- if(!nghttp2_session_want_read(httpc->h2) &&
- !nghttp2_session_want_write(httpc->h2)) {
+ if(should_close_session(httpc)) {
DEBUGF(infof(data,
"nghttp2_session_send(): nothing to do in this session\n"));
return CURLE_HTTP2;