From 01f15fd3d66655872e10c36dd6a631f491fbbed0 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sat, 10 Mar 2018 23:48:43 +0100
Subject: [PATCH 1/2] http2: mark the connection for close on GOAWAY
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
... don't consider it an error!
Assisted-by: Jay Satiro
Reported-by: Ćukasz Domeradzki
Fixes #2365
Closes #2375
Upstream-commit: 8b498a875c975294545581282289991bbcfeabf4
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/http.h | 5 ++---
lib/http2.c | 33 +++++++++++++++++++++------------
lib/multi.c | 9 +++------
3 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/lib/http.h b/lib/http.h
index a845f56..e8e41e3 100644
--- a/lib/http.h
+++ b/lib/http.h
@@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -174,8 +174,6 @@ struct HTTP {
size_t pauselen; /* the number of bytes left in data */
bool closed; /* TRUE on HTTP2 stream close */
bool close_handled; /* TRUE if stream closure is handled by libcurl */
- uint32_t error_code; /* HTTP/2 error code */
-
char *mem; /* points to a buffer in memory to store received data */
size_t len; /* size of the buffer 'mem' points to */
size_t memlen; /* size of data copied to mem */
@@ -228,6 +226,7 @@ struct http_conn {
/* list of settings that will be sent */
nghttp2_settings_entry local_settings[3];
size_t local_settings_num;
+ uint32_t error_code; /* HTTP/2 error code */
#else
int unused; /* prevent a compiler warning */
#endif
diff --git a/lib/http2.c b/lib/http2.c
index 0e55801..14ab0f7 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -205,7 +205,6 @@ void Curl_http2_setup_req(struct Curl_easy *data)
http->status_code = -1;
http->pausedata = NULL;
http->pauselen = 0;
- http->error_code = NGHTTP2_NO_ERROR;
http->closed = FALSE;
http->close_handled = FALSE;
http->mem = data->state.buffer;
@@ -218,6 +217,7 @@ void Curl_http2_setup_conn(struct connectdata *conn)
{
conn->proto.httpc.settings.max_concurrent_streams =
DEFAULT_MAX_CONCURRENT_STREAMS;
+ conn->proto.httpc.error_code = NGHTTP2_NO_ERROR;
}
/*
@@ -778,6 +778,7 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
(void)stream_id;
if(stream_id) {
+ struct http_conn *httpc;
/* get the stream from the hash based on Stream ID, stream ID zero is for
connection-oriented stuff */
data_s = nghttp2_session_get_stream_user_data(session, stream_id);
@@ -792,10 +793,11 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
if(!stream)
return NGHTTP2_ERR_CALLBACK_FAILURE;
- stream->error_code = error_code;
stream->closed = TRUE;
data_s->state.drain++;
- conn->proto.httpc.drain_total++;
+ httpc = &conn->proto.httpc;
+ httpc->drain_total++;
+ httpc->error_code = error_code;
/* remove the entry from the hash as the stream is now gone */
nghttp2_session_set_stream_user_data(session, stream_id, 0);
@@ -1223,13 +1225,14 @@ static int h2_session_send(struct Curl_easy *data,
* This function returns 0 if it succeeds, or -1 and error code will
* be assigned to *err.
*/
-static int h2_process_pending_input(struct Curl_easy *data,
+static int h2_process_pending_input(struct connectdata *conn,
struct http_conn *httpc,
CURLcode *err)
{
ssize_t nread;
char *inbuf;
ssize_t rv;
+ struct Curl_easy *data = conn->data;
nread = httpc->inbuflen - httpc->nread_inbuf;
inbuf = httpc->inbuf + httpc->nread_inbuf;
@@ -1267,7 +1270,13 @@ static int h2_process_pending_input(struct Curl_easy *data,
if(should_close_session(httpc)) {
DEBUGF(infof(data,
"h2_process_pending_input: nothing to do in this session\n"));
- *err = CURLE_HTTP2;
+ if(httpc->error_code)
+ *err = CURLE_HTTP2;
+ else {
+ /* not an error per se, but should still close the connection */
+ connclose(conn, "GOAWAY received");
+ *err = CURLE_OK;
+ }
return -1;
}
@@ -1298,7 +1307,7 @@ CURLcode Curl_http2_done_sending(struct connectdata *conn)
that it can signal EOF to nghttp2 */
(void)nghttp2_session_resume_data(h2, stream->stream_id);
- (void)h2_process_pending_input(conn->data, httpc, &result);
+ (void)h2_process_pending_input(conn, httpc, &result);
}
}
return result;
@@ -1322,7 +1331,7 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
data->state.drain = 0;
if(httpc->pause_stream_id == 0) {
- if(h2_process_pending_input(data, httpc, err) != 0) {
+ if(h2_process_pending_input(conn, httpc, err) != 0) {
return -1;
}
}
@@ -1331,10 +1340,10 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
/* Reset to FALSE to prevent infinite loop in readwrite_data function. */
stream->closed = FALSE;
- if(stream->error_code != NGHTTP2_NO_ERROR) {
+ if(httpc->error_code != NGHTTP2_NO_ERROR) {
failf(data, "HTTP/2 stream %u was not closed cleanly: %s (err %d)",
- stream->stream_id, Curl_http2_strerror(stream->error_code),
- stream->error_code);
+ stream->stream_id, Curl_http2_strerror(httpc->error_code),
+ httpc->error_code);
*err = CURLE_HTTP2_STREAM;
return -1;
}
@@ -1482,7 +1491,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
/* We have paused nghttp2, but we have no pause data (see
on_data_chunk_recv). */
httpc->pause_stream_id = 0;
- if(h2_process_pending_input(data, httpc, &result) != 0) {
+ if(h2_process_pending_input(conn, httpc, &result) != 0) {
*err = result;
return -1;
}
@@ -1512,7 +1521,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
frames, then we have to call it again with 0-length data.
Without this, on_stream_close callback will not be called,
and stream could be hanged. */
- if(h2_process_pending_input(data, httpc, &result) != 0) {
+ if(h2_process_pending_input(conn, httpc, &result) != 0) {
*err = result;
return -1;
}
diff --git a/lib/multi.c b/lib/multi.c
index d5bc532..7b9ba61 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -572,11 +572,8 @@ static CURLcode multi_done(struct connectdata **connp,
result = CURLE_ABORTED_BY_CALLBACK;
}
- if(conn->send_pipe.size + conn->recv_pipe.size != 0 &&
- !data->set.reuse_forbid &&
- !conn->bits.close) {
- /* Stop if pipeline is not empty and we do not have to close
- connection. */
+ if(conn->send_pipe.size || conn->recv_pipe.size) {
+ /* Stop if pipeline is not empty . */
data->easy_conn = NULL;
DEBUGF(infof(data, "Connection still in use, no more multi_done now!\n"));
return CURLE_OK;
--
2.14.4
From 84ddda3994c1f12d79946780dee9111b3cf1c308 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Thu, 19 Apr 2018 20:03:30 +0200
Subject: [PATCH 2/2] http2: handle GOAWAY properly
When receiving REFUSED_STREAM, mark the connection for close and retry
streams accordingly on another/fresh connection.
Reported-by: Terry Wu
Fixes #2416
Fixes #1618
Closes #2510
Upstream-commit: d122df5972fc01e39ae28e6bca705237d7e3318a
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
lib/http2.c | 17 ++++++++++++-----
lib/multi.c | 4 +++-
lib/transfer.c | 17 +++++++++++++++--
lib/urldata.h | 2 +-
4 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/lib/http2.c b/lib/http2.c
index b2c34e9..fba4d70 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1070,7 +1070,6 @@ void Curl_http2_done(struct connectdata *conn, bool premature)
struct http_conn *httpc = &conn->proto.httpc;
if(http->header_recvbuf) {
- DEBUGF(infof(data, "free header_recvbuf!!\n"));
Curl_add_buffer_free(http->header_recvbuf);
http->header_recvbuf = NULL; /* clear the pointer */
Curl_add_buffer_free(http->trailer_recvbuf);
@@ -1340,7 +1339,15 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
/* Reset to FALSE to prevent infinite loop in readwrite_data function. */
stream->closed = FALSE;
- if(httpc->error_code != NGHTTP2_NO_ERROR) {
+ if(httpc->error_code == NGHTTP2_REFUSED_STREAM) {
+ DEBUGF(infof(data, "REFUSED_STREAM (%d), try again on a new connection!\n",
+ stream->stream_id));
+ connclose(conn, "REFUSED_STREAM"); /* don't use this anymore */
+ data->state.refused_stream = TRUE;
+ *err = CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */
+ return -1;
+ }
+ else if(httpc->error_code != NGHTTP2_NO_ERROR) {
failf(data, "HTTP/2 stream %u was not closed cleanly: %s (err %d)",
stream->stream_id, Curl_http2_strerror(httpc->error_code),
httpc->error_code);
@@ -1568,9 +1575,9 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
}
if(nread == 0) {
- failf(data, "Unexpected EOF");
- *err = CURLE_RECV_ERROR;
- return -1;
+ DEBUGF(infof(data, "end of stream\n"));
+ *err = CURLE_OK;
+ return 0;
}
DEBUGF(infof(data, "nread=%zd\n", nread));
diff --git a/lib/multi.c b/lib/multi.c
index 98e5fca..d69e5f9 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -575,7 +575,9 @@ static CURLcode multi_done(struct connectdata **connp,
if(conn->send_pipe.size || conn->recv_pipe.size) {
/* Stop if pipeline is not empty . */
data->easy_conn = NULL;
- DEBUGF(infof(data, "Connection still in use, no more multi_done now!\n"));
+ DEBUGF(infof(data, "Connection still in use %d/%d, "
+ "no more multi_done now!\n",
+ conn->send_pipe.size, conn->recv_pipe.size));
return CURLE_OK;
}
diff --git a/lib/transfer.c b/lib/transfer.c
index fd9af31..5c29cc9 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1896,7 +1896,7 @@ CURLcode Curl_retry_request(struct connectdata *conn,
char **url)
{
struct Curl_easy *data = conn->data;
-
+ bool retry = FALSE;
*url = NULL;
/* if we're talking upload, we can't do the checks below, unless the protocol
@@ -1909,7 +1909,7 @@ CURLcode Curl_retry_request(struct connectdata *conn,
conn->bits.reuse &&
(!data->set.opt_no_body
|| (conn->handler->protocol & PROTO_FAMILY_HTTP)) &&
- (data->set.rtspreq != RTSPREQ_RECEIVE)) {
+ (data->set.rtspreq != RTSPREQ_RECEIVE))
/* We got no data, we attempted to re-use a connection. For HTTP this
can be a retry so we try again regardless if we expected a body.
For other protocols we only try again only if we expected a body.
@@ -1917,6 +1917,19 @@ CURLcode Curl_retry_request(struct connectdata *conn,
This might happen if the connection was left alive when we were
done using it before, but that was closed when we wanted to read from
it again. Bad luck. Retry the same request on a fresh connect! */
+ retry = TRUE;
+ else if(data->state.refused_stream &&
+ (data->req.bytecount + data->req.headerbytecount == 0) ) {
+ /* This was sent on a refused stream, safe to rerun. A refused stream
+ error can typically only happen on HTTP/2 level if the stream is safe
+ to issue again, but the nghttp2 API can deliver the message to other
+ streams as well, which is why this adds the check the data counters
+ too. */
+ infof(conn->data, "REFUSED_STREAM, retrying a fresh connect\n");
+ data->state.refused_stream = FALSE; /* clear again */
+ retry = TRUE;
+ }
+ if(retry) {
infof(conn->data, "Connection died, retrying a fresh connect\n");
*url = strdup(conn->data->change.url);
if(!*url)
diff --git a/lib/urldata.h b/lib/urldata.h
index 3d7b9e5..6a36ee9 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1391,7 +1391,7 @@ struct UrlState {
curl_off_t current_speed; /* the ProgressShow() function sets this,
bytes / second */
bool this_is_a_follow; /* this is a followed Location: request */
-
+ bool refused_stream; /* this was refused, try again */
char *first_host; /* host name of the first (not followed) request.
if set, this should be the host name that we will
sent authorization to, no else. Used to make Location:
--
2.14.4