From 5b071115294e2c7d4b36ac21842ef09c7edd02b6 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 14:00:19 +0200 Subject: [PATCH 1/3] tests: add test commit with angle brackets in the author name --- tests-clar/network/remotelocal.c | 4 ++-- tests-clar/refs/branches/foreach.c | 4 ++-- tests-clar/refs/foreachglob.c | 4 ++-- .../1b/8cbad43e867676df601306689fe7c3def5e689 | Bin 0 -> 51 bytes .../25/8f0e2a959a364e40ed6603d5d44fbb24765b10 | Bin 0 -> 168 bytes .../resources/testrepo.git/refs/heads/haacked | 1 + tests-clar/revwalk/basic.c | 4 ++-- 7 files changed, 9 insertions(+), 8 deletions(-) create mode 100644 tests-clar/resources/testrepo.git/objects/1b/8cbad43e867676df601306689fe7c3def5e689 create mode 100644 tests-clar/resources/testrepo.git/objects/25/8f0e2a959a364e40ed6603d5d44fbb24765b10 create mode 100644 tests-clar/resources/testrepo.git/refs/heads/haacked diff --git a/tests-clar/network/remotelocal.c b/tests-clar/network/remotelocal.c index 41922975e..5e20b4240 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, 21); + cl_assert_equal_i(how_many_refs, 22); } 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, 21); + cl_assert_equal_i(how_many_refs, 22); 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 51e3381f8..b6e973799 100644 --- a/tests-clar/refs/branches/foreach.c +++ b/tests-clar/refs/branches/foreach.c @@ -48,7 +48,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, 9); + assert_retrieval(GIT_BRANCH_LOCAL | GIT_BRANCH_REMOTE, 10); } void test_refs_branches_foreach__retrieve_remote_branches(void) @@ -58,7 +58,7 @@ void test_refs_branches_foreach__retrieve_remote_branches(void) void test_refs_branches_foreach__retrieve_local_branches(void) { - assert_retrieval(GIT_BRANCH_LOCAL, 7); + assert_retrieval(GIT_BRANCH_LOCAL, 8); } struct expectations { diff --git a/tests-clar/refs/foreachglob.c b/tests-clar/refs/foreachglob.c index 8bbcd71ed..d1412a94b 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) { /* 7 heads (including one packed head) + 1 note + 2 remotes + 6 tags */ - assert_retrieval("*", GIT_REF_LISTALL, 16); + assert_retrieval("*", GIT_REF_LISTALL, 17); } 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, 7); + assert_retrieval("refs/heads/*", GIT_REF_LISTALL, 8); } void test_refs_foreachglob__retrieve_partially_named_references(void) diff --git a/tests-clar/resources/testrepo.git/objects/1b/8cbad43e867676df601306689fe7c3def5e689 b/tests-clar/resources/testrepo.git/objects/1b/8cbad43e867676df601306689fe7c3def5e689 new file mode 100644 index 0000000000000000000000000000000000000000..6048d4bad339c5b92a5a4a36002ce3985d5b0bf4 GIT binary patch literal 51 zcmblLAXeIVsSp^AMPkgZGeHH721AUD zWR#;MNoSX>)`Ir>DjW3A9Ucu_#|3Ug@y%&~K9_Rg56$buO)T>OEh_ZdIgN0Zt(4-h W$IZ%r2gH3D>qry)O5zK?(n*;`X-(Du literal 0 HcmV?d00001 diff --git a/tests-clar/resources/testrepo.git/refs/heads/haacked b/tests-clar/resources/testrepo.git/refs/heads/haacked new file mode 100644 index 000000000..17f591222 --- /dev/null +++ b/tests-clar/resources/testrepo.git/refs/heads/haacked @@ -0,0 +1 @@ +258f0e2a959a364e40ed6603d5d44fbb24765b10 diff --git a/tests-clar/revwalk/basic.c b/tests-clar/revwalk/basic.c index a5a9b2eda..6f3c1c06d 100644 --- a/tests-clar/revwalk/basic.c +++ b/tests-clar/revwalk/basic.c @@ -129,8 +129,8 @@ void test_revwalk_basic__glob_heads(void) i++; } - /* git log --branches --oneline | wc -l => 13 */ - cl_assert(i == 13); + /* git log --branches --oneline | wc -l => 14 */ + cl_assert(i == 14); } void test_revwalk_basic__push_head(void) From 118cf57d426ede29b6695204e707810bbe3888ef Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 4 Jul 2012 16:06:07 +0200 Subject: [PATCH 2/3] revwalk: relax the parsing of the commit time --- src/revwalk.c | 31 ++++++++++++++----- tests-clar/revwalk/signatureparsing.c | 44 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 tests-clar/revwalk/signatureparsing.c diff --git a/src/revwalk.c b/src/revwalk.c index 7bcdc4af8..9dff283f5 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -188,7 +188,7 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo const size_t parent_len = strlen("parent ") + GIT_OID_HEXSZ + 1; unsigned char *buffer = raw->data; unsigned char *buffer_end = buffer + raw->len; - unsigned char *parents_start; + unsigned char *parents_start, *committer_start; int i, parents = 0; int commit_time; @@ -219,17 +219,34 @@ static int commit_quick_parse(git_revwalk *walk, commit_object *commit, git_rawo commit->out_degree = (unsigned short)parents; + if ((committer_start = buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) + return commit_error(commit, "object is corrupted"); + + buffer++; + if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL) return commit_error(commit, "object is corrupted"); - if ((buffer = memchr(buffer, '<', buffer_end - buffer)) == NULL || - (buffer = memchr(buffer, '>', buffer_end - buffer)) == NULL) - return commit_error(commit, "malformed author information"); + /* Skip trailing spaces */ + while (buffer > committer_start && git__isspace(*buffer)) + buffer--; - while (*buffer == '>' || git__isspace(*buffer)) - buffer++; + /* Seek for the begining of the pack of digits */ + while (buffer > committer_start && git__isdigit(*buffer)) + buffer--; - if (git__strtol32(&commit_time, (char *)buffer, NULL, 10) < 0) + /* Skip potential timezone offset */ + if ((buffer > committer_start) && (*buffer == '+' || *buffer == '-')) { + buffer--; + + while (buffer > committer_start && git__isspace(*buffer)) + buffer--; + + while (buffer > committer_start && git__isdigit(*buffer)) + buffer--; + } + + if ((buffer == committer_start) || (git__strtol32(&commit_time, (char *)(buffer + 1), NULL, 10) < 0)) return commit_error(commit, "cannot parse commit time"); commit->time = (time_t)commit_time; diff --git a/tests-clar/revwalk/signatureparsing.c b/tests-clar/revwalk/signatureparsing.c new file mode 100644 index 000000000..94de1a343 --- /dev/null +++ b/tests-clar/revwalk/signatureparsing.c @@ -0,0 +1,44 @@ +#include "clar_libgit2.h" + +static git_repository *_repo; +static git_revwalk *_walk; + +void test_revwalk_signatureparsing__initialize(void) +{ + cl_git_pass(git_repository_open(&_repo, cl_fixture("testrepo.git"))); + cl_git_pass(git_revwalk_new(&_walk, _repo)); +} + +void test_revwalk_signatureparsing__cleanup(void) +{ + git_revwalk_free(_walk); + git_repository_free(_repo); +} + +void test_revwalk_signatureparsing__do_not_choke_when_name_contains_angle_brackets(void) +{ + git_reference *ref; + git_oid commit_oid; + git_commit *commit; + const git_signature *signature; + + /* + * The branch below points at a commit with angle brackets in the committer/author name + * committer 1323847743 +0100 + */ + cl_git_pass(git_reference_lookup(&ref, _repo, "refs/heads/haacked")); + + git_revwalk_push(_walk, git_reference_oid(ref)); + cl_git_pass(git_revwalk_next(&commit_oid, _walk)); + + cl_git_pass(git_commit_lookup(&commit, _repo, git_reference_oid(ref))); + + signature = git_commit_committer(commit); + cl_assert_equal_s("Yu V. Bin Haacked", signature->email); + cl_assert_equal_s("", signature->name); + cl_assert_equal_i(1323847743, (int)signature->when.time); + cl_assert_equal_i(60, signature->when.offset); + + git_commit_free(commit); + git_reference_free(ref); +} From 8aedf1d5581f518da286ca4a33d6f7a98db38651 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Thu, 5 Jul 2012 17:02:03 +0200 Subject: [PATCH 3/3] signature: prevent angle bracket usage in identity --- include/git2/signature.h | 3 +++ src/signature.c | 24 +++++++++++++---- tests-clar/commit/signature.c | 50 ++++++++++++++++++++++------------- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/include/git2/signature.h b/include/git2/signature.h index cbf94269f..cdbe67879 100644 --- a/include/git2/signature.h +++ b/include/git2/signature.h @@ -23,6 +23,9 @@ GIT_BEGIN_DECL * Create a new action signature. The signature must be freed * manually or using git_signature_free * + * Note: angle brackets ('<' and '>') characters are not allowed + * to be used in either the `name` or the `email` parameter. + * * @param sig_out new signature, in case of error NULL * @param name name of the person * @param email email of the person diff --git a/src/signature.c b/src/signature.c index 332bdf65f..1f788356b 100644 --- a/src/signature.c +++ b/src/signature.c @@ -40,7 +40,7 @@ static const char *skip_trailing_spaces(const char *buffer_start, const char *bu static int signature_error(const char *msg) { - giterr_set(GITERR_INVALID, "Failed to parse signature - %s", msg); + giterr_set(GITERR_INVALID, "Failed to process signature - %s", msg); return -1; } @@ -72,9 +72,16 @@ static int process_trimming(const char *input, char **storage, const char *input return 0; } +static bool contains_angle_brackets(const char *input) +{ + if (strchr(input, '<') != NULL) + return true; + + return strchr(input, '>') != NULL; +} + int git_signature_new(git_signature **sig_out, const char *name, const char *email, git_time_t time, int offset) { - int error; git_signature *p = NULL; assert(name && email); @@ -84,11 +91,18 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema p = git__calloc(1, sizeof(git_signature)); GITERR_CHECK_ALLOC(p); - if ((error = process_trimming(name, &p->name, name + strlen(name), 1)) < 0 || - (error = process_trimming(email, &p->email, email + strlen(email), 1)) < 0) + if (process_trimming(name, &p->name, name + strlen(name), 1) < 0 || + process_trimming(email, &p->email, email + strlen(email), 1) < 0) { git_signature_free(p); - return error; + return -1; + } + + if (contains_angle_brackets(p->email) || + contains_angle_brackets(p->name)) + { + git_signature_free(p); + return signature_error("Neither `name` nor `email` should contain angle brackets chars."); } p->when.time = time; diff --git a/tests-clar/commit/signature.c b/tests-clar/commit/signature.c index 290b11fa3..9364efb10 100644 --- a/tests-clar/commit/signature.c +++ b/tests-clar/commit/signature.c @@ -13,17 +13,39 @@ static int try_build_signature(const char *name, const char *email, git_time_t t return error; } - -void test_commit_signature__create_trim(void) +static void assert_name_and_email( + const char *expected_name, + const char *expected_email, + const char *name, + const char *email) { - // creating a signature trims leading and trailing spaces - git_signature *sign; - cl_git_pass(git_signature_new(&sign, " nulltoken ", " emeric.fermas@gmail.com ", 1234567890, 60)); - cl_assert(strcmp(sign->name, "nulltoken") == 0); - cl_assert(strcmp(sign->email, "emeric.fermas@gmail.com") == 0); - git_signature_free((git_signature *)sign); + git_signature *sign; + + cl_git_pass(git_signature_new(&sign, name, email, 1234567890, 60)); + cl_assert_equal_s(expected_name, sign->name); + cl_assert_equal_s(expected_email, sign->email); + + git_signature_free(sign); } +void test_commit_signature__leading_and_trailing_spaces_are_trimmed(void) +{ + assert_name_and_email("nulltoken", "emeric.fermas@gmail.com", " nulltoken ", " emeric.fermas@gmail.com "); +} + +void test_commit_signature__angle_brackets_in_names_are_not_supported(void) +{ + cl_git_fail(try_build_signature("Haack", "phil@haack", 1234567890, 60)); + cl_git_fail(try_build_signature("", "phil@haack", 1234567890, 60)); +} + +void test_commit_signature__angle_brackets_in_email_are_not_supported(void) +{ + cl_git_fail(try_build_signature("Phil Haack", ">phil@haack", 1234567890, 60)); + cl_git_fail(try_build_signature("Phil Haack", "phil@>haack", 1234567890, 60)); + cl_git_fail(try_build_signature("Phil Haack", "", 1234567890, 60)); +} void test_commit_signature__create_empties(void) { @@ -39,21 +61,13 @@ void test_commit_signature__create_empties(void) void test_commit_signature__create_one_char(void) { // creating a one character signature - git_signature *sign; - cl_git_pass(git_signature_new(&sign, "x", "foo@bar.baz", 1234567890, 60)); - cl_assert(strcmp(sign->name, "x") == 0); - cl_assert(strcmp(sign->email, "foo@bar.baz") == 0); - git_signature_free((git_signature *)sign); + assert_name_and_email("x", "foo@bar.baz", "x", "foo@bar.baz"); } void test_commit_signature__create_two_char(void) { // creating a two character signature - git_signature *sign; - cl_git_pass(git_signature_new(&sign, "xx", "x@y.z", 1234567890, 60)); - cl_assert(strcmp(sign->name, "xx") == 0); - cl_assert(strcmp(sign->email, "x@y.z") == 0); - git_signature_free((git_signature *)sign); + assert_name_and_email("xx", "foo@bar.baz", "xx", "foo@bar.baz"); } void test_commit_signature__create_zero_char(void)