From fdc637c4e266349b35ac4fb45a4e5aa63c5a78e0 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sun, 12 Aug 2012 09:08:45 -0700 Subject: [PATCH 1/2] Check prettify message output buffer after cleanup This makes the message prettify buffer length check accurate. --- src/message.c | 10 ++-- tests-clar/object/commit/commitstagedfile.c | 60 +++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/message.c b/src/message.c index a4aadb28f..a5cc26237 100644 --- a/src/message.c +++ b/src/message.c @@ -63,10 +63,7 @@ int git_message_prettify(char *message_out, size_t buffer_size, const char *mess { git_buf buf = GIT_BUF_INIT; - if (strlen(message) + 1 > buffer_size) { /* We have to account for a potentially missing \n */ - giterr_set(GITERR_INVALID, "Buffer too short to hold the cleaned message"); - return -1; - } + assert(message_out && buffer_size); *message_out = '\0'; @@ -75,6 +72,11 @@ int git_message_prettify(char *message_out, size_t buffer_size, const char *mess return -1; } + if (buf.size + 1 > buffer_size) { /* +1 for NUL byte */ + giterr_set(GITERR_INVALID, "Buffer too short to hold the cleaned message"); + return -1; + } + git_buf_copy_cstr(message_out, buffer_size, &buf); git_buf_free(&buf); diff --git a/tests-clar/object/commit/commitstagedfile.c b/tests-clar/object/commit/commitstagedfile.c index 628ef43c2..1e4affb8c 100644 --- a/tests-clar/object/commit/commitstagedfile.c +++ b/tests-clar/object/commit/commitstagedfile.c @@ -128,3 +128,63 @@ void test_object_commit_commitstagedfile__generate_predictable_object_ids(void) git_tree_free(tree); git_index_free(index); } + +void test_object_commit_commitstagedfile__message_prettify(void) +{ + char buffer[100]; + + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "", 0)); + cl_assert_equal_s(buffer, ""); + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "", 1)); + cl_assert_equal_s(buffer, ""); + + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "Short", 0)); + cl_assert_equal_s(buffer, "Short\n"); + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "Short", 1)); + cl_assert_equal_s(buffer, "Short\n"); + + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 0)); + cl_assert_equal_s(buffer, "This is longer\nAnd multiline\n# with some comments still in\n"); + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 1)); + cl_assert_equal_s(buffer, "This is longer\nAnd multiline\n"); + + /* try out overflow */ + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "12345678", + 0)); + cl_assert_equal_s(buffer, + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n"); + + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n", + 0)); + cl_assert_equal_s(buffer, + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n"); + + cl_git_fail(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "123456789", + 0)); + cl_git_fail(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "123456789\n", + 0)); + cl_git_fail(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890", + 0)); + cl_git_fail(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890""x", + 0)); + + cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + "1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n" + "# 1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n" + "1234567890", + 1)); +} From 85a0e28b80e42a52247e16478b5f75475b00e56b Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 14 Aug 2012 10:50:58 -0700 Subject: [PATCH 2/2] Make git_message_prettify return bytes written If you want to be absolutely safe with git_message_prettify, you can now pass a NULL pointer for the buffer and get back the number of bytes that would be copied into the buffer. This means that an error is a non-negative return code and a success will be greater than zero from this function. --- include/git2/message.h | 8 +++-- src/message.c | 26 ++++++++------- tests-clar/object/commit/commitstagedfile.c | 35 ++++++++++++--------- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/include/git2/message.h b/include/git2/message.h index 7f2558583..b42cb7677 100644 --- a/include/git2/message.h +++ b/include/git2/message.h @@ -23,8 +23,9 @@ GIT_BEGIN_DECL * * Optionally, can remove lines starting with a "#". * - * @param message_out The user allocated buffer which will be filled with - * the cleaned up message. + * @param message_out The user allocated buffer which will be filled with + * the cleaned up message. Pass NULL if you just want to get the size of the + * prettified message as the output value. * * @param size The size of the allocated buffer message_out. * @@ -32,7 +33,8 @@ GIT_BEGIN_DECL * * @param strip_comments 1 to remove lines starting with a "#", 0 otherwise. * - * @return GIT_SUCCESS or an error code + * @return -1 on error, else number of characters in prettified message + * including the trailing NUL byte */ GIT_EXTERN(int) git_message_prettify(char *message_out, size_t buffer_size, const char *message, int strip_comments); diff --git a/src/message.c b/src/message.c index a5cc26237..e6dedc9fb 100644 --- a/src/message.c +++ b/src/message.c @@ -62,23 +62,25 @@ int git_message__prettify(git_buf *message_out, const char *message, int strip_c int git_message_prettify(char *message_out, size_t buffer_size, const char *message, int strip_comments) { git_buf buf = GIT_BUF_INIT; + ssize_t out_size = -1; - assert(message_out && buffer_size); + if (message_out && buffer_size) + *message_out = '\0'; - *message_out = '\0'; + if (git_message__prettify(&buf, message, strip_comments) < 0) + goto done; - if (git_message__prettify(&buf, message, strip_comments) < 0) { - git_buf_free(&buf); - return -1; - } - - if (buf.size + 1 > buffer_size) { /* +1 for NUL byte */ + if (message_out && buf.size + 1 > buffer_size) { /* +1 for NUL byte */ giterr_set(GITERR_INVALID, "Buffer too short to hold the cleaned message"); - return -1; + goto done; } - git_buf_copy_cstr(message_out, buffer_size, &buf); - git_buf_free(&buf); + if (message_out) + git_buf_copy_cstr(message_out, buffer_size, &buf); - return 0; + out_size = buf.size + 1; + +done: + git_buf_free(&buf); + return out_size; } diff --git a/tests-clar/object/commit/commitstagedfile.c b/tests-clar/object/commit/commitstagedfile.c index 1e4affb8c..882fb49ae 100644 --- a/tests-clar/object/commit/commitstagedfile.c +++ b/tests-clar/object/commit/commitstagedfile.c @@ -109,7 +109,7 @@ void test_object_commit_commitstagedfile__generate_predictable_object_ids(void) cl_git_pass(git_signature_new(&signature, "nulltoken", "emeric.fermas@gmail.com", 1323847743, 60)); cl_git_pass(git_tree_lookup(&tree, repo, &tree_oid)); - cl_git_pass(git_message_prettify(buffer, 128, "Initial commit", 0)); + cl_assert_equal_i(16, git_message_prettify(buffer, 128, "Initial commit", 0)); cl_git_pass(git_commit_create_v( &commit_oid, @@ -133,34 +133,35 @@ void test_object_commit_commitstagedfile__message_prettify(void) { char buffer[100]; - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "", 0)); + cl_assert(git_message_prettify(buffer, sizeof(buffer), "", 0) == 1); cl_assert_equal_s(buffer, ""); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "", 1)); + cl_assert(git_message_prettify(buffer, sizeof(buffer), "", 1) == 1); cl_assert_equal_s(buffer, ""); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "Short", 0)); - cl_assert_equal_s(buffer, "Short\n"); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "Short", 1)); - cl_assert_equal_s(buffer, "Short\n"); + cl_assert_equal_i(7, git_message_prettify(buffer, sizeof(buffer), "Short", 0)); + cl_assert_equal_s("Short\n", buffer); + cl_assert_equal_i(7, git_message_prettify(buffer, sizeof(buffer), "Short", 1)); + cl_assert_equal_s("Short\n", buffer); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 0)); + cl_assert(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 0) > 0); cl_assert_equal_s(buffer, "This is longer\nAnd multiline\n# with some comments still in\n"); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 1)); + + cl_assert(git_message_prettify(buffer, sizeof(buffer), "This is longer\nAnd multiline\n# with some comments still in\n", 1) > 0); cl_assert_equal_s(buffer, "This is longer\nAnd multiline\n"); /* try out overflow */ - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + cl_assert(git_message_prettify(buffer, sizeof(buffer), "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "12345678", - 0)); + 0) > 0); cl_assert_equal_s(buffer, "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n"); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + cl_assert(git_message_prettify(buffer, sizeof(buffer), "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n", - 0)); + 0) > 0); cl_assert_equal_s(buffer, "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "1234567890" "12345678\n"); @@ -182,9 +183,13 @@ void test_object_commit_commitstagedfile__message_prettify(void) "1234567890" "1234567890" "1234567890" "1234567890" "1234567890""x", 0)); - cl_git_pass(git_message_prettify(buffer, sizeof(buffer), + cl_assert(git_message_prettify(buffer, sizeof(buffer), "1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n" "# 1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n" "1234567890", - 1)); + 1) > 0); + + cl_assert(git_message_prettify(NULL, 0, "", 0) == 1); + cl_assert(git_message_prettify(NULL, 0, "Short test", 0) == 12); + cl_assert(git_message_prettify(NULL, 0, "Test\n# with\nComments", 1) == 15); }