Skip to content

Commit afa2c6d

Browse files
pks-tgitster
authored andcommitted
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 c8aed5e (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8a676bd commit afa2c6d

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

http-fetch.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#define USE_THE_REPOSITORY_VARIABLE
2+
13
#include "git-compat-util.h"
24
#include "config.h"
35
#include "gettext.h"
@@ -127,8 +129,12 @@ int cmd_main(int argc, const char **argv)
127129
} else if (skip_prefix(argv[arg], "--packfile=", &p)) {
128130
const char *end;
129131

132+
if (nongit)
133+
die(_("not a git repository"));
134+
130135
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)
132138
die(_("argument to --packfile must be a valid hash (got '%s')"), p);
133139
} else if (skip_prefix(argv[arg], "--index-pack-arg=", &p)) {
134140
strvec_push(&index_pack_args, p);

t/t5550-http-fetch-dumb.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ test_expect_success 'setup repository' '
2525
git commit -m two
2626
'
2727

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+
2834
setup_post_update_server_info_hook () {
2935
test_hook --setup -C "$1" post-update <<-\EOF &&
3036
exec git update-server-info

t/test-lib-functions.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,11 @@ test_must_fail_acceptable () {
10961096
done
10971097
fi
10981098

1099+
if test "$1" = "nongit"
1100+
then
1101+
shift
1102+
fi
1103+
10991104
case "$1" in
11001105
git|__git*|scalar|test-tool|test_terminal)
11011106
return 0

0 commit comments

Comments
 (0)