From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Mon, 28 Aug 2023 04:35:08 +0000 (+1000)
Subject: hyper: fix ownership problems
X-Git-Tag: curl-8_3_0~98
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b84f274f16449e2d01be9e64faa86845495a3b8;p=thirdparty%2Fcurl.git

hyper: fix ownership problems

Some of these changes come from comparing `Curl_http` and
`start_CONNECT`, which are similar, and adding things to them that are
present in one and missing in another.

The most important changes:
- In `start_CONNECT`, add a missing `hyper_clientconn_free` call on the
  happy path.
- In `start_CONNECT`, add a missing `hyper_request_free` on the error
  path.
- In `bodysend`, add a missing `hyper_body_free` on an early-exit path.
- In `bodysend`, remove an unnecessary `hyper_body_free` on a different
  error path that would cause a double-free.
  https://docs.rs/hyper/latest/hyper/ffi/fn.hyper_request_set_body.html
  says of `hyper_request_set_body`: "This takes ownership of the
  hyper_body *, you must not use it or free it after setting it on the
  request." This is true even if `hyper_request_set_body` returns an
  error; I confirmed this by looking at the hyper source code.

Other changes are minor but make things slightly nicer.

Closes #11745
---

diff --git a/lib/c-hyper.c b/lib/c-hyper.c
index 808e4e1172..f0f49ce6c2 100644
--- a/lib/c-hyper.c
+++ b/lib/c-hyper.c
@@ -808,15 +808,16 @@ static CURLcode bodysend(struct Curl_easy *data,
       hyper_body_set_data_func(body, uploadpostfields);
     else {
       result = Curl_get_upload_buffer(data);
-      if(result)
+      if(result) {
+        hyper_body_free(body);
         return result;
+      }
       /* init the "upload from here" pointer */
       data->req.upload_fromhere = data->state.ulbuf;
       hyper_body_set_data_func(body, uploadstreamed);
     }
     if(HYPERE_OK != hyper_request_set_body(hyperreq, body)) {
       /* fail */
-      hyper_body_free(body);
       result = CURLE_OUT_OF_MEMORY;
     }
   }
@@ -1215,6 +1216,7 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
     result = CURLE_OUT_OF_MEMORY;
     goto error;
   }
+  sendtask = NULL; /* ownership passed on */
 
   hyper_clientconn_free(client);
   client = NULL;
diff --git a/lib/cf-h1-proxy.c b/lib/cf-h1-proxy.c
index 981ef228da..e9bc13d2a8 100644
--- a/lib/cf-h1-proxy.c
+++ b/lib/cf-h1-proxy.c
@@ -715,14 +715,13 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf,
   }
 
   options = hyper_clientconn_options_new();
-  hyper_clientconn_options_set_preserve_header_case(options, 1);
-  hyper_clientconn_options_set_preserve_header_order(options, 1);
-
   if(!options) {
     failf(data, "Couldn't create hyper client options");
     result = CURLE_OUT_OF_MEMORY;
     goto error;
   }
+  hyper_clientconn_options_set_preserve_header_case(options, 1);
+  hyper_clientconn_options_set_preserve_header_order(options, 1);
 
   hyper_clientconn_options_exec(options, h->exec);
 
@@ -753,6 +752,7 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf,
 
   client = hyper_task_value(task);
   hyper_task_free(task);
+
   req = hyper_request_new();
   if(!req) {
     failf(data, "Couldn't hyper_request_new");
@@ -861,12 +861,17 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf,
     result = CURLE_OUT_OF_MEMORY;
     goto error;
   }
+  req = NULL;
 
   if(HYPERE_OK != hyper_executor_push(h->exec, sendtask)) {
     failf(data, "Couldn't hyper_executor_push the send");
     result = CURLE_OUT_OF_MEMORY;
     goto error;
   }
+  sendtask = NULL; /* ownership passed on */
+
+  hyper_clientconn_free(client);
+  client = NULL;
 
 error:
   free(host);
@@ -879,6 +884,9 @@ error:
     hyper_task_free(handshake);
   if(client)
     hyper_clientconn_free(client);
+  if(req)
+    hyper_request_free(req);
+
   return result;
 }