remote: Handle fetching negative refspecs

ryan-ph · ryan-ph · commit f7f30ec13690 · 2024-12-17T09:23:31.000+09:00
Add support for fetching with negative refspecs. Fetching from the
remote with a negative refspec will now validate any references fetched
do not match _any_ negative refspecs and at least one non-negative
refspec. As we must ensure that fetched references do not match any of
the negative refspecs provided, we cannot short-circuit when checking
for matching refspecs.
diff --git a/src/libgit2/refspec.c b/src/libgit2/refspec.c
@@ -22,6 +22,7 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 	const char *lhs, *rhs;
 	int valid = 0;
 	unsigned int flags;
+	bool is_neg_refspec = false;
 
 	GIT_ASSERT_ARG(refspec);
 	GIT_ASSERT_ARG(input);
@@ -34,6 +35,9 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 		refspec->force = 1;
 		lhs++;
 	}
+	if (*lhs == '^') {
+		is_neg_refspec = true;
+	}
 
 	rhs = strrchr(lhs, ':');
 
@@ -62,7 +66,14 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 
 	llen = (rhs ? (size_t)(rhs - lhs - 1) : strlen(lhs));
 	if (1 <= llen && memchr(lhs, '*', llen)) {
-		if ((rhs && !is_glob) || (!rhs && is_fetch))
+		/*
+		 * If the lefthand side contains a glob, then one of the following must be
+		 * true, otherwise the spec is invalid
+		 *   1) the rhs exists and also contains a glob
+		 *   2) it is a negative refspec (i.e. no rhs)
+		 *   3) the rhs doesn't exist and we're fetching
+		 */
+		if ((rhs && !is_glob) || (rhs && is_neg_refspec) || (!rhs && is_fetch && !is_neg_refspec))
 			goto invalid;
 		is_glob = 1;
 	} else if (rhs && is_glob)
diff --git a/src/libgit2/remote.c b/src/libgit2/remote.c
@@ -2628,17 +2628,21 @@ int git_remote_name_is_valid(int *valid, const char *remote_name)
 git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname)
 {
 	git_refspec *spec;
+	git_refspec *match = NULL;
 	size_t i;
 
 	git_vector_foreach(&remote->active_refspecs, i, spec) {
 		if (spec->push)
 			continue;
 
+		if (git_refspec_is_negative(spec) && git_refspec_src_matches_negative(spec, refname))
+			return NULL;
+
 		if (git_refspec_src_matches(spec, refname))
-			return spec;
+			match = spec;
 	}
 
-	return NULL;
+	return match;
 }
 
 git_refspec *git_remote__matching_dst_refspec(git_remote *remote, const char *refname)
diff --git a/tests/libgit2/refs/normalize.c b/tests/libgit2/refs/normalize.c
@@ -402,6 +402,8 @@ void test_refs_normalize__negative_refspec_pattern(void)
 {
 	ensure_refname_normalized(
 		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar", "^foo/bar");
+	ensure_refname_normalized(
+		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar/*", "^foo/bar/*");
 	ensure_refname_invalid(
 		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "foo/^bar");
 }
diff --git a/tests/libgit2/remote/fetch.c b/tests/libgit2/remote/fetch.c
@@ -10,8 +10,11 @@ static char* repo2_path;
 
 static const char *REPO1_REFNAME = "refs/heads/main";
 static const char *REPO2_REFNAME = "refs/remotes/repo1/main";
+static const char *REPO1_UNDERSCORE_REFNAME = "refs/heads/_branch";
+static const char *REPO2_UNDERSCORE_REFNAME = "refs/remotes/repo1/_branch";
 static char *FORCE_FETCHSPEC = "+refs/heads/main:refs/remotes/repo1/main";
 static char *NON_FORCE_FETCHSPEC = "refs/heads/main:refs/remotes/repo1/main";
+static char *NEGATIVE_SPEC = "^refs/heads/_*";
 
 void test_remote_fetch__initialize(void) {
 	git_config *c;
@@ -170,3 +173,64 @@ void test_remote_fetch__do_update_refs_if_not_descendant_and_force(void) {
 
 	git_reference_free(ref);
 }
+
+/**
+ * This checks that negative refspecs are respected when fetching. We create a
+ * repository with a '_' prefixed reference. A second repository is configured
+ * with a negative refspec to ignore any refs prefixed with '_' and fetch the
+ * first repository into the second.
+ *
+ * @param commit1id A pointer to an OID which will be populated with the first
+ *                  commit.
+ */
+static void do_fetch_repo_with_ref_matching_negative_refspec(git_oid *commit1id) {
+	/* create a commit in repo 1 and a reference to it with '_' prefix */
+	{
+		git_oid empty_tree_id;
+		git_tree *empty_tree;
+		git_signature *sig;
+		git_treebuilder *tb;
+		cl_git_pass(git_treebuilder_new(&tb, repo1, NULL));
+		cl_git_pass(git_treebuilder_write(&empty_tree_id, tb));
+		cl_git_pass(git_tree_lookup(&empty_tree, repo1, &empty_tree_id));
+		cl_git_pass(git_signature_default(&sig, repo1));
+		cl_git_pass(git_commit_create(commit1id, repo1, REPO1_UNDERSCORE_REFNAME, sig,
+					sig, NULL, "one", empty_tree, 0, NULL));
+
+		git_tree_free(empty_tree);
+		git_signature_free(sig);
+		git_treebuilder_free(tb);
+	}
+
+	/* fetch the remote with negative refspec for references prefixed with '_' */
+	{
+		char *refspec_strs = { NEGATIVE_SPEC };
+		git_strarray refspecs = {
+			.count = 1,
+			.strings = &refspec_strs,
+		};
+
+		git_remote *remote;
+
+		cl_git_pass(git_remote_create_anonymous(&remote, repo2,
+					git_repository_path(repo1)));
+		cl_git_pass(git_remote_fetch(remote, &refspecs, NULL, "some message"));
+
+		git_remote_free(remote);
+	}
+}
+
+void test_remote_fetch__skip_negative_refspec_match(void) {
+	git_oid commit1id;
+	git_reference *ref1;
+	git_reference *ref2;
+
+	do_fetch_repo_with_ref_matching_negative_refspec(&commit1id);
+
+	/* assert that the reference in exists in repo1 but not in repo2 */
+	cl_git_pass(git_reference_lookup(&ref1, repo1, REPO1_UNDERSCORE_REFNAME));
+	cl_assert_equal_b(git_reference_lookup(&ref2, repo2, REPO2_UNDERSCORE_REFNAME), GIT_ENOTFOUND);
+
+	git_reference_free(ref1);
+	git_reference_free(ref2);
+}

Original file line number	Diff line number	Diff line change
`@@ -402,6 +402,8 @@ void test_refs_normalize__negative_refspec_pattern(void)`
`402`	`402`	`{`
`403`	`403`	`ensure_refname_normalized(`
`404`	`404`	`GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar", "^foo/bar");`
	`405`	`+ ensure_refname_normalized(`
	`406`	`+ GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar/", "^foo/bar/");`
`405`	`407`	`ensure_refname_invalid(`
`406`	`408`	`GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "foo/^bar");`
`407`	`409`	`}`