Merge pull request #5060 from pks-t/pks/refspec-nested-globs

Loosen restriction on wildcard "*" refspecs

4 files changed, 49 insertions, 36 deletions

diff --git a/src/refs.c b/src/refs.c
index a031842bf..4604d4c4c 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -897,14 +897,13 @@ static int is_valid_ref_char(char ch)
 	case '\\':
 	case '?':
 	case '[':
-	case '*':
 		return 0;
 	default:
 		return 1;
 	}
 }
 
-static int ensure_segment_validity(const char *name)
+static int ensure_segment_validity(const char *name, char may_contain_glob)
 {
 	const char *current = name;
 	char prev = '\0';
@@ -927,6 +926,12 @@ static int ensure_segment_validity(const char *name)
 		if (prev == '@' && *current == '{')
 			return -1; /* Refname contains "@{" */
 
+		if (*current == '*') {
+			if (!may_contain_glob)
+				return -1;
+			may_contain_glob = 0;
+		}
+
 		prev = *current;
 	}
 
@@ -1005,19 +1010,20 @@ int git_reference__normalize_name(
 	}
 
 	while (true) {
-		segment_len = ensure_segment_validity(current);
-		if (segment_len < 0) {
-			if ((process_flags & GIT_REFERENCE_FORMAT_REFSPEC_PATTERN) &&
-					current[0] == '*' &&
-					(current[1] == '\0' || current[1] == '/')) {
-				/* Accept one wildcard as a full refname component. */
-				process_flags &= ~GIT_REFERENCE_FORMAT_REFSPEC_PATTERN;
-				segment_len = 1;
-			} else
-				goto cleanup;
-		}
+		char may_contain_glob = process_flags & GIT_REFERENCE_FORMAT_REFSPEC_PATTERN;
+
+		segment_len = ensure_segment_validity(current, may_contain_glob);
+		if (segment_len < 0)
+			goto cleanup;
 
 		if (segment_len > 0) {
+			/*
+			 * There may only be one glob in a pattern, thus we reset
+			 * the pattern-flag in case the current segment has one.
+			 */
+			if (memchr(current, '*', segment_len))
+				process_flags &= ~GIT_REFERENCE_FORMAT_REFSPEC_PATTERN;
+
 			if (normalize) {
 				size_t cur_len = git_buf_len(buf);
 
diff --git a/src/refspec.c b/src/refspec.c
index eaf0c2c83..dab053634 100644
--- a/src/refspec.c
+++ b/src/refspec.c
@@ -228,7 +228,6 @@ static int refspec_transform(
 	git_buf *out, const char *from, const char *to, const char *name)
 {
 	const char *from_star, *to_star;
-	const char *name_slash, *from_slash;
 	size_t replacement_len, star_offset;
 
 	git_buf_sanitize(out);
@@ -251,17 +250,11 @@ static int refspec_transform(
 	/* the first half is copied over */
 	git_buf_put(out, to, to_star - to);
 
-	/* then we copy over the replacement, from the star's offset to the next slash in 'name' */
-	name_slash = strchr(name + star_offset, '/');
-	if (!name_slash)
-		name_slash = strrchr(name, '\0');
-
-	/* if there is no slash after the star in 'from', we want to copy everything over */
-	from_slash = strchr(from + star_offset, '/');
-	if (!from_slash)
-		name_slash = strrchr(name, '\0');
-
-	replacement_len = (name_slash - name) - star_offset;
+	/*
+	 * Copy over the name, but exclude the trailing part in "from" starting
+	 * after the glob
+	 */
+	replacement_len = strlen(name + star_offset) - strlen(from_star + 1);
 	git_buf_put(out, name + star_offset, replacement_len);
 
 	return git_buf_puts(out, to_star + 1);
diff --git a/tests/network/refspecs.c b/tests/network/refspecs.c
index 1a65fd246..734759060 100644
--- a/tests/network/refspecs.c
+++ b/tests/network/refspecs.c
@@ -70,15 +70,18 @@ void test_network_refspecs__parsing(void)
 	assert_refspec(GIT_DIRECTION_PUSH, ":refs/remotes/frotz/delete me", false);
 	assert_refspec(GIT_DIRECTION_FETCH, ":refs/remotes/frotz/HEAD to me", false);
 
-	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads/*/for-linus:refs/remotes/mine/*-blah", false);
-	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads/*/for-linus:refs/remotes/mine/*-blah", false);
+	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads/*/for-linus:refs/remotes/mine/*-blah", true);
+	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads/*/for-linus:refs/remotes/mine/*-blah", true);
 
-	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads*/for-linus:refs/remotes/mine/*", false);
-	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads*/for-linus:refs/remotes/mine/*", false);
+	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads*/for-linus:refs/remotes/mine/*", true);
+	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads*/for-linus:refs/remotes/mine/*", true);
 
 	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads/*/*/for-linus:refs/remotes/mine/*", false);
 	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads/*/*/for-linus:refs/remotes/mine/*", false);
 
+	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads/*g*/for-linus:refs/remotes/mine/*", false);
+	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads/*g*/for-linus:refs/remotes/mine/*", false);
+
 	assert_refspec(GIT_DIRECTION_FETCH, "refs/heads/*/for-linus:refs/remotes/mine/*", true);
 	assert_refspec(GIT_DIRECTION_PUSH, "refs/heads/*/for-linus:refs/remotes/mine/*", true);
 
@@ -93,7 +96,7 @@ static void assert_valid_transform(const char *refspec, const char *name, const
 	git_refspec spec;
 	git_buf buf = GIT_BUF_INIT;
 
-	git_refspec__parse(&spec, refspec, true);
+	cl_git_pass(git_refspec__parse(&spec, refspec, true));
 	cl_git_pass(git_refspec_transform(&buf, &spec, name));
 	cl_assert_equal_s(result, buf.ptr);
 
@@ -111,6 +114,17 @@ void test_network_refspecs__transform_mid_star(void)
 	assert_valid_transform("refs/*:refs/*", "refs/heads/master", "refs/heads/master");
 }
 
+void test_network_refspecs__transform_loosened_star(void)
+{
+	assert_valid_transform("refs/heads/branch-*:refs/remotes/origin/branch-*", "refs/heads/branch-a", "refs/remotes/origin/branch-a");
+	assert_valid_transform("refs/heads/branch-*/head:refs/remotes/origin/branch-*/head", "refs/heads/branch-a/head", "refs/remotes/origin/branch-a/head");
+}
+
+void test_network_refspecs__transform_nested_star(void)
+{
+	assert_valid_transform("refs/heads/x*x/for-linus:refs/remotes/mine/*", "refs/heads/xbranchx/for-linus", "refs/remotes/mine/branch");
+}
+
 void test_network_refspecs__no_dst(void)
 {
 	assert_valid_transform("refs/heads/master:", "refs/heads/master", "");
diff --git a/tests/refs/normalize.c b/tests/refs/normalize.c
index 6da005da0..ff815002c 100644
--- a/tests/refs/normalize.c
+++ b/tests/refs/normalize.c
@@ -352,12 +352,12 @@ void test_refs_normalize__buffer_has_to_be_big_enough_to_hold_the_normalized_ver
 
 void test_refs_normalize__refspec_pattern(void)
 {
-	ensure_refname_invalid(
-		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/*foo/bar");
-	ensure_refname_invalid(
-		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/foo*/bar");
-	ensure_refname_invalid(
-		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/f*o/bar");
+	ensure_refname_normalized(
+		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/*foo/bar", "heads/*foo/bar");
+	ensure_refname_normalized(
+		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/foo*/bar", "heads/foo*/bar");
+	ensure_refname_normalized(
+		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "heads/f*o/bar", "heads/f*o/bar");
 
 	ensure_refname_invalid(
 		GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "foo");