Merge pull request #875 from arrbee/fix-message-prettify-length-check

Fix message prettify length check

include/git2/message.h

@@ -24,7 +24,8 @@ 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.
+ * 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);
 

src/message.c

@@ -62,21 +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;
 
-	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;
-	}
-
+	if (message_out && buffer_size)
 		*message_out = '\0';
 
-	if (git_message__prettify(&buf, message, strip_comments) < 0) {
-		git_buf_free(&buf);
-		return -1;
+	if (git_message__prettify(&buf, message, strip_comments) < 0)
+		goto done;
+
+	if (message_out && buf.size + 1 > buffer_size) { /* +1 for NUL byte */
+		giterr_set(GITERR_INVALID, "Buffer too short to hold the cleaned message");
+		goto done;
 	}
 
+	if (message_out)
 		git_buf_copy_cstr(message_out, buffer_size, &buf);
-	git_buf_free(&buf);
 
-	return 0;
+	out_size = buf.size + 1;
+
+done:
+	git_buf_free(&buf);
+	return out_size;
 }

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,
@@ -128,3 +128,68 @@ 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_assert(git_message_prettify(buffer, sizeof(buffer), "", 0) == 1);
+	cl_assert_equal_s(buffer, "");
+	cl_assert(git_message_prettify(buffer, sizeof(buffer), "", 1) == 1);
+	cl_assert_equal_s(buffer, "");
+
+	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_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_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_assert(git_message_prettify(buffer, sizeof(buffer),
+		"1234567890" "1234567890" "1234567890" "1234567890" "1234567890"
+		"1234567890" "1234567890" "1234567890" "1234567890" "12345678",
+		0) > 0);
+	cl_assert_equal_s(buffer,
+		"1234567890" "1234567890" "1234567890" "1234567890" "1234567890"
+		"1234567890" "1234567890" "1234567890" "1234567890" "12345678\n");
+
+	cl_assert(git_message_prettify(buffer, sizeof(buffer),
+		"1234567890" "1234567890" "1234567890" "1234567890" "1234567890"
+		"1234567890" "1234567890" "1234567890" "1234567890" "12345678\n",
+		0) > 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_assert(git_message_prettify(buffer, sizeof(buffer),
+		"1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n"
+		"# 1234567890" "1234567890" "1234567890" "1234567890" "1234567890\n"
+		"1234567890",
+		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);
+}