Merge pull request #861 from josh/parse-chomped-oid

Parse ref oids without trailing newline

src/refs.c

@@ -128,6 +128,7 @@ static int reference_read(
 
 	result = git_futils_readbuffer_updated(file_content, path.ptr, mtime, updated);
 	git_buf_free(&path);
+
 	return result;
 }
 
@@ -135,12 +136,13 @@ static int loose_parse_symbolic(git_reference *ref, git_buf *file_content)
 {
 	const unsigned int header_len = (unsigned int)strlen(GIT_SYMREF);
 	const char *refname_start;
-	char *eol;
 
 	refname_start = (const char *)file_content->ptr;
 
-	if (git_buf_len(file_content) < header_len + 1)
-		goto corrupt;
+	if (git_buf_len(file_content) < header_len + 1) {
+		giterr_set(GITERR_REFERENCE, "Corrupted loose reference file");
+		return -1;
+	}
 
 	/*
 	 * Assume we have already checked for the header
@@ -151,45 +153,16 @@ static int loose_parse_symbolic(git_reference *ref, git_buf *file_content)
 	ref->target.symbolic = git__strdup(refname_start);
 	GITERR_CHECK_ALLOC(ref->target.symbolic);
 
-	/* remove newline at the end of file */
-	eol = strchr(ref->target.symbolic, '\n');
-	if (eol == NULL)
-		goto corrupt;
-
-	*eol = '\0';
-	if (eol[-1] == '\r')
-		eol[-1] = '\0';
-
 	return 0;
-
-corrupt:
-	giterr_set(GITERR_REFERENCE, "Corrupted loose reference file");
-	return -1;
 }
 
 static int loose_parse_oid(git_oid *oid, git_buf *file_content)
 {
-	char *buffer;
-
-	buffer = (char *)file_content->ptr;
-
-	/* File format: 40 chars (OID) + newline */
-	if (git_buf_len(file_content) < GIT_OID_HEXSZ + 1)
-		goto corrupt;
-
-	if (git_oid_fromstr(oid, buffer) < 0)
-		goto corrupt;
-
-	buffer = buffer + GIT_OID_HEXSZ;
-	if (*buffer == '\r')
-		buffer++;
-
-	if (*buffer != '\n')
-		goto corrupt;
-
+	/* File format: 40 chars (OID) */
+	if (git_buf_len(file_content) == GIT_OID_HEXSZ &&
+		git_oid_fromstr(oid, git_buf_cstr(file_content)) == 0)
 		return 0;
 
-corrupt:
 	giterr_set(GITERR_REFERENCE, "Corrupted loose reference file");
 	return -1;
 }
@@ -226,6 +199,8 @@ static int loose_lookup(git_reference *ref)
 	if (!updated)
 		return 0;
 
+	git_buf_rtrim(&ref_file);
+
 	if (ref->flags & GIT_REF_SYMBOLIC) {
 		git__free(ref->target.symbolic);
 		ref->target.symbolic = NULL;
@@ -259,6 +234,8 @@ static int loose_lookup_to_packfile(
 	if (reference_read(&ref_file, NULL, repo->path_repository, name, NULL) < 0)
 		return -1;
 
+	git_buf_rtrim(&ref_file);
+
 	name_len = strlen(name);
 	ref = git__malloc(sizeof(struct packref) + name_len + 1);
 	GITERR_CHECK_ALLOC(ref);

tests-clar/network/remotelocal.c

@@ -107,7 +107,7 @@ void test_network_remotelocal__retrieve_advertised_references(void)
 
 	cl_git_pass(git_remote_ls(remote, &count_ref__cb, &how_many_refs));
 
-	cl_assert_equal_i(how_many_refs, 23);
+	cl_assert_equal_i(how_many_refs, 25);
 }
 
 void test_network_remotelocal__retrieve_advertised_references_from_spaced_repository(void)
@@ -121,7 +121,7 @@ void test_network_remotelocal__retrieve_advertised_references_from_spaced_reposi
 
 	cl_git_pass(git_remote_ls(remote, &count_ref__cb, &how_many_refs));
 
-	cl_assert_equal_i(how_many_refs, 23);
+	cl_assert_equal_i(how_many_refs, 25);
 
 	git_remote_free(remote);	/* Disconnect from the "spaced repo" before the cleanup */
 	remote = NULL;

tests-clar/refs/branches/foreach.c

@@ -47,7 +47,7 @@ static void assert_retrieval(unsigned int flags, unsigned int expected_count)
 
 void test_refs_branches_foreach__retrieve_all_branches(void)
 {
-	assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 11);
+	assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 13);
 }
 
 void test_refs_branches_foreach__retrieve_remote_branches(void)
@@ -57,7 +57,7 @@ void test_refs_branches_foreach__retrieve_remote_branches(void)
 
 void test_refs_branches_foreach__retrieve_local_branches(void)
 {
-	assert_retrieval(GIT_BRANCH_LOCAL, 9);
+	assert_retrieval(GIT_BRANCH_LOCAL, 11);
 }
 
 struct expectations {

tests-clar/refs/foreachglob.c

@@ -45,8 +45,8 @@ static void assert_retrieval(const char *glob, unsigned int flags, int expected_
 
 void test_refs_foreachglob__retrieve_all_refs(void)
 {
-	/* 7 heads (including one packed head) + 1 note + 2 remotes + 6 tags */
-	assert_retrieval("*", GIT_REF_LISTALL, 18);
+	/* 8 heads (including one packed head) + 1 note + 2 remotes + 6 tags */
+	assert_retrieval("*", GIT_REF_LISTALL, 20);
 }
 
 void test_refs_foreachglob__retrieve_remote_branches(void)
@@ -56,7 +56,7 @@ void test_refs_foreachglob__retrieve_remote_branches(void)
 
 void test_refs_foreachglob__retrieve_local_branches(void)
 {
-	assert_retrieval("refs/heads/*", GIT_REF_LISTALL, 9);
+	assert_retrieval("refs/heads/*", GIT_REF_LISTALL, 11);
 }
 
 void test_refs_foreachglob__retrieve_partially_named_references(void)

tests-clar/refs/read.c

@@ -193,6 +193,30 @@ void test_refs_read__loose_first(void)
 	git_reference_free(reference);
 }
 
+void test_refs_read__chomped(void)
+{
+	git_reference *test, *chomped;
+
+	cl_git_pass(git_reference_lookup(&test, g_repo, "refs/heads/test"));
+	cl_git_pass(git_reference_lookup(&chomped, g_repo, "refs/heads/chomped"));
+	cl_git_pass(git_oid_cmp(git_reference_oid(test), git_reference_oid(chomped)));
+
+	git_reference_free(test);
+	git_reference_free(chomped);
+}
+
+void test_refs_read__trailing(void)
+{
+	git_reference *test, *trailing;
+
+	cl_git_pass(git_reference_lookup(&test, g_repo, "refs/heads/test"));
+	cl_git_pass(git_reference_lookup(&trailing, g_repo, "refs/heads/trailing"));
+	cl_git_pass(git_oid_cmp(git_reference_oid(test), git_reference_oid(trailing)));
+
+	git_reference_free(test);
+	git_reference_free(trailing);
+}
+
 void test_refs_read__unfound_return_ENOTFOUND(void)
 {
 	git_reference *reference;

tests-clar/resources/testrepo.git/refs/heads/chomped

@@ -0,0 +1 @@
+e90810b8df3e80c413d903f631643c716887138d

tests-clar/resources/testrepo.git/refs/heads/trailing

@@ -0,0 +1 @@
+e90810b8df3e80c413d903f631643c716887138d