Skip to content

Commit 041fad4

Browse files
author
Vicent Marti
committed
Merge pull request libgit2#2210 from libgit2/rb/submodule-api-with-no-submodules
Fix segfault if gitmodules is invalid
2 parents f4afcaa + 380f864 commit 041fad4

File tree

4 files changed

+139
-7
lines changed

4 files changed

+139
-7
lines changed

‎include/git2/submodule.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ typedef enum {
115115
*
116116
* - The submodule is not mentioned in the HEAD, the index, and the config,
117117
* but does "exist" in the working directory (i.e. there is a subdirectory
118-
* that is a valid self-contained git repo). In this case, this function
119-
* returns GIT_EEXISTS to indicate the the submodule exists but not in a
118+
* that appears to be a Git repository). In this case, this function
119+
* returns GIT_EEXISTS to indicate a sub-repository exists but not in a
120120
* state where a git_submodule can be instantiated.
121121
* - The submodule is not mentioned in the HEAD, index, or config and the
122122
* working directory doesn't contain a value git repo at that path.
@@ -129,7 +129,7 @@ typedef enum {
129129
* @param repo The parent repository
130130
* @param name The name of or path to the submodule; trailing slashes okay
131131
* @return 0 on success, GIT_ENOTFOUND if submodule does not exist,
132-
* GIT_EEXISTS if submodule exists in working directory only,
132+
* GIT_EEXISTS if a repository is found in working directory only,
133133
* -1 on other errors.
134134
*/
135135
GIT_EXTERN(int) git_submodule_lookup(

‎src/submodule.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ int git_submodule_lookup(
156156
if (git_buf_joinpath(&path, git_repository_workdir(repo), name) < 0)
157157
return -1;
158158

159-
if (git_path_contains_dir(&path, DOT_GIT))
159+
if (git_path_contains(&path, DOT_GIT))
160160
error = GIT_EEXISTS;
161161

162162
git_buf_free(&path);
@@ -846,7 +846,8 @@ int git_submodule_reload_all(git_repository *repo, int force)
846846
if (repo->submodules)
847847
git_strmap_foreach_value(repo->submodules, sm, { sm->flags = 0; });
848848

849-
error = load_submodule_config(repo, true);
849+
if ((error = load_submodule_config(repo, true)) < 0)
850+
return error;
850851

851852
git_strmap_foreach_value(repo->submodules, sm, {
852853
git_strmap *cache = repo->submodules;
@@ -1278,14 +1279,17 @@ static int submodule_load_from_config(
12781279
}
12791280
}
12801281

1282+
/* Found a alternate key for the submodule */
12811283
if (alternate) {
12821284
void *old_sm = NULL;
12831285
git_strmap_insert2(smcfg, alternate, sm, old_sm, error);
12841286

12851287
if (error < 0)
12861288
goto done;
1287-
if (error >= 0)
1288-
GIT_REFCOUNT_INC(sm); /* inserted under a new key */
1289+
if (error > 0)
1290+
error = 0;
1291+
1292+
GIT_REFCOUNT_INC(sm); /* increase refcount for new key */
12891293

12901294
/* if we replaced an old module under this key, release the old one */
12911295
if (old_sm && ((git_submodule *)old_sm) != sm) {

‎tests/submodule/lookup.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,36 @@ void test_submodule_lookup__lookup_even_with_missing_index(void)
141141
refute_submodule_exists(g_repo, "just_a_file", GIT_ENOTFOUND);
142142
refute_submodule_exists(g_repo, "no_such_file", GIT_ENOTFOUND);
143143
}
144+
145+
void test_submodule_lookup__just_added(void)
146+
{
147+
git_submodule *sm;
148+
149+
cl_git_pass(git_submodule_add_setup(&sm, g_repo, "https://github.com/libgit2/libgit2.git", "sm_just_added", 1));
150+
git_submodule_free(sm);
151+
assert_submodule_exists(g_repo, "sm_just_added");
152+
153+
cl_git_pass(git_submodule_add_setup(&sm, g_repo, "https://github.com/libgit2/libgit2.git", "sm_just_added_2", 1));
154+
assert_submodule_exists(g_repo, "sm_just_added_2");
155+
git_submodule_free(sm);
156+
157+
cl_git_append2file("submod2/.gitmodules", "\n[submodule \"mismatch_name\"]\n\tpath = mismatch_path\n\turl = https://example.com/example.git\n\n");
158+
159+
cl_git_pass(git_submodule_reload_all(g_repo, 1));
160+
161+
assert_submodule_exists(g_repo, "mismatch_name");
162+
assert_submodule_exists(g_repo, "mismatch_path");
163+
164+
assert_submodule_exists(g_repo, "sm_just_added");
165+
assert_submodule_exists(g_repo, "sm_just_added_2");
166+
167+
/* all the regular ones should still be working right, too */
168+
169+
assert_submodule_exists(g_repo, "sm_unchanged");
170+
assert_submodule_exists(g_repo, "sm_added_and_uncommited");
171+
assert_submodule_exists(g_repo, "sm_gitmodules_only");
172+
refute_submodule_exists(g_repo, "not-submodule", GIT_EEXISTS);
173+
refute_submodule_exists(g_repo, "just_a_dir", GIT_ENOTFOUND);
174+
refute_submodule_exists(g_repo, "just_a_file", GIT_ENOTFOUND);
175+
refute_submodule_exists(g_repo, "no_such_file", GIT_ENOTFOUND);
176+
}

‎tests/submodule/nosubs.c

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/* test the submodule APIs on repositories where there are no submodules */
2+
3+
#include "clar_libgit2.h"
4+
#include "posix.h"
5+
6+
void test_submodule_nosubs__cleanup(void)
7+
{
8+
cl_git_sandbox_cleanup();
9+
}
10+
11+
void test_submodule_nosubs__lookup(void)
12+
{
13+
git_repository *repo = cl_git_sandbox_init("status");
14+
git_submodule *sm = NULL;
15+
16+
p_mkdir("status/subrepo", 0777);
17+
cl_git_mkfile("status/subrepo/.git", "gitdir: ../.git");
18+
19+
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(&sm, repo, "subdir"));
20+
21+
cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo"));
22+
23+
cl_git_pass(git_submodule_reload_all(repo, 0));
24+
25+
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(&sm, repo, "subdir"));
26+
27+
cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo"));
28+
}
29+
30+
void test_submodule_nosubs__immediate_reload(void)
31+
{
32+
git_repository *repo = cl_git_sandbox_init("status");
33+
cl_git_pass(git_submodule_reload_all(repo, 0));
34+
}
35+
36+
static int fake_submod_cb(git_submodule *sm, const char *n, void *p)
37+
{
38+
GIT_UNUSED(sm); GIT_UNUSED(n); GIT_UNUSED(p);
39+
return 0;
40+
}
41+
42+
void test_submodule_nosubs__foreach(void)
43+
{
44+
git_repository *repo = cl_git_sandbox_init("status");
45+
cl_git_pass(git_submodule_foreach(repo, fake_submod_cb, NULL));
46+
}
47+
48+
void test_submodule_nosubs__add(void)
49+
{
50+
git_repository *repo = cl_git_sandbox_init("status");
51+
git_submodule *sm, *sm2;
52+
53+
cl_git_pass(git_submodule_add_setup(&sm, repo, "https://github.com/libgit2/libgit2.git", "submodules/libgit2", 1));
54+
55+
cl_git_pass(git_submodule_lookup(&sm2, repo, "submodules/libgit2"));
56+
git_submodule_free(sm2);
57+
58+
cl_git_pass(git_submodule_foreach(repo, fake_submod_cb, NULL));
59+
cl_git_pass(git_submodule_reload_all(repo, 0));
60+
61+
git_submodule_free(sm);
62+
}
63+
64+
void test_submodule_nosubs__reload_add_reload(void)
65+
{
66+
git_repository *repo = cl_git_sandbox_init("status");
67+
git_submodule *sm;
68+
69+
cl_git_pass(git_submodule_reload_all(repo, 0));
70+
71+
cl_git_pass(git_submodule_add_setup(&sm, repo, "https://github.com/libgit2/libgit2.git", "submodules/libgit2", 1));
72+
73+
cl_git_pass(git_submodule_reload_all(repo, 0));
74+
75+
cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
76+
git_submodule_free(sm);
77+
78+
cl_git_pass(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
79+
cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
80+
git_submodule_free(sm);
81+
}
82+
83+
void test_submodule_nosubs__bad_gitmodules(void)
84+
{
85+
git_repository *repo = cl_git_sandbox_init("status");
86+
87+
cl_git_mkfile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=flooble\n\n");
88+
cl_git_fail(git_submodule_reload_all(repo, 0));
89+
90+
cl_git_rewritefile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=rebase\n\n");
91+
cl_git_pass(git_submodule_reload_all(repo, 0));
92+
93+
cl_git_pass(git_submodule_lookup(NULL, repo, "foobar"));
94+
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(NULL, repo, "subdir"));
95+
}

0 commit comments

Comments
 (0)