From 6ab6829097ff3356a3f7f7d0850a593d24096bf7 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 9 Aug 2012 12:39:09 -0500 Subject: [PATCH 1/5] Parse ref oids without trailing newline --- src/refs.c | 9 ++++++--- tests-clar/network/remotelocal.c | 4 ++-- tests-clar/refs/branches/foreach.c | 4 ++-- tests-clar/refs/foreachglob.c | 6 +++--- tests-clar/refs/read.c | 12 ++++++++++++ tests-clar/resources/testrepo.git/refs/heads/chomped | 1 + 6 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 tests-clar/resources/testrepo.git/refs/heads/chomped diff --git a/src/refs.c b/src/refs.c index 0e0a491ec..2f1292b0b 100644 --- a/src/refs.c +++ b/src/refs.c @@ -173,8 +173,8 @@ static int loose_parse_oid(git_oid *oid, git_buf *file_content) buffer = (char *)file_content->ptr; - /* File format: 40 chars (OID) + newline */ - if (git_buf_len(file_content) < GIT_OID_HEXSZ + 1) + /* File format: 40 chars (OID) */ + if (git_buf_len(file_content) < GIT_OID_HEXSZ) goto corrupt; if (git_oid_fromstr(oid, buffer) < 0) @@ -184,7 +184,10 @@ static int loose_parse_oid(git_oid *oid, git_buf *file_content) if (*buffer == '\r') buffer++; - if (*buffer != '\n') + if (*buffer == '\n') + buffer++; + + if (*buffer != '\0') goto corrupt; return 0; diff --git a/tests-clar/network/remotelocal.c b/tests-clar/network/remotelocal.c index 16e3fe2dd..9c8ce359d 100644 --- a/tests-clar/network/remotelocal.c +++ b/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, 24); } 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, 24); git_remote_free(remote); /* Disconnect from the "spaced repo" before the cleanup */ remote = NULL; diff --git a/tests-clar/refs/branches/foreach.c b/tests-clar/refs/branches/foreach.c index 79c7e59e4..ca1393b2f 100644 --- a/tests-clar/refs/branches/foreach.c +++ b/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, 12); } 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, 10); } struct expectations { diff --git a/tests-clar/refs/foreachglob.c b/tests-clar/refs/foreachglob.c index 66827e525..ba58c20fe 100644 --- a/tests-clar/refs/foreachglob.c +++ b/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, 19); } 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, 10); } void test_refs_foreachglob__retrieve_partially_named_references(void) diff --git a/tests-clar/refs/read.c b/tests-clar/refs/read.c index 1948e0a56..395225be1 100644 --- a/tests-clar/refs/read.c +++ b/tests-clar/refs/read.c @@ -193,6 +193,18 @@ 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__unfound_return_ENOTFOUND(void) { git_reference *reference; diff --git a/tests-clar/resources/testrepo.git/refs/heads/chomped b/tests-clar/resources/testrepo.git/refs/heads/chomped new file mode 100644 index 000000000..0166a7f92 --- /dev/null +++ b/tests-clar/resources/testrepo.git/refs/heads/chomped @@ -0,0 +1 @@ +e90810b8df3e80c413d903f631643c716887138d \ No newline at end of file From 2fe293b6fb29ab2df6421cca15d29d4035850e7a Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 9 Aug 2012 11:36:21 -0700 Subject: [PATCH 2/5] trim whitespace when parsing loose refs --- src/refs.c | 49 +++++++++++++------------------------------------ 1 file changed, 13 insertions(+), 36 deletions(-) diff --git a/src/refs.c b/src/refs.c index 0e0a491ec..c602d1b18 100644 --- a/src/refs.c +++ b/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; + /* 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; - 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; - - 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); From e60af90498c5be3fc132901009f7d8fc8bb0087f Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 9 Aug 2012 14:39:43 -0500 Subject: [PATCH 3/5] Test trailing space after ref oid --- tests-clar/network/remotelocal.c | 4 ++-- tests-clar/refs/branches/foreach.c | 4 ++-- tests-clar/refs/foreachglob.c | 4 ++-- tests-clar/refs/read.c | 12 ++++++++++++ .../resources/testrepo.git/refs/heads/trailing | 1 + 5 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 tests-clar/resources/testrepo.git/refs/heads/trailing diff --git a/tests-clar/network/remotelocal.c b/tests-clar/network/remotelocal.c index 9c8ce359d..63016db5f 100644 --- a/tests-clar/network/remotelocal.c +++ b/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, 24); + 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, 24); + cl_assert_equal_i(how_many_refs, 25); git_remote_free(remote); /* Disconnect from the "spaced repo" before the cleanup */ remote = NULL; diff --git a/tests-clar/refs/branches/foreach.c b/tests-clar/refs/branches/foreach.c index ca1393b2f..aca11ecd9 100644 --- a/tests-clar/refs/branches/foreach.c +++ b/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, 12); + 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, 10); + assert_retrieval(GIT_BRANCH_LOCAL, 11); } struct expectations { diff --git a/tests-clar/refs/foreachglob.c b/tests-clar/refs/foreachglob.c index ba58c20fe..054846fe6 100644 --- a/tests-clar/refs/foreachglob.c +++ b/tests-clar/refs/foreachglob.c @@ -46,7 +46,7 @@ static void assert_retrieval(const char *glob, unsigned int flags, int expected_ void test_refs_foreachglob__retrieve_all_refs(void) { /* 8 heads (including one packed head) + 1 note + 2 remotes + 6 tags */ - assert_retrieval("*", GIT_REF_LISTALL, 19); + 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, 10); + assert_retrieval("refs/heads/*", GIT_REF_LISTALL, 11); } void test_refs_foreachglob__retrieve_partially_named_references(void) diff --git a/tests-clar/refs/read.c b/tests-clar/refs/read.c index 395225be1..f33658754 100644 --- a/tests-clar/refs/read.c +++ b/tests-clar/refs/read.c @@ -205,6 +205,18 @@ void test_refs_read__chomped(void) 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; diff --git a/tests-clar/resources/testrepo.git/refs/heads/trailing b/tests-clar/resources/testrepo.git/refs/heads/trailing new file mode 100644 index 000000000..2a4a6e62f --- /dev/null +++ b/tests-clar/resources/testrepo.git/refs/heads/trailing @@ -0,0 +1 @@ +e90810b8df3e80c413d903f631643c716887138d From 28e0068172942433ae304c9f965ee73588498f49 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 9 Aug 2012 14:39:56 -0500 Subject: [PATCH 4/5] Ignore ref oid terminator --- src/refs.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/refs.c b/src/refs.c index 2f1292b0b..270e7e8e6 100644 --- a/src/refs.c +++ b/src/refs.c @@ -180,16 +180,6 @@ static int loose_parse_oid(git_oid *oid, git_buf *file_content) if (git_oid_fromstr(oid, buffer) < 0) goto corrupt; - buffer = buffer + GIT_OID_HEXSZ; - if (*buffer == '\r') - buffer++; - - if (*buffer == '\n') - buffer++; - - if (*buffer != '\0') - goto corrupt; - return 0; corrupt: From 186c054d6556d6dfe8b9e17e82990603b4b33b5a Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 9 Aug 2012 14:47:29 -0500 Subject: [PATCH 5/5] Revert implementation changes --- src/refs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/refs.c b/src/refs.c index 270e7e8e6..0e0a491ec 100644 --- a/src/refs.c +++ b/src/refs.c @@ -173,13 +173,20 @@ static int loose_parse_oid(git_oid *oid, git_buf *file_content) buffer = (char *)file_content->ptr; - /* File format: 40 chars (OID) */ - if (git_buf_len(file_content) < GIT_OID_HEXSZ) + /* 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; + return 0; corrupt: