http-fetch: don't crash when parsing packfile without a repo

The git-http-fetch(1) command accepts a `--packfile=` option, which
allows the user to specify that it shall fetch a specific packfile,
only. The parameter here is the hash of the packfile, which is specific
to the object hash used by the repository. This requirement is implicit
though via our use of `parse_oid_hex()`, which internally uses
`the_repository`.

The git-http-fetch(1) command allows for there to be no repository
though, which only exists such that we can show usage via the "-h"
option. In that case though, starting with c8aed5e8da (repository: stop
setting SHA1 as the default object hash, 2024-05-07), `the_repository`
does not have its object hash initialized anymore and thus we would
crash when trying to parse the object ID outside of a repository.

Fix this issue by dying immediately when we see a "--packfile="
parameter when outside a Git repository. This is not a functional
regression as we would die later on with the same error anyway.

Add a test to detect the segfault. We use the "nongit" function to do
so, which we need to allow-list in `test_must_fail ()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http-fetch.c b/http-fetch.c
index bec94988bbe563a5048c6dd5905f1a9827193489..d460bb1837d06f1c49f43e33e37423069a6c22fb 100644 (file)
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "config.h"
 #include "gettext.h"
@@ -127,8 +129,12 @@ int cmd_main(int argc, const char **argv)
                } else if (skip_prefix(argv[arg], "--packfile=", &p)) {
                        const char *end;
 
+                       if (nongit)
+                               die(_("not a git repository"));
+
                        packfile = 1;
-                       if (parse_oid_hex(p, &packfile_hash, &end) || *end)
+                       if (parse_oid_hex_algop(p, &packfile_hash, &end,
+                                               the_repository->hash_algo) || *end)
                                die(_("argument to --packfile must be a valid hash (got '%s')"), p);
                } else if (skip_prefix(argv[arg], "--index-pack-arg=", &p)) {
                        strvec_push(&index_pack_args, p);

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 5f16cbc58d461a0757a34433f4baf919d6301de2..ea8e48f627315f115a2dbcaf08b47f704c755eaa 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -25,6 +25,12 @@ test_expect_success 'setup repository' '
        git commit -m two
 '
 
+test_expect_success 'packfile without repository does not crash' '
+       echo "fatal: not a git repository" >expect &&
+       test_must_fail nongit git http-fetch --packfile=abc 2>err &&
+       test_cmp expect err
+'
+
 setup_post_update_server_info_hook () {
        test_hook --setup -C "$1" post-update <<-\EOF &&
        exec git update-server-info

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 862d80c9748c7f9d6234c6b47d68ed7854f3de03..34bc7d7da4e82f61ea25863b9bfb9705c99ca409 100644 (file)
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1096,6 +1096,11 @@ test_must_fail_acceptable () {
                done
        fi
 
+       if test "$1" = "nongit"
+       then
+               shift
+       fi
+
        case "$1" in
        git|__git*|scalar|test-tool|test_terminal)
                return 0