From 04f788023fd04e60a12c2a19787857c72b3817f4 Mon Sep 17 00:00:00 2001 From: Brodie Rao Date: Tue, 9 Aug 2011 20:49:12 -0700 Subject: [PATCH 1/4] commit: properly parse empty commit messages This ensures commit->message is always non-NULL, even if the commit message is empty or consists of only a newline. One such commit can be found in the wild in the jQuery repository: https://github.com/jquery/jquery/commit/25b424134f9927a5bf0bab5cba836a0aa6c3cfc1 --- src/commit.c | 4 ++-- .../36/97d64be941a53d4ae8f6a271e4e3fa56b022cc | Bin 0 -> 23 bytes .../94/4c0f6e4dfa41595e6eb3ceecdb14f50fe18162 | Bin 0 -> 119 bytes .../a6/5fedf39aefe402d3bb6e24df4d4f5fe4547750 | Bin 0 -> 150 bytes tests/resources/testrepo.git/refs/heads/master | Bin 41 -> 41 bytes tests/t04-commit.c | 2 ++ tests/t10-refs.c | 2 +- 7 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 tests/resources/testrepo.git/objects/36/97d64be941a53d4ae8f6a271e4e3fa56b022cc create mode 100644 tests/resources/testrepo.git/objects/94/4c0f6e4dfa41595e6eb3ceecdb14f50fe18162 create mode 100644 tests/resources/testrepo.git/objects/a6/5fedf39aefe402d3bb6e24df4d4f5fe4547750 diff --git a/src/commit.c b/src/commit.c index 0ee3854c4..ced457ecc 100644 --- a/src/commit.c +++ b/src/commit.c @@ -221,10 +221,10 @@ int git_commit__parse_buffer(git_commit *commit, const void *data, size_t len) } /* parse commit message */ - while (buffer < buffer_end && *buffer == '\n') + while (buffer < buffer_end - 1 && *buffer == '\n') buffer++; - if (buffer < buffer_end) { + if (buffer <= buffer_end) { commit->message = git__strndup(buffer, buffer_end - buffer); if (!commit->message) return GIT_ENOMEM; diff --git a/tests/resources/testrepo.git/objects/36/97d64be941a53d4ae8f6a271e4e3fa56b022cc b/tests/resources/testrepo.git/objects/36/97d64be941a53d4ae8f6a271e4e3fa56b022cc new file mode 100644 index 0000000000000000000000000000000000000000..9bb5b623bdbc11a70db482867b5b26d0d7b3215c GIT binary patch literal 23 fcmb7G-5C`FfcPQQ3!H%bn$g%SfOmF@NI2De~WFK%%kl}eMcVO zI|fyeRFs&PoDrXvnUktlQc=QSHvO9SOUI?QUN62aDtz+zSJ(!nGf<^@spViL%SGD` Z-hZ)NCTD++(0m7PL-bAdvgQ4=D9j?f9l*|80Bjm_@g(h>T5Jb3V=xAqv| z9qz`e3Ykc3jY?Bxav=weT2NGFDvoNfRo&4=Z(h9WGN34ih^$ys4#gM3fJH=+gHJqJ zrkZLbGW;2HU*RTw47koLeSyp9!Onu(!!tv)S!%=s&G7U;E`l EFjbjGl>h($ literal 0 HcmV?d00001 diff --git a/tests/resources/testrepo.git/refs/heads/master b/tests/resources/testrepo.git/refs/heads/master index 9536ad89cfb3b156e2ad6bafe06c6a708d7d2b73..3d8f0a402b492c248b12ac72b6a66eebe941023f 100644 GIT binary patch literal 41 vcmYc^GfhiPNi()gOifEQF)&IoPD(OMH8M#_Gf6Q?1In40nwXoL8gKys2{Q}F literal 41 ucmV~$!4Uu;2m`Rc(|EMVIZi>?e*}|k_<email, "schacon@gmail.com") == 0); must_be_true(strcmp(committer->name, "Scott Chacon") == 0); must_be_true(strcmp(committer->email, "schacon@gmail.com") == 0); + must_be_true(message != NULL); must_be_true(strchr(message, '\n') != NULL); must_be_true(commit_time > 0); must_be_true(parents <= 2); diff --git a/tests/t10-refs.c b/tests/t10-refs.c index 67034155d..1b3e0a3aa 100644 --- a/tests/t10-refs.c +++ b/tests/t10-refs.c @@ -70,7 +70,7 @@ END_TEST static const char *head_tracker_sym_ref_name = "head-tracker"; static const char *current_head_target = "refs/heads/master"; -static const char *current_master_tip = "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"; +static const char *current_master_tip = "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"; BEGIN_TEST(readsym0, "lookup a symbolic reference") git_repository *repo; From cf7b13f3c30595e3fe88103f9bfb653655a41177 Mon Sep 17 00:00:00 2001 From: Brodie Rao Date: Thu, 11 Aug 2011 14:05:55 -0700 Subject: [PATCH 2/4] tag: avoid a double-free when parsing tags without a tagger field The v0.99 tag in the Git repo triggers this behavior: http://git.kernel.org/?p=git/git.git;a=tag;h=d6602ec5194c87b0fc87103ca4d67251c76f233a Ideally, we'd allow the tag to be instantiated even though the tagger field is missing, but this at the very least prevents libgit2 from crashing. To test this bug, a new repository has been added based on the test branch in testrepo.git. It contains a "e90810b" tag that looks like this: object e90810b8df3e80c413d903f631643c716887138d type commit tag e90810b This is a very simple tag. --- src/tag.c | 5 +---- tests/resources/bad_tag.git/HEAD | Bin 0 -> 23 bytes tests/resources/bad_tag.git/config | Bin 0 -> 119 bytes ...28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx | Bin 0 -> 1268 bytes ...8f4e000a17f49a41d7a79fc2f762a8a7d9164.pack | Bin 0 -> 596 bytes tests/resources/bad_tag.git/packed-refs | Bin 0 -> 127 bytes tests/t08-tag.c | 18 ++++++++++++++++++ 7 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 tests/resources/bad_tag.git/HEAD create mode 100644 tests/resources/bad_tag.git/config create mode 100644 tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx create mode 100644 tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack create mode 100644 tests/resources/bad_tag.git/packed-refs diff --git a/src/tag.c b/src/tag.c index fc9f8b5e4..08ebb71cb 100644 --- a/src/tag.c +++ b/src/tag.c @@ -126,11 +126,8 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer if (tag->tagger == NULL) return GIT_ENOMEM; - if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) != 0) { - free(tag->tag_name); - git_signature_free(tag->tagger); + if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) != 0) return git__rethrow(error, "Failed to parse tag"); - } if( *buffer != '\n' ) return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. No new line before message"); diff --git a/tests/resources/bad_tag.git/HEAD b/tests/resources/bad_tag.git/HEAD new file mode 100644 index 0000000000000000000000000000000000000000..cb089cd89a7d7686d284d8761201649346b5aa1c GIT binary patch literal 23 ecmXR)O|w!cN=+-)&qz&7Db~+TEG|hc;sO9;xClW2 literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/config b/tests/resources/bad_tag.git/config new file mode 100644 index 0000000000000000000000000000000000000000..2f8958058adf2a043db52a721e66a626658814a1 GIT binary patch literal 119 zcmaz}&M!)h?o#qRO=VqTIxivecsD%=|nBTLlBSnzYQE)ZF}(RG?r< gQE4h%JSnjVLo6phJuxSzC^fCLASJORwHT%c08(KibN~PV literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx b/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx new file mode 100644 index 0000000000000000000000000000000000000000..c404aa15bb04cf5b4fbfc8bb4d9638cd5d420643 GIT binary patch literal 1268 zcmexg;-AdGz`z8=Fu(|8jAF{d02H2-U}m6xVlWF(-6*Ck3|N6-I8-nj(5=Xr9muB& zW=a#Y@_f{Ax{fgShl8jc9xWd3HDVpEvWF5LTe<(DX}8CMQgab9b@(VF?oZ>4zX8h_h;G0T14 z|BH0+oRA_vW%fS@wRi2Ms}zD$t^>2>HXv>VW-mpc-tVk0A!ABL;wH) literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack b/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack new file mode 100644 index 0000000000000000000000000000000000000000..90eac50322033985e5623ba611255e53895e2d58 GIT binary patch literal 596 zcmWG=boORoU|<4b_6d9ybLO6LB2ZL-fLMnU}tes`9ibm`tURpR~n zN4$6HD)yU)CWLQZoTPkI!7rVgzg+9v``Z_ETFHpbhKWQbKFLPm4J!}4hZ-;;1 za#iqhJ1^nhV_bNmPxbulr)SmY-soaD~5%da!2C_U9Y#XivXB zA!=`fYW_quaW@M^#aa1lT|@sZc&3)$dY@sb~&ytJ0pw-~IMyXRO#PXcU)}Br5Fo$U@q0+Q~i{wM|OG8(lK~ z2Jxh8?sP1EZO^0=z3#`;4;ObQpM7d2VsCVSdxM_%Er7piGjqipZckmklV`lcJwgzA9)3El zAFkma>fz(b*d@-qv^Yq#$mnyBs8LXN&`ySH`hq*KTjgPveR5ybqU$e~U%h;5+LmYO uLDBtE3=FJx{CBo8;;@Q|AvB%6%_2xXV*ZRJb+SL>o2Gb(o_c6~o*4lB%?J|! literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/packed-refs b/tests/resources/bad_tag.git/packed-refs new file mode 100644 index 0000000000000000000000000000000000000000..f9fd2fd4ad59d2a735fe4fada3c4b584ae010b30 GIT binary patch literal 127 zcmXZTK?(vf3;@7;UlH&uo0zmse2fsAR7DSqyWsal@HUq@!0O|9eCOQY^VsiaSTkQ4 zP%_-6R6n_C$e`|M(Ud~9Hk&T#M!i<}-DUUNwxppFUd!bVjmfDvgg6X&Hl`*#IyKc! GtWmyyrz5=p literal 0 HcmV?d00001 diff --git a/tests/t08-tag.c b/tests/t08-tag.c index b0d4af87b..079a9e0a8 100644 --- a/tests/t08-tag.c +++ b/tests/t08-tag.c @@ -30,6 +30,7 @@ static const char *tag1_id = "b25fa35b38051e4ae45d4222e795f9df2e43f1d1"; static const char *tag2_id = "7b4384978d2493e851f9cca7858815fac9b10980"; static const char *tagged_commit = "e90810b8df3e80c413d903f631643c716887138d"; +static const char *bad_tag_id = "eda9f45a2a98d4c17a09d681d88569fa4ea91755"; BEGIN_TEST(read0, "read and parse a tag from the repository") git_repository *repo; @@ -106,6 +107,22 @@ BEGIN_TEST(read2, "list all tag names from the repository matching a specified p git_repository_free(repo); END_TEST +#define BAD_TAG_REPOSITORY_FOLDER TEST_RESOURCES "/bad_tag.git/" + +BEGIN_TEST(read3, "read and parse a tag without a tagger field") + git_repository *repo; + git_tag *bad_tag; + git_oid id; + + must_pass(git_repository_open(&repo, BAD_TAG_REPOSITORY_FOLDER)); + + git_oid_fromstr(&id, bad_tag_id); + + must_fail(git_tag_lookup(&bad_tag, repo, &id)); + + git_repository_free(repo); +END_TEST + #define TAGGER_NAME "Vicent Marti" #define TAGGER_EMAIL "vicent@github.com" @@ -304,6 +321,7 @@ BEGIN_SUITE(tag) ADD_TEST(read0); ADD_TEST(read1); ADD_TEST(read2); + ADD_TEST(read3); ADD_TEST(write0); ADD_TEST(write2); From 15b0bed2ba170526969348170d6f5e0d2ee16d7b Mon Sep 17 00:00:00 2001 From: Brodie Rao Date: Thu, 11 Aug 2011 16:12:29 -0700 Subject: [PATCH 3/4] tag: allow the tagger field to be missing when parsing tags Instead of bailing out with an error, this sets tagger to NULL when the field is missing from the object. This makes it possible to inspect tags like this one: http://git.kernel.org/?p=git/git.git;a=tag;h=f25a265a342aed6041ab0cc484224d9ca54b6f41 --- src/tag.c | 14 +++++++++----- tests/t08-tag.c | 20 ++++++++++++++++++-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/tag.c b/src/tag.c index 08ebb71cb..ba75104ef 100644 --- a/src/tag.c +++ b/src/tag.c @@ -122,12 +122,16 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer buffer = search + 1; - tag->tagger = git__malloc(sizeof(git_signature)); - if (tag->tagger == NULL) - return GIT_ENOMEM; + tag->tagger = NULL; + if (*buffer != '\n') { + tag->tagger = git__malloc(sizeof(git_signature)); + if (tag->tagger == NULL) + return GIT_ENOMEM; - if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) != 0) - return git__rethrow(error, "Failed to parse tag"); + if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n') != 0)) { + return git__rethrow(error, "Failed to parse tag"); + } + } if( *buffer != '\n' ) return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. No new line before message"); diff --git a/tests/t08-tag.c b/tests/t08-tag.c index 079a9e0a8..216fb9dfc 100644 --- a/tests/t08-tag.c +++ b/tests/t08-tag.c @@ -31,6 +31,7 @@ static const char *tag1_id = "b25fa35b38051e4ae45d4222e795f9df2e43f1d1"; static const char *tag2_id = "7b4384978d2493e851f9cca7858815fac9b10980"; static const char *tagged_commit = "e90810b8df3e80c413d903f631643c716887138d"; static const char *bad_tag_id = "eda9f45a2a98d4c17a09d681d88569fa4ea91755"; +static const char *badly_tagged_commit = "e90810b8df3e80c413d903f631643c716887138d"; BEGIN_TEST(read0, "read and parse a tag from the repository") git_repository *repo; @@ -112,13 +113,28 @@ END_TEST BEGIN_TEST(read3, "read and parse a tag without a tagger field") git_repository *repo; git_tag *bad_tag; - git_oid id; + git_commit *commit; + git_oid id, id_commit; must_pass(git_repository_open(&repo, BAD_TAG_REPOSITORY_FOLDER)); git_oid_fromstr(&id, bad_tag_id); + git_oid_fromstr(&id_commit, badly_tagged_commit); - must_fail(git_tag_lookup(&bad_tag, repo, &id)); + must_pass(git_tag_lookup(&bad_tag, repo, &id)); + must_be_true(bad_tag != NULL); + + must_be_true(strcmp(git_tag_name(bad_tag), "e90810b") == 0); + must_be_true(git_oid_cmp(&id, git_tag_id(bad_tag)) == 0); + must_be_true(bad_tag->tagger == NULL); + + must_pass(git_tag_target((git_object **)&commit, bad_tag)); + must_be_true(commit != NULL); + + must_be_true(git_oid_cmp(&id_commit, git_commit_id(commit)) == 0); + + git_tag_close(bad_tag); + git_commit_close(commit); git_repository_free(repo); END_TEST From 6f2856f308918455563413d012e4c4958e57ab40 Mon Sep 17 00:00:00 2001 From: Brodie Rao Date: Wed, 5 Oct 2011 15:17:37 -0700 Subject: [PATCH 4/4] signature: don't blow up trying to parse names containing '>' When trying to find the end of an email, instead of starting at the beginning of the signature, we start at the end of the name (after the first '<'). This brings libgit2 more in line with Git's behavior when reading out existing signatures. However, note that Git does not allow names like these through the usual porcelain; instead, it silently strips any '>' characters it sees. --- src/signature.c | 2 +- tests/t04-commit.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/signature.c b/src/signature.c index ebb56d7ab..388e8d9c9 100644 --- a/src/signature.c +++ b/src/signature.c @@ -279,7 +279,7 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, if ((name_end = strchr(buffer, '<')) == NULL) return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Cannot find '<' in signature"); - if ((email_end = strchr(buffer, '>')) == NULL) + if ((email_end = strchr(name_end, '>')) == NULL) return git__throw(GIT_EOBJCORRUPTED, "Failed to parse signature. Cannot find '>' in signature"); if (email_end < name_end) diff --git a/tests/t04-commit.c b/tests/t04-commit.c index 58d24bf73..3fb4d370c 100644 --- a/tests/t04-commit.c +++ b/tests/t04-commit.c @@ -412,6 +412,14 @@ BEGIN_TEST(parse1, "parse the signature line in a commit") 1234567890, 0); + TEST_SIGNATURE_PASS( + "author A U Thor> and others 1234567890\n", + "author ", + "A U Thor>", + "author@example.com", + 1234567890, + 0); + TEST_SIGNATURE_FAIL( "committer Vicent Marti tanoku@gmail.com> 123456 -0100 \n", "committer ");