transport-helper: fix leaking helper name

When initializing the transport helper in `transport_get()`, we
allocate the name of the helper. We neither end up transferring
ownership of the name, nor do we free it. The associated memory thus
leaks.

Fix this memory leak by freeing the string at the calling side in
`transport_get()`. `transport_helper_init()` now creates its own copy of
the string and thus can free it as required.

An alterantive way to fix this would be to transfer ownership of the
string passed into `transport_helper_init()`, which would avoid the call
to xstrdup(1). But it does make for a more surprising calling convention
as we do not typically transfer ownership of strings like this.

Mark now-passing tests as leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Patrick Steinhardt 2024-05-27 13:45:56 +02:00 committed by Junio C Hamano
parent fba95dad6a
commit 97613b9cb9
6 changed files with 9 additions and 2 deletions

View file

@ -2,6 +2,7 @@
test_description='reftable HTTPD tests' test_description='reftable HTTPD tests'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh . ./test-lib.sh
. "$TEST_DIRECTORY"/lib-httpd.sh . "$TEST_DIRECTORY"/lib-httpd.sh

View file

@ -2,6 +2,7 @@
test_description='test http auth header and credential helper interop' test_description='test http auth header and credential helper interop'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh . ./test-lib.sh
. "$TEST_DIRECTORY"/lib-httpd.sh . "$TEST_DIRECTORY"/lib-httpd.sh

View file

@ -2,6 +2,7 @@
test_description="test fetching through http proxy" test_description="test fetching through http proxy"
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh . ./test-lib.sh
. "$TEST_DIRECTORY"/lib-httpd.sh . "$TEST_DIRECTORY"/lib-httpd.sh

View file

@ -4,6 +4,7 @@ test_description='test GIT_CURL_VERBOSE'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh . ./test-lib.sh
. "$TEST_DIRECTORY"/lib-httpd.sh . "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd start_httpd

View file

@ -22,7 +22,7 @@
static int debug; static int debug;
struct helper_data { struct helper_data {
const char *name; char *name;
struct child_process *helper; struct child_process *helper;
FILE *out; FILE *out;
unsigned fetch : 1, unsigned fetch : 1,
@ -111,6 +111,7 @@ static void do_take_over(struct transport *transport)
data = (struct helper_data *)transport->data; data = (struct helper_data *)transport->data;
transport_take_over(transport, data->helper); transport_take_over(transport, data->helper);
fclose(data->out); fclose(data->out);
free(data->name);
free(data); free(data);
} }
@ -253,6 +254,7 @@ static int disconnect_helper(struct transport *transport)
close(data->helper->out); close(data->helper->out);
fclose(data->out); fclose(data->out);
res = finish_command(data->helper); res = finish_command(data->helper);
FREE_AND_NULL(data->name);
FREE_AND_NULL(data->helper); FREE_AND_NULL(data->helper);
} }
return res; return res;
@ -1297,7 +1299,7 @@ static struct transport_vtable vtable = {
int transport_helper_init(struct transport *transport, const char *name) int transport_helper_init(struct transport *transport, const char *name)
{ {
struct helper_data *data = xcalloc(1, sizeof(*data)); struct helper_data *data = xcalloc(1, sizeof(*data));
data->name = name; data->name = xstrdup(name);
transport_check_allowed(name); transport_check_allowed(name);

View file

@ -1176,6 +1176,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
int len = external_specification_len(url); int len = external_specification_len(url);
char *handler = xmemdupz(url, len); char *handler = xmemdupz(url, len);
transport_helper_init(ret, handler); transport_helper_init(ret, handler);
free(handler);
} }
if (ret->smart_options) { if (ret->smart_options) {