Merge pull request #805 from nulltoken/fix/revwalk-email-parsing Fix revwalk email parsing
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342
diff --git a/include/git2/signature.h b/include/git2/signature.h
index cbf9426..cdbe678 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/revwalk.c b/src/revwalk.c
index 7bcdc4a..9dff283 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--;
+
+ /* Seek for the begining of the pack of digits */
+ while (buffer > committer_start && git__isdigit(*buffer))
+ buffer--;
- while (*buffer == '>' || git__isspace(*buffer))
- buffer++;
+ /* 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 (git__strtol32(&commit_time, (char *)buffer, NULL, 10) < 0)
+ 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/src/signature.c b/src/signature.c
index 332bdf6..1f78835 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 290b11f..9364efb 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;
}
+static void assert_name_and_email(
+ const char *expected_name,
+ const char *expected_email,
+ const char *name,
+ const char *email)
+{
+ 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__create_trim(void)
+void test_commit_signature__leading_and_trailing_spaces_are_trimmed(void)
{
- // 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);
+ 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("<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>", "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", "<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)
diff --git a/tests-clar/network/remotelocal.c b/tests-clar/network/remotelocal.c
index 4192297..5e20b42 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 51e3381..b6e9737 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 8bbcd71..d1412a9 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 0000000..6048d4b
Binary files /dev/null and b/tests-clar/resources/testrepo.git/objects/1b/8cbad43e867676df601306689fe7c3def5e689 differ
diff --git a/tests-clar/resources/testrepo.git/objects/25/8f0e2a959a364e40ed6603d5d44fbb24765b10 b/tests-clar/resources/testrepo.git/objects/25/8f0e2a959a364e40ed6603d5d44fbb24765b10
new file mode 100644
index 0000000..cb1ed57
Binary files /dev/null and b/tests-clar/resources/testrepo.git/objects/25/8f0e2a959a364e40ed6603d5d44fbb24765b10 differ
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 0000000..17f5912
--- /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 a5a9b2e..6f3c1c0 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)
diff --git a/tests-clar/revwalk/signatureparsing.c b/tests-clar/revwalk/signatureparsing.c
new file mode 100644
index 0000000..94de1a3
--- /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 <Yu V. Bin Haacked> <foo@example.com> 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);
+}