http: move HTTP/2 cleanup code off http_disconnect()

kdudka · kdudka · commit f7dcc7c11817 · 2015-07-30T15:16:43.000+02:00
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
diff --git a/lib/http.c b/lib/http.c
@@ -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);
@@ -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 */
@@ -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 */
@@ -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).
diff --git a/lib/http2.c b/lib/http2.c
@@ -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;
 
@@ -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;
