From: Jeff King <peff@peff.net>
Date: Thu, 2 Jul 2026 08:07:07 +0000 (-0400)
Subject: http: discard hash in dumb-http http_object_request
X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=a2d8ea5a766c7f16afd4294acb7a618b67de75f2;p=thirdparty%2Fgit.git

http: discard hash in dumb-http http_object_request

Usually an object request results in finish_http_object_request()
calling git_hash_final_oid(), after we've received all of the data. But
if we hit an error, we'll bail early and free the http_object_request,
dropping the git_hash_ctx entirely.  This can cause a leak for hash
implementations that allocate memory in their context, like OpenSSL >=
3.0.

The obvious fix is for abort_http_object_request() to call
git_hash_discard(), under the assumption that every request is either
finished or aborted. But that's not quite true:

  1. Not everybody calls the abort function. Sometimes they jump
     straight to release_http_object_request(). So we'd have to put it
     there.

  2. After the finish function finalizes the hash, we can still
     encounter errors! In that case we end up aborting or releasing,
     and they must not discard that hash (since that would be a
     double-free).

So we'll keep a flag marking the validity of the hash_ctx field of the
request. The lifetime is simple: it is valid immediately after creation,
up until we call finalize. And then our release function can just
conditionally discard the hash based on that flag.

This fixes test failures in t5550 and t5619 when run with:

  make SANITIZE=leak \
       OPENSSL_SHA256=1 \
       GIT_TEST_DEFAULT_HASH=sha256 \
       test

The flag handling could be removed if the hash-discard function were
idempotent. This could be done easily-ish by having the underlying
hash functions (like the ones in sha256/openssl.h) set the context
pointer to NULL after free-ing. But it's something that every platform
implementation would have to remember to do, and the benefit for the
callers is not that huge (it would let us shave a few lines here and
probably in a few other spots).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

diff --git a/http.c b/http.c
index 67c9c6fc60..8b0de51984 100644
--- a/http.c
+++ b/http.c
@@ -2806,6 +2806,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	git_inflate_init(&freq->stream);
 
 	the_hash_algo->init_fn(&freq->c);
+	freq->hash_ctx_valid = 1;
 
 	freq->url = get_remote_object_url(base_url, hex, 0);
 
@@ -2913,6 +2914,7 @@ int finish_http_object_request(struct http_object_request *freq)
 	}
 
 	git_hash_final_oid(&freq->real_oid, &freq->c);
+	freq->hash_ctx_valid = 0;
 	if (freq->zret != Z_STREAM_END) {
 		unlink_or_warn(freq->tmpfile.buf);
 		return -1;
@@ -2953,6 +2955,8 @@ void release_http_object_request(struct http_object_request **freq_p)
 	curl_slist_free_all(freq->headers);
 	strbuf_release(&freq->tmpfile);
 	git_inflate_end(&freq->stream);
+	if (freq->hash_ctx_valid)
+		git_hash_discard(&freq->c);
 
 	free(freq);
 	*freq_p = NULL;
diff --git a/http.h b/http.h
index f9ee888c3e..dc9700c113 100644
--- a/http.h
+++ b/http.h
@@ -249,6 +249,7 @@ struct http_object_request {
 	struct object_id oid;
 	struct object_id real_oid;
 	struct git_hash_ctx c;
+	int hash_ctx_valid;
 	git_zstream stream;
 	int zret;
 	int rename;