Skip to content

Commit

Permalink
http: move HTTP/2 cleanup code off http_disconnect()
Browse files Browse the repository at this point in the history
Otherwise it would never be called for an HTTP/2 connection, which has
its own disconnect handler.

I spotted this while debugging <https://bugzilla.redhat.com/1248389>
where the http_disconnect() handler was called on an FTP session handle
causing 'dnf' to crash.  conn->data->req.protop of type (struct FTP *)
was reinterpreted as type (struct HTTP *) which resulted in SIGSEGV in
Curl_add_buffer_free() after printing the "Connection cache is full,
closing the oldest one." message.

A previously working version of libcurl started to crash after it was
recompiled with the HTTP/2 support despite the HTTP/2 protocol was not
actually used.  This commit makes it work again although I suspect the
root cause (reinterpreting session handle data of incompatible protocol)
still has to be fixed.  Otherwise the same will happen when mixing FTP
and HTTP/2 connections and exceeding the connection cache limit.

Reported-by: Tomas Tomecek
Bug: https://bugzilla.redhat.com/1248389
  • Loading branch information
kdudka committed Jul 30, 2015
1 parent ecf7618 commit f7dcc7c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 23 deletions.
25 changes: 2 additions & 23 deletions lib/http.c
Expand Up @@ -86,7 +86,6 @@
* Forward declarations.
*/

static CURLcode http_disconnect(struct connectdata *conn, bool dead);
static int http_getsock_do(struct connectdata *conn,
curl_socket_t *socks,
int numsocks);
Expand Down Expand Up @@ -117,7 +116,7 @@ const struct Curl_handler Curl_handler_http = {
http_getsock_do, /* doing_getsock */
ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
http_disconnect, /* disconnect */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
PORT_HTTP, /* defport */
CURLPROTO_HTTP, /* protocol */
Expand All @@ -141,7 +140,7 @@ const struct Curl_handler Curl_handler_https = {
http_getsock_do, /* doing_getsock */
ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
http_disconnect, /* disconnect */
ZERO_NULL, /* disconnect */
ZERO_NULL, /* readwrite */
PORT_HTTPS, /* defport */
CURLPROTO_HTTPS, /* protocol */
Expand Down Expand Up @@ -169,26 +168,6 @@ CURLcode Curl_http_setup_conn(struct connectdata *conn)
return CURLE_OK;
}

static CURLcode http_disconnect(struct connectdata *conn, bool dead_connection)
{
#ifdef USE_NGHTTP2
struct HTTP *http = conn->data->req.protop;
if(http) {
Curl_add_buffer_free(http->header_recvbuf);
http->header_recvbuf = NULL; /* clear the pointer */
for(; http->push_headers_used > 0; --http->push_headers_used) {
free(http->push_headers[http->push_headers_used - 1]);
}
free(http->push_headers);
http->push_headers = NULL;
}
#else
(void)conn;
#endif
(void)dead_connection;
return CURLE_OK;
}

/*
* checkheaders() checks the linked list of custom HTTP headers for a
* particular header (prefix).
Expand Down
11 changes: 11 additions & 0 deletions lib/http2.c
Expand Up @@ -80,6 +80,7 @@ static int http2_getsock(struct connectdata *conn,
static CURLcode http2_disconnect(struct connectdata *conn,
bool dead_connection)
{
struct HTTP *http = conn->data->req.protop;
struct http_conn *c = &conn->proto.httpc;
(void)dead_connection;

Expand All @@ -89,6 +90,16 @@ static CURLcode http2_disconnect(struct connectdata *conn,
Curl_safefree(c->inbuf);
Curl_hash_destroy(&c->streamsh);

if(http) {
Curl_add_buffer_free(http->header_recvbuf);
http->header_recvbuf = NULL; /* clear the pointer */
for(; http->push_headers_used > 0; --http->push_headers_used) {
free(http->push_headers[http->push_headers_used - 1]);
}
free(http->push_headers);
http->push_headers = NULL;
}

DEBUGF(infof(conn->data, "HTTP/2 DISCONNECT done\n"));

return CURLE_OK;
Expand Down

0 comments on commit f7dcc7c

Please sign in to comment.