Merge pull request #2511 from libgit2/cmn/remote-default-restrict

Restrict which refs can be the default branch

5 files changed, 138 insertions, 27 deletions

diff --git a/script/cibuild.sh b/script/cibuild.sh
index 981f95b7e..e58c0849c 100755
--- a/script/cibuild.sh
+++ b/script/cibuild.sh
@@ -41,7 +41,5 @@ export GITTEST_REMOTE_SSH_PASSPHRASE=""
 
 if [ -e ./libgit2_clar ]; then
     ./libgit2_clar -sonline::push -sonline::clone::cred_callback &&
-    rm -rf $HOME/_temp/test.git &&
-    git init --bare $HOME/_temp/test.git && # create an empty one
     ./libgit2_clar -sonline::clone::ssh_with_paths
 fi
diff --git a/src/clone.c b/src/clone.c
index 7835be1b1..43b839003 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -144,9 +144,9 @@ static int update_head_to_remote(
 		const git_signature *signature,
 		const char *reflog_message)
 {
-	int error = 0, found_branch = 0;
+	int error = 0;
 	size_t refs_len;
-	git_refspec dummy_spec, *refspec;
+	git_refspec *refspec;
 	const git_remote_head *remote_head, **refs;
 	const git_oid *remote_head_id;
 	git_buf remote_master_name = GIT_BUF_INIT;
@@ -155,28 +155,30 @@ static int update_head_to_remote(
 	if ((error = git_remote_ls(&refs, &refs_len, remote)) < 0)
 		return error;
 
-	/* Did we just clone an empty repository? */
-	if (refs_len == 0)
+	/* We cloned an empty repository or one with an unborn HEAD */
+	if (refs_len == 0 || strcmp(refs[0]->name, GIT_HEAD_FILE))
 		return setup_tracking_config(
 			repo, "master", GIT_REMOTE_ORIGIN, GIT_REFS_HEADS_MASTER_FILE);
 
-	error = git_remote_default_branch(&branch, remote);
-	if (error == GIT_ENOTFOUND) {
-		git_buf_puts(&branch, GIT_REFS_HEADS_MASTER_FILE);
-	} else {
-		found_branch = 1;
-	}
-
-	/* Get the remote's HEAD. This is always the first ref in the list. */
+	/* We know we have HEAD, let's see where it points */
 	remote_head = refs[0];
 	assert(remote_head);
 
 	remote_head_id = &remote_head->oid;
+
+	error = git_remote_default_branch(&branch, remote);
+	if (error == GIT_ENOTFOUND) {
+		error = git_repository_set_head_detached(
+			repo, remote_head_id, signature, reflog_message);
+		goto cleanup;
+	}
+
 	refspec = git_remote__matching_refspec(remote, git_buf_cstr(&branch));
 
 	if (refspec == NULL) {
-		memset(&dummy_spec, 0, sizeof(git_refspec));
-		refspec = &dummy_spec;
+		giterr_set(GITERR_NET, "the remote's default branch does not fit the refspec configuration");
+		error = GIT_EINVALIDSPEC;
+		goto cleanup;
 	}
 
 	/* Determine the remote tracking reference name from the local master */
@@ -184,21 +186,18 @@ static int update_head_to_remote(
 		&remote_master_name,
 		refspec,
 		git_buf_cstr(&branch))) < 0)
-		return error;
+		goto cleanup;
 
-	if (found_branch) {
-		error = update_head_to_new_branch(
-			repo,
-			remote_head_id,
-			git_buf_cstr(&branch),
-			signature, reflog_message);
-	} else {
-		error = git_repository_set_head_detached(
-			repo, remote_head_id, signature, reflog_message);
-	}
+	error = update_head_to_new_branch(
+		repo,
+		remote_head_id,
+		git_buf_cstr(&branch),
+		signature, reflog_message);
 
+cleanup:
 	git_buf_free(&remote_master_name);
 	git_buf_free(&branch);
+
 	return error;
 }
 
diff --git a/src/remote.c b/src/remote.c
index 433015f60..fa5ec8b4c 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1955,6 +1955,9 @@ int git_remote_default_branch(git_buf *out, git_remote *remote)
 	if (heads_len == 0)
 		return GIT_ENOTFOUND;
 
+	if (strcmp(heads[0]->name, GIT_HEAD_FILE))
+		return GIT_ENOTFOUND;
+
 	git_buf_sanitize(out);
 	/* the first one must be HEAD so if that has the symref info, we're done */
 	if (heads[0]->symref_target)
@@ -1971,6 +1974,9 @@ int git_remote_default_branch(git_buf *out, git_remote *remote)
 		if (git_oid_cmp(head_id, &heads[i]->oid))
 			continue;
 
+		if (git__prefixcmp(heads[i]->name, GIT_REFS_HEADS_DIR))
+			continue;
+
 		if (!guess) {
 			guess = heads[i];
 			continue;
diff --git a/tests/network/remote/defaultbranch.c b/tests/network/remote/defaultbranch.c
index fa3a329db..243369fa2 100644
--- a/tests/network/remote/defaultbranch.c
+++ b/tests/network/remote/defaultbranch.c
@@ -48,3 +48,61 @@ void test_network_remote_defaultbranch__master_on_detached(void)
 	cl_git_pass(git_repository_detach_head(g_repo_a, NULL, NULL));
 	assert_default_branch("refs/heads/master");
 }
+
+void test_network_remote_defaultbranch__no_default_branch(void)
+{
+	git_remote *remote_b;
+	const git_remote_head **heads;
+	size_t len;
+	git_buf buf = GIT_BUF_INIT;
+
+	cl_git_pass(git_remote_create(&remote_b, g_repo_b, "self", git_repository_path(g_repo_b)));
+	cl_git_pass(git_remote_connect(remote_b, GIT_DIRECTION_FETCH));
+	cl_git_pass(git_remote_ls(&heads, &len, remote_b));
+	cl_assert_equal_i(0, len);
+
+	cl_git_fail_with(GIT_ENOTFOUND, git_remote_default_branch(&buf, remote_b));
+
+	git_remote_free(remote_b);
+}
+
+void test_network_remote_defaultbranch__detached_sharing_nonbranch_id(void)
+{
+	git_oid id, id_cloned;
+	git_reference *ref;
+	git_buf buf = GIT_BUF_INIT;
+	git_repository *cloned_repo;
+
+	cl_git_pass(git_reference_name_to_id(&id, g_repo_a, "HEAD"));
+	cl_git_pass(git_repository_detach_head(g_repo_a, NULL, NULL));
+	cl_git_pass(git_reference_remove(g_repo_a, "refs/heads/master"));
+	cl_git_pass(git_reference_remove(g_repo_a, "refs/heads/not-good"));
+	cl_git_pass(git_reference_create(&ref, g_repo_a, "refs/foo/bar", &id, 1, NULL, NULL));
+	git_reference_free(ref);
+
+	cl_git_pass(git_remote_connect(g_remote, GIT_DIRECTION_FETCH));
+	cl_git_fail_with(GIT_ENOTFOUND, git_remote_default_branch(&buf, g_remote));
+
+	cl_git_pass(git_clone(&cloned_repo, git_repository_path(g_repo_a), "./local-detached", NULL));
+
+	cl_assert(git_repository_head_detached(cloned_repo));
+	cl_git_pass(git_reference_name_to_id(&id_cloned, g_repo_a, "HEAD"));
+	cl_assert(git_oid_equal(&id, &id_cloned));
+
+	git_repository_free(cloned_repo);
+}
+
+void test_network_remote_defaultbranch__unborn_HEAD_with_branches(void)
+{
+	git_reference *ref;
+	git_repository *cloned_repo;
+
+	cl_git_pass(git_reference_symbolic_create(&ref, g_repo_a, "HEAD", "refs/heads/i-dont-exist", 1, NULL, NULL));
+	git_reference_free(ref);
+
+	cl_git_pass(git_clone(&cloned_repo, git_repository_path(g_repo_a), "./semi-empty", NULL));
+
+	cl_assert(git_repository_head_unborn(cloned_repo));
+
+	git_repository_free(cloned_repo);
+}
diff --git a/tests/network/remote/remotes.c b/tests/network/remote/remotes.c
index 45f2a795f..d176774ea 100644
--- a/tests/network/remote/remotes.c
+++ b/tests/network/remote/remotes.c
@@ -509,3 +509,53 @@ void test_network_remote_remotes__query_refspecs(void)
 
 	git_remote_free(remote);
 }
+
+static int remote_single_branch(git_remote **out, git_repository *repo, const char *name, const char *url, void *payload)
+{
+	char *fetch_refspecs[] = {
+		"refs/heads/first-merge:refs/remotes/origin/first-merge",
+	};
+	git_strarray fetch_refspecs_strarray = {
+		fetch_refspecs,
+		1,
+	};
+
+	GIT_UNUSED(payload);
+
+	cl_git_pass(git_remote_create(out, repo, name, url));
+	cl_git_pass(git_remote_set_fetch_refspecs(*out, &fetch_refspecs_strarray));
+
+	return 0;
+}
+
+void test_network_remote_remotes__single_branch(void)
+{
+	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
+	git_repository *repo;
+	git_strarray refs;
+	size_t i, count = 0;
+
+	opts.remote_cb = remote_single_branch;
+	opts.checkout_branch = "first-merge";
+
+	cl_git_pass(git_clone(&repo, "git://github.com/libgit2/TestGitRepository", "./single-branch", &opts));
+	cl_git_pass(git_reference_list(&refs, repo));
+
+	for (i = 0; i < refs.count; i++) {
+		if (!git__prefixcmp(refs.strings[i], "refs/heads/"))
+			count++;
+	}
+	cl_assert_equal_i(1, count);
+
+	git_repository_free(repo);
+}
+
+void test_network_remote_remotes__restricted_refspecs(void)
+{
+	git_clone_options opts = GIT_CLONE_OPTIONS_INIT;
+	git_repository *repo;
+
+	opts.remote_cb = remote_single_branch;
+
+	cl_git_fail_with(GIT_EINVALIDSPEC, git_clone(&repo, "git://github.com/libgit2/TestGitRepository", "./restrict-refspec", &opts));
+}