Skip to content

Commit dd8e8c7

Browse files
committed
submodule add: sanity check existing .gitmodules
"git submodule add" tries to find if a submodule with the same name already exists at a different path, by looking up an entry in the .gitmodules file. If the entry in the file is incomplete, e.g., when the submodule.<name>.something variable is defined but there is no definition of submodule.<name>.path variable, it accesses the missing .path member of the submodule structure and triggers a segfault. A brief audit was done to make sure that the code does not assume members other than those that are absolutely certain to exist: a submodule obtained by submodule_from_name() should have .name member, while a submodule obtained by submodule_from_path() should also have .path as well as .name member, and we cannot assume anything else. Luckily, the module_add() codepath was the only problematic one. It is fairly recent code that comes from 1fa06ce (submodule: prevent overwriting .gitmodules on path reuse, 2025-07-24). A helper used by update_submodule() seems to assume that its call to submodule_from_path() always yields a submodule object without a failure, which seems to rely on the caller making sure it is the case. Leave an assert() with a NEEDSWORK comment there for future developers to make sure the assumption actually holds. Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent bb5c624 commit dd8e8c7

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

builtin/submodule--helper.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1913,6 +1913,13 @@ static int determine_submodule_update_strategy(struct repository *r,
19131913
const char *val;
19141914
int ret;
19151915

1916+
/*
1917+
* NEEDSWORK: audit and ensure that update_submodule() has right
1918+
* to assume that submodule_from_path() above will always succeed.
1919+
*/
1920+
if (!sub)
1921+
BUG("update_submodule assumes a submodule exists at path (%s)",
1922+
path);
19161923
key = xstrfmt("submodule.%s.update", sub->name);
19171924

19181925
if (update) {
@@ -3537,14 +3544,15 @@ static int module_add(int argc, const char **argv, const char *prefix,
35373544
}
35383545
}
35393546

3540-
if(!add_data.sm_name)
3547+
if (!add_data.sm_name)
35413548
add_data.sm_name = add_data.sm_path;
35423549

35433550
existing = submodule_from_name(the_repository,
35443551
null_oid(the_hash_algo),
35453552
add_data.sm_name);
35463553

3547-
if (existing && strcmp(existing->path, add_data.sm_path)) {
3554+
if (existing && existing->path &&
3555+
strcmp(existing->path, add_data.sm_path)) {
35483556
if (!force) {
35493557
die(_("submodule name '%s' already used for path '%s'"),
35503558
add_data.sm_name, existing->path);

t/t7400-submodule-basic.sh

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,25 @@ test_expect_success 'submodule deinit works on empty repository' '
4848
git submodule deinit --all
4949
'
5050

51+
test_expect_success 'submodule add with incomplete .gitmodules' '
52+
test_when_finished "rm -f expect actual" &&
53+
test_when_finished "git config remove-section submodule.one" &&
54+
test_when_finished "git rm -f one .gitmodules" &&
55+
git init one &&
56+
git -C one commit --allow-empty -m one-initial &&
57+
git config -f .gitmodules submodule.one.ignore all &&
58+
59+
git submodule add ./one &&
60+
61+
for var in ignore path url
62+
do
63+
git config -f .gitmodules --get "submodule.one.$var" ||
64+
return 1
65+
done >actual &&
66+
test_write_lines all one ./one >expect &&
67+
test_cmp expect actual
68+
'
69+
5170
test_expect_success 'setup - initial commit' '
5271
>t &&
5372
git add t &&

0 commit comments

Comments
 (0)