From 81cd24adb8bf09ef26ab8cf56955136711dbd7df Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 23 Jul 2014 09:23:56 +0200 Subject: [PATCH] http2: more and better error checking 1 - fixes the warnings when built without http2 support 2 - adds CURLE_HTTP2, a new error code for errors detected by nghttp2 basically when they are about http2 specific things. --- docs/libcurl/libcurl-errors.3 | 3 +++ docs/libcurl/symbols-in-versions | 1 + include/curl/curl.h | 4 +++- lib/http.c | 14 +++++++++----- lib/http2.c | 24 ++++++++++++++++-------- lib/http2.h | 4 ++-- lib/strerror.c | 4 +++- 7 files changed, 37 insertions(+), 17 deletions(-) diff --git a/docs/libcurl/libcurl-errors.3 b/docs/libcurl/libcurl-errors.3 index 9f295d470..46aa3fef3 100644 --- a/docs/libcurl/libcurl-errors.3 +++ b/docs/libcurl/libcurl-errors.3 @@ -83,6 +83,9 @@ FTP servers return a 227-line as a response to a PASV command. If libcurl fails to parse that line, this return code is passed back. .IP "CURLE_FTP_CANT_GET_HOST (15)" An internal failure to lookup the host used for the new connection. +.IP "CURLE_HTTP2 (16)" +A problem was detected in the HTTP2 framing layer. This is somewhat generic +and can be one out of several problems, see the error buffer for details. .IP "CURLE_FTP_COULDNT_SET_TYPE (17)" Received an error when trying to set the transfer mode to binary or ASCII. .IP "CURLE_PARTIAL_FILE (18)" diff --git a/docs/libcurl/symbols-in-versions b/docs/libcurl/symbols-in-versions index 0c9b2b039..6d7334867 100644 --- a/docs/libcurl/symbols-in-versions +++ b/docs/libcurl/symbols-in-versions @@ -79,6 +79,7 @@ CURLE_HTTP_PORT_FAILED 7.3 7.12.0 CURLE_HTTP_POST_ERROR 7.1 CURLE_HTTP_RANGE_ERROR 7.1 7.17.0 CURLE_HTTP_RETURNED_ERROR 7.10.3 +CURLE_HTTP2 7.38.0 CURLE_INTERFACE_FAILED 7.12.0 CURLE_LDAP_CANNOT_BIND 7.1 CURLE_LDAP_INVALID_URL 7.10.8 diff --git a/include/curl/curl.h b/include/curl/curl.h index 526c7213e..5528d87bf 100644 --- a/include/curl/curl.h +++ b/include/curl/curl.h @@ -423,7 +423,9 @@ typedef enum { CURLE_FTP_WEIRD_PASV_REPLY, /* 13 */ CURLE_FTP_WEIRD_227_FORMAT, /* 14 */ CURLE_FTP_CANT_GET_HOST, /* 15 */ - CURLE_OBSOLETE16, /* 16 - NOT USED */ + CURLE_HTTP2, /* 16 - A problem in the http2 framing layer. + [was obsoleted in August 2007 for 7.17.0, + reused in July 2014 for 7.38.0] */ CURLE_FTP_COULDNT_SET_TYPE, /* 17 */ CURLE_PARTIAL_FILE, /* 18 */ CURLE_FTP_COULDNT_RETR_FILE, /* 19 */ diff --git a/lib/http.c b/lib/http.c index 5e8499aa2..f50ea5262 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1760,8 +1760,9 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) if(result) return result; - /* TODO: add error checking here */ - Curl_http2_switched(conn); + result = Curl_http2_switched(conn); + if(result) + return result; break; case NPN_HTTP1_1: /* continue with HTTP/1.1 when explicitly requested */ @@ -1773,7 +1774,9 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) } else { /* prepare for a http2 request */ - Curl_http2_setup(conn); + result = Curl_http2_setup(conn); + if(result) + return result; } http = data->req.protop; @@ -3007,8 +3010,9 @@ CURLcode Curl_http_readwrite_headers(struct SessionHandle *data, k->upgr101 = UPGR101_RECEIVED; /* switch to http2 now */ - /* TODO: add error checking */ - Curl_http2_switched(conn); + result = Curl_http2_switched(conn); + if(result) + return result; } break; default: diff --git a/lib/http2.c b/lib/http2.c index c850fdb8f..fcc583dc9 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -810,12 +810,12 @@ CURLcode Curl_http2_setup(struct connectdata *conn) return Curl_add_buffer(httpc->header_recvbuf, "HTTP/2.0 200\r\n", 14); } -int Curl_http2_switched(struct connectdata *conn) +CURLcode Curl_http2_switched(struct connectdata *conn) { - /* TODO: May get CURLE_AGAIN */ CURLcode rc; struct http_conn *httpc = &conn->proto.httpc; int rv; + struct SessionHandle *data = conn->data; httpc->recv_underlying = (recving)conn->recv[FIRSTSOCKET]; httpc->send_underlying = (sending)conn->send[FIRSTSOCKET]; @@ -827,7 +827,15 @@ int Curl_http2_switched(struct connectdata *conn) NGHTTP2_CLIENT_CONNECTION_PREFACE, NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN, &rc); - assert(rv == 24); + if(rc) + /* TODO: This may get CURLE_AGAIN */ + return rc; + + if(rv != 24) { + failf(data, "Only sent partial HTTP2 packet"); + return CURLE_SEND_ERROR; + } + if(conn->data->req.upgr101 == UPGR101_RECEIVED) { /* stream 1 is opened implicitly on upgrade */ httpc->stream_id = 1; @@ -835,9 +843,9 @@ int Curl_http2_switched(struct connectdata *conn) rv = nghttp2_session_upgrade(httpc->h2, httpc->binsettings, httpc->binlen, NULL); if(rv != 0) { - failf(conn->data, "nghttp2_session_upgrade() failed: %s(%d)", + failf(data, "nghttp2_session_upgrade() failed: %s(%d)", nghttp2_strerror(rv), rv); - return -1; + return CURLE_HTTP2; } } else { @@ -845,12 +853,12 @@ int Curl_http2_switched(struct connectdata *conn) httpc->stream_id = -1; rv = nghttp2_submit_settings(httpc->h2, NGHTTP2_FLAG_NONE, NULL, 0); if(rv != 0) { - failf(conn->data, "nghttp2_submit_settings() failed: %s(%d)", + failf(data, "nghttp2_submit_settings() failed: %s(%d)", nghttp2_strerror(rv), rv); - return -1; + return CURLE_HTTP2; } } - return 0; + return CURLE_OK; } #endif diff --git a/lib/http2.h b/lib/http2.h index 5c0ce8e80..66aa6fd5c 100644 --- a/lib/http2.h +++ b/lib/http2.h @@ -37,13 +37,13 @@ CURLcode Curl_http2_send_request(struct connectdata *conn); CURLcode Curl_http2_request_upgrade(Curl_send_buffer *req, struct connectdata *conn); CURLcode Curl_http2_setup(struct connectdata *conn); -int Curl_http2_switched(struct connectdata *conn); +CURLcode Curl_http2_switched(struct connectdata *conn); #else /* USE_NGHTTP2 */ #define Curl_http2_init(x) CURLE_UNSUPPORTED_PROTOCOL #define Curl_http2_send_request(x) CURLE_UNSUPPORTED_PROTOCOL #define Curl_http2_request_upgrade(x,y) CURLE_UNSUPPORTED_PROTOCOL #define Curl_http2_setup(x) CURLE_UNSUPPORTED_PROTOCOL -#define Curl_http2_switched(x) (-1) +#define Curl_http2_switched(x) CURLE_UNSUPPORTED_PROTOCOL #endif #endif /* HEADER_CURL_HTTP2_H */ diff --git a/lib/strerror.c b/lib/strerror.c index aec6d38f3..66033f219 100644 --- a/lib/strerror.c +++ b/lib/strerror.c @@ -105,6 +105,9 @@ curl_easy_strerror(CURLcode error) case CURLE_FTP_CANT_GET_HOST: return "FTP: can't figure out the host in the PASV response"; + case CURLE_HTTP2: + return "Error in the HTTP2 framing layer"; + case CURLE_FTP_COULDNT_SET_TYPE: return "FTP: couldn't set file type"; @@ -296,7 +299,6 @@ curl_easy_strerror(CURLcode error) return "The max connection limit is reached"; /* error codes not used by current libcurl */ - case CURLE_OBSOLETE16: case CURLE_OBSOLETE20: case CURLE_OBSOLETE24: case CURLE_OBSOLETE29: