diff options
| -rw-r--r-- | include/git2/sys/warning.h | 11 | ||||
| -rw-r--r-- | src/signature.c | 39 | ||||
| -rw-r--r-- | src/warning.c | 16 | ||||
| -rw-r--r-- | src/warning.h | 13 | ||||
| -rw-r--r-- | tests/commit/parse.c | 107 |
5 files changed, 135 insertions, 51 deletions
diff --git a/include/git2/sys/warning.h b/include/git2/sys/warning.h index ec1262a6b..02adfbe40 100644 --- a/include/git2/sys/warning.h +++ b/include/git2/sys/warning.h @@ -11,8 +11,10 @@ GIT_BEGIN_DECL typedef enum { GIT_WARNING_NONE = 0, - GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP, - GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE, + GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP, /* default continue */ + GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE, /* default continue */ + GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_MISSING, /* default error */ + GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_UNTERMINATED, /* error */ } git_warning_t; /** @@ -42,10 +44,13 @@ typedef struct { * It will be passed a warning structure describing the problem. * * @param warning A git_warning structure for the specific situation + * @param default_rval Default return code (i.e. <0 if warning defaults + * to error, 0 if defaults to no error) * @param payload The payload set when callback function was specified * @return 0 to continue, <0 to convert the warning to an error */ -typedef int (*git_warning_callback)(git_warning *warning, void *payload); +typedef int (*git_warning_callback)( + git_warning *warning, int default_rval, void *payload); /** * Set the callback to be invoked when an invalid but recoverable diff --git a/src/signature.c b/src/signature.c index a513b0e65..a57d9c4ac 100644 --- a/src/signature.c +++ b/src/signature.c @@ -177,17 +177,38 @@ int git_signature__parse( } email_start = git__memrchr(buffer, '<', buffer_end - buffer); - email_end = git__memrchr(buffer, '>', buffer_end - buffer); - - if (!email_start || !email_end || email_end <= email_start) - return signature_error("malformed e-mail"); + if (!email_start) { + if (git_warn_invalid_data( + GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_MISSING, -1, + buffer, (int)(buffer_end - buffer), + "missing signature %semail", header) < 0) + return signature_error("missing e-mail"); + + /* just stop now with everything as name */ + sig->name = extract_trimmed(buffer, buffer_end - buffer); + sig->email = git__strdup(""); + *buffer_out = buffer_end + 1; + return 0; + } + sig->name = extract_trimmed(buffer, email_start - buffer); email_start += 1; - sig->name = extract_trimmed(buffer, email_start - buffer - 1); - sig->email = extract_trimmed(email_start, email_end - email_start); + + email_end = git__memrchr(email_start, '>', buffer_end - email_start); + if (!email_end) { + if (git_warn_invalid_data( + GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_UNTERMINATED, -1, + email_start, (int)(buffer_end - email_start), + "unterminated signature %semail", header) < 0) + return signature_error("malformed e-mail"); + + sig->email = extract_trimmed(email_start, buffer_end - email_start); + } else { + sig->email = extract_trimmed(email_start, email_end - email_start); + } /* Do we even have a time at the end of the signature? */ - if (email_end + 2 < buffer_end) { + if (email_end != NULL && email_end + 2 < buffer_end) { const char *time_start = email_end + 2; const char *time_end; @@ -200,7 +221,7 @@ int git_signature__parse( /* warn (and return error if requested) */ if (git_warn_invalid_data( - GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP, + GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP, 0, time_start, (int)(time_end - time_start), "invalid signature %stimestamp", header) < 0) return signature_error("invalid Unix timestamp"); @@ -224,7 +245,7 @@ int git_signature__parse( /* warn (and return error if requested) */ if (git_warn_invalid_data( - GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE, + GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE, 0, tz_start, (int)(tz_end - tz_start), "invalid timezone in signature %s", header) < 0) return signature_error("invalid timezone"); diff --git a/src/warning.c b/src/warning.c index 1066c2c8d..b0bc7c03d 100644 --- a/src/warning.c +++ b/src/warning.c @@ -19,18 +19,18 @@ void git_warning_set_callback(git_warning_callback cb, void *payload) } static int git_warning__send( - git_warning *warning, const char *fmt, va_list ap) + git_warning *warning, int default_rval, const char *fmt, va_list ap) { int error = 0; git_buf buf = GIT_BUF_INIT; git_warning_callback cb = _warning_cb; if (!cb) - return 0; + return default_rval; if (!(error = git_buf_vprintf(&buf, fmt, ap))) { warning->message = git_buf_cstr(&buf); - error = cb(warning, _warning_payload); + error = cb(warning, default_rval, _warning_payload); } git_buf_free(&buf); @@ -40,6 +40,7 @@ static int git_warning__send( int git_warn( git_warning_t type, + int default_rval, const char *fmt, ...) { @@ -48,12 +49,12 @@ int git_warn( git_warning warning; if (!_warning_cb) - return 0; + return default_rval; warning.type = type; va_start(ap, fmt); - error = git_warning__send(&warning, fmt, ap); + error = git_warning__send(&warning, default_rval, fmt, ap); va_end(ap); return error; @@ -61,6 +62,7 @@ int git_warn( int git_warn_invalid_data( git_warning_t type, + int default_rval, const char *data, int datalen, const char *fmt, @@ -71,7 +73,7 @@ int git_warn_invalid_data( git_warning_invalid_data warning; if (!_warning_cb) - return 0; + return default_rval; warning.base.type = type; warning.invalid_data = git__strndup(data, datalen); @@ -79,7 +81,7 @@ int git_warn_invalid_data( warning.invalid_data_len = datalen; va_start(ap, fmt); - error = git_warning__send((git_warning *)&warning, fmt, ap); + error = git_warning__send((git_warning *)&warning, default_rval, fmt, ap); va_end(ap); git__free((char *)warning.invalid_data); diff --git a/src/warning.h b/src/warning.h index 6d25783f4..8c14e2869 100644 --- a/src/warning.h +++ b/src/warning.h @@ -10,13 +10,26 @@ #include "common.h" #include "git2/sys/warning.h" +/** + * Use this to raise a warning + * + * @param warning A git_warning_t code from include/git2/sys/warning.h + * @param default_rval Default return value (i.e. error code or zero) + * @param fmt Printf-style format string for warning message + * @return 0 to continue, less than 0 to raise error + */ int git_warn( git_warning_t warning, + int default_rval, const char *fmt, ...); +/** + * Raise a warning about invalid data, via a git_warning_invalid_data struct + */ int git_warn_invalid_data( git_warning_t warning, + int default_rval, const char *data, int datalen, const char *fmt, diff --git a/tests/commit/parse.c b/tests/commit/parse.c index 07f304ce3..ff4464845 100644 --- a/tests/commit/parse.c +++ b/tests/commit/parse.c @@ -3,7 +3,7 @@ #include "commit.h" #include "signature.h" -// Fixture setup +/* Fixture setup */ static git_repository *g_repo; void test_commit_parse__initialize(void) { @@ -15,7 +15,7 @@ void test_commit_parse__cleanup(void) } -// Header parsing +/* Header parsing */ typedef struct { const char *line; const char *header; @@ -70,7 +70,7 @@ void test_commit_parse__header(void) } -// Signature parsing +/* Signature parsing */ typedef struct { const char *string; const char *header; @@ -89,27 +89,27 @@ passing_signature_test_case passing_signature_cases[] = { {"committer Vicent Marti <tanoku@gmail.com> 123456 +0000 \n", "committer ", "Vicent Marti", "tanoku@gmail.com", 123456, 0}, {"committer Vicent Marti <tanoku@gmail.com> 123456 +0100 \n", "committer ", "Vicent Marti", "tanoku@gmail.com", 123456, 60}, {"committer Vicent Marti <tanoku@gmail.com> 123456 -0100 \n", "committer ", "Vicent Marti", "tanoku@gmail.com", 123456, -60}, - // Parse a signature without an author field + /* Parse a signature without an author field */ {"committer <tanoku@gmail.com> 123456 -0100 \n", "committer ", "", "tanoku@gmail.com", 123456, -60}, - // Parse a signature without an author field + /* Parse a signature without an author field */ {"committer <tanoku@gmail.com> 123456 -0100 \n", "committer ", "", "tanoku@gmail.com", 123456, -60}, - // Parse a signature with an empty author field + /* Parse a signature with an empty author field */ {"committer <tanoku@gmail.com> 123456 -0100 \n", "committer ", "", "tanoku@gmail.com", 123456, -60}, - // Parse a signature with an empty email field + /* Parse a signature with an empty email field */ {"committer Vicent Marti <> 123456 -0100 \n", "committer ", "Vicent Marti", "", 123456, -60}, - // Parse a signature with an empty email field + /* Parse a signature with an empty email field */ {"committer Vicent Marti < > 123456 -0100 \n", "committer ", "Vicent Marti", "", 123456, -60}, - // Parse a signature with empty name and email + /* Parse a signature with empty name and email */ {"committer <> 123456 -0100 \n", "committer ", "", "", 123456, -60}, - // Parse a signature with empty name and email + /* Parse a signature with empty name and email */ {"committer <> 123456 -0100 \n", "committer ", "", "", 123456, -60}, - // Parse a signature with empty name and email + /* Parse a signature with empty name and email */ {"committer < > 123456 -0100 \n", "committer ", "", "", 123456, -60}, - // Parse an obviously invalid signature + /* Parse an obviously invalid signature */ {"committer foo<@bar> 123456 -0100 \n", "committer ", "foo", "@bar", 123456, -60}, - // Parse an obviously invalid signature + /* Parse an obviously invalid signature */ {"committer foo<@bar> 123456 -0100 \n", "committer ", "foo", "@bar", 123456, -60}, - // Parse an obviously invalid signature + /* Parse an obviously invalid signature */ {"committer <>\n", "committer ", "", "", 0, 0}, {"committer Vicent Marti <tanoku@gmail.com> 123456 -1500 \n", "committer ", "Vicent Marti", "tanoku@gmail.com", 123456, 0}, {"committer Vicent Marti <tanoku@gmail.com> 123456 +0163 \n", "committer ", "Vicent Marti", "tanoku@gmail.com", 123456, 0}, @@ -177,25 +177,27 @@ void test_commit_parse__signature(void) assert_signature_doesnt_parse(failcase); } -static int pass_on_warn(git_warning *warning, void *payload) +static int pass_on_warn(git_warning *warning, int rval, void *payload) { git_warning *expected = payload; + GIT_UNUSED(rval); cl_assert_equal_i(expected->type, warning->type); if (expected->message) cl_assert(strstr(warning->message, expected->message) != NULL); return 0; } -static int fail_on_warn(git_warning *warning, void *payload) +static int fail_on_warn(git_warning *warning, int rval, void *payload) { git_warning *expected = payload; + GIT_UNUSED(rval); cl_assert_equal_i(expected->type, warning->type); if (expected->message) cl_assert(strstr(warning->message, expected->message) != NULL); return -1; } -void test_commit_parse__signature_semivalid(void) +void test_commit_parse__signature_time_warnings(void) { git_warning expected = { 0 }; passing_signature_test_case passcase = {"author Vicent Marti <tanoku@gmail.com> 9999999999998589934592 \n", "author ", "Vicent Marti", "tanoku@gmail.com", -1, 0}; @@ -228,67 +230,108 @@ void test_commit_parse__signature_semivalid(void) git_warning_set_callback(NULL, NULL); } +static int default_on_warn(git_warning *warning, int rval, void *payload) +{ + git_warning *expected = payload; + cl_assert_equal_i(expected->type, warning->type); + if (expected->message) + cl_assert(strstr(warning->message, expected->message) != NULL); + return rval; +} + +void test_commit_parse__signature_email_warnings(void) +{ + git_warning expected = { 0 }; + passing_signature_test_case passcase1 = {"author Vicent Marti tanoku@gmail.com> 98589934592 \n", "author ", "Vicent Marti tanoku@gmail.com> 98589934592", "", 0, 0}; + passing_signature_test_case passcase2 = {"committer Vicent Marti <tanoku@gmail.com 98589934592 \n", "committer ", "Vicent Marti", "tanoku@gmail.com 98589934592", 0, 0}; + + failing_signature_test_case failcase1 = {"author Vicent Marti tanoku@gmail.com> 98589934592 \n", "author "}; + failing_signature_test_case failcase2 = {"committer Vicent Marti <tanoku@gmail.com 98589934592 \n", "author "}; + + git_warning_set_callback(default_on_warn, &expected); + + expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_MISSING; + expected.message = "author"; + assert_signature_doesnt_parse(&failcase1); + + expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_UNTERMINATED; + expected.message = "committer"; + assert_signature_doesnt_parse(&failcase2); + + git_warning_set_callback(pass_on_warn, &expected); + + expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_MISSING; + expected.message = "author"; + assert_signature_parses(&passcase1); + + expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_EMAIL_UNTERMINATED; + expected.message = "committer"; + assert_signature_parses(&passcase2); + + git_warning_set_callback(NULL, NULL); +} + static char *failing_commit_cases[] = { -// empty commit +/* empty commit */ "", -// random garbage +/* random garbage */ "asd97sa9du902e9a0jdsuusad09as9du098709aweu8987sd\n", -// broken endlines 1 +/* broken endlines 1 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\r\n\ parent 05452d6349abcd67aa396dfb28660d765d8b2a36\r\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\r\n\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\r\n\ \r\n\ a test commit with broken endlines\r\n", -// broken endlines 2 +/* broken endlines 2 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\ parent 05452d6349abcd67aa396dfb28660d765d8b2a36\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\ \ another test commit with broken endlines", -// starting endlines +/* starting endlines */ "\ntree f6c0dad3c7b3481caa9d73db21f91964894a945b\n\ parent 05452d6349abcd67aa396dfb28660d765d8b2a36\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ \n\ a test commit with a starting endline\n", -// corrupted commit 1 +/* corrupted commit 1 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\n\ parent 05452d6349abcd67aa396df", -// corrupted commit 2 +/* corrupted commit 2 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\n\ parent ", -// corrupted commit 3 +/* corrupted commit 3 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\n\ parent ", -// corrupted commit 4 +/* corrupted commit 4 */ "tree f6c0dad3c7b3481caa9d73db21f91964894a945b\n\ par", }; static char *passing_commit_cases[] = { -// simple commit with no message +/* simple commit with no message */ "tree 1810dff58d8a660512d4832e740f692884338ccd\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ \n", -// simple commit, no parent +/* simple commit, no parent */ "tree 1810dff58d8a660512d4832e740f692884338ccd\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ \n\ a simple commit which works\n", -// simple commit, no parent, no newline in message +/* simple commit, no parent, no newline in message */ "tree 1810dff58d8a660512d4832e740f692884338ccd\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ committer Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ \n\ a simple commit which works", -// simple commit, 1 parent +/* simple commit, 1 parent */ "tree 1810dff58d8a660512d4832e740f692884338ccd\n\ parent e90810b8df3e80c413d903f631643c716887138d\n\ author Vicent Marti <tanoku@gmail.com> 1273848544 +0200\n\ @@ -367,7 +410,7 @@ void test_commit_parse__entire_commit(void) } -// query the details on a parsed commit +/* query the details on a parsed commit */ void test_commit_parse__details0(void) { static const char *commit_ids[] = { "a4a7dce85cf63874e984719f4fdd239f5145052f", /* 0 */ @@ -415,7 +458,7 @@ void test_commit_parse__details0(void) { old_parent = parent; cl_git_pass(git_commit_parent(&parent, commit, p)); cl_assert(parent != NULL); - cl_assert(git_commit_author(parent) != NULL); // is it really a commit? + cl_assert(git_commit_author(parent) != NULL); /* is it really a commit? */ } git_commit_free(old_parent); git_commit_free(parent); |