Commit b2a389c87019c729ffaf179a338236dd129b473c

Patrick Steinhardt 2018-05-30T08:35:06

submodule: detect duplicated submodule paths When loading submodule names, we build a map of submodule paths and their respective names. While looping over the configuration keys, we do not check though whether a submodule path was seen already. This leads to a memory leak in case we have multiple submodules with the same path, as we just overwrite the old value in the map in that case. Fix the error by verifying that the path to be added is not yet part of the string map. Git does not allow to have multiple submodules for a path anyway, so we now do the same and detect this duplication, reporting it to the user.

diff --git a/src/submodule.c b/src/submodule.c
index b927b17..b832286 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -231,6 +231,13 @@ static int load_submodule_names(git_strmap *out, git_repository *repo, git_confi
 		fdot = strchr(entry->name, '.');
 		ldot = strrchr(entry->name, '.');
 
+		if (git_strmap_exists(out, entry->value)) {
+			giterr_set(GITERR_SUBMODULE,
+				   "duplicated submodule path '%s'", entry->value);
+			error = -1;
+			goto out;
+		}
+
 		git_buf_clear(&buf);
 		git_buf_put(&buf, fdot + 1, ldot - fdot - 1);
 		isvalid = git_submodule_name_is_valid(repo, buf.ptr, 0);
diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c
index 170be5a..5db5c2d 100644
--- a/tests/submodule/lookup.c
+++ b/tests/submodule/lookup.c
@@ -132,6 +132,32 @@ void test_submodule_lookup__foreach(void)
 	cl_assert_equal_i(8, data.count);
 }
 
+static int sm_dummy_cb(git_submodule *sm, const char *name, void *payload)
+{
+	GIT_UNUSED(sm);
+	GIT_UNUSED(name);
+	GIT_UNUSED(payload);
+	return 0;
+}
+
+void test_submodule_lookup__duplicated_path(void)
+{
+	/*
+	 * Manually invoke cleanup methods to remove leftovers
+	 * from `setup_fixture_submod2`
+	 */
+	cl_git_sandbox_cleanup();
+	cl_fixture_cleanup("submod2_target");
+
+	g_repo = setup_fixture_submodules();
+
+	/*
+	 * This should fail, as the submodules repo has an
+	 * invalid gitmodules file with duplicated paths.
+	 */
+	cl_git_fail(git_submodule_foreach(g_repo, sm_dummy_cb, NULL));
+}
+
 void test_submodule_lookup__lookup_even_with_unborn_head(void)
 {
 	git_reference *head;