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>

Patrick Steinhardt committed Jun 14, 2024 at 08:50 UTC afa2c6ddc88db766d16d471aa12b5db6bac39158
3 files changed +18 -1
http-fetch.c
+7 -1
@@ -1,3 +1,5 @@
1 +#define USE_THE_REPOSITORY_VARIABLE
2 +
3 #include "git-compat-util.h"
4 #include "config.h"
5 #include "gettext.h"
@@ -127,8 +129,12 @@ int cmd_main(int argc, const char **argv)
129 } else if (skip_prefix(argv[arg], "--packfile=", &p)) {
130 const char *end;
131
132 + if (nongit)
133 + die(_("not a git repository"));
134 +
135 packfile = 1;
131 - if (parse_oid_hex(p, &packfile_hash, &end) || *end)
136 + if (parse_oid_hex_algop(p, &packfile_hash, &end,
137 + the_repository->hash_algo) || *end)
138 die(_("argument to --packfile must be a valid hash (got '%s')"), p);
139 } else if (skip_prefix(argv[arg], "--index-pack-arg=", &p)) {
140 strvec_push(&index_pack_args, p);
t/t5550-http-fetch-dumb.sh
+6
@@ -25,6 +25,12 @@ test_expect_success 'setup repository' '
25 git commit -m two
26 '
27
28 +test_expect_success 'packfile without repository does not crash' '
29 + echo "fatal: not a git repository" >expect &&
30 + test_must_fail nongit git http-fetch --packfile=abc 2>err &&
31 + test_cmp expect err
32 +'
33 +
34 setup_post_update_server_info_hook () {
35 test_hook --setup -C "$1" post-update <<-\EOF &&
36 exec git update-server-info
t/test-lib-functions.sh
+5
@@ -1096,6 +1096,11 @@ test_must_fail_acceptable () {
1096 done
1097 fi
1098
1099 + if test "$1" = "nongit"
1100 + then
1101 + shift
1102 + fi
1103 +
1104 case "$1" in
1105 git|__git*|scalar|test-tool|test_terminal)
1106 return 0