git_commit: validate tree and parent ids

When `GIT_OPT_ENABLE_STRICT_OBJECT_CREATION` is turned on, validate
the tree and parent ids given to commit creation functions.

2 files changed, 143 insertions, 16 deletions

diff --git a/src/commit.c b/src/commit.c
index 5a0509803..685c642aa 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -17,6 +17,7 @@
 #include "signature.h"
 #include "message.h"
 #include "refs.h"
+#include "object.h"
 
 void git_commit__free(void *_commit)
 {
@@ -36,7 +37,7 @@ void git_commit__free(void *_commit)
 	git__free(commit);
 }
 
-int git_commit_create_from_callback(
+static int git_commit__create_internal(
 	git_oid *id,
 	git_repository *repo,
 	const char *update_ref,
@@ -46,7 +47,8 @@ int git_commit_create_from_callback(
 	const char *message,
 	const git_oid *tree,
 	git_commit_parent_callback parent_cb,
-	void *parent_payload)
+	void *parent_payload,
+	bool validate)
 {
 	git_reference *ref = NULL;
 	int error = 0, matched_parent = 0;
@@ -58,6 +60,9 @@ int git_commit_create_from_callback(
 
 	assert(id && repo && tree && parent_cb);
 
+	if (validate && !git_object__is_valid(repo, tree, GIT_OBJ_TREE))
+		return -1;
+
 	if (update_ref) {
 		error = git_reference_lookup_resolved(&ref, repo, update_ref, 10);
 		if (error < 0 && error != GIT_ENOTFOUND)
@@ -71,6 +76,11 @@ int git_commit_create_from_callback(
 	git_oid__writebuf(&commit, "tree ", tree);
 
 	while ((parent = parent_cb(i, parent_payload)) != NULL) {
+		if (validate && !git_object__is_valid(repo, parent, GIT_OBJ_COMMIT)) {
+			error = -1;
+			goto on_error;
+		}
+
 		git_oid__writebuf(&commit, "parent ", parent);
 		if (i == 0 && current_id && git_oid_equal(current_id, parent))
 			matched_parent = 1;
@@ -114,10 +124,26 @@ int git_commit_create_from_callback(
 
 on_error:
 	git_buf_free(&commit);
-	giterr_set(GITERR_OBJECT, "Failed to create commit.");
 	return -1;
 }
 
+int git_commit_create_from_callback(
+	git_oid *id,
+	git_repository *repo,
+	const char *update_ref,
+	const git_signature *author,
+	const git_signature *committer,
+	const char *message_encoding,
+	const char *message,
+	const git_oid *tree,
+	git_commit_parent_callback parent_cb,
+	void *parent_payload)
+{
+	return git_commit__create_internal(
+		id, repo, update_ref, author, committer, message_encoding, message,
+		tree, parent_cb, parent_payload, true);
+}
+
 typedef struct {
 	size_t total;
 	va_list args;
@@ -153,10 +179,10 @@ int git_commit_create_v(
 	data.total = parent_count;
 	va_start(data.args, parent_count);
 
-	error = git_commit_create_from_callback(
+	error = git_commit__create_internal(
 		id, repo, update_ref, author, committer,
 		message_encoding, message, git_tree_id(tree),
-		commit_parent_from_varargs, &data);
+		commit_parent_from_varargs, &data, false);
 
 	va_end(data.args);
 	return error;
@@ -187,10 +213,10 @@ int git_commit_create_from_ids(
 {
 	commit_parent_oids data = { parent_count, parents };
 
-	return git_commit_create_from_callback(
+	return git_commit__create_internal(
 		id, repo, update_ref, author, committer,
 		message_encoding, message, tree,
-		commit_parent_from_ids, &data);
+		commit_parent_from_ids, &data, true);
 }
 
 typedef struct {
@@ -227,10 +253,10 @@ int git_commit_create(
 
 	assert(tree && git_tree_owner(tree) == repo);
 
-	return git_commit_create_from_callback(
+	return git_commit__create_internal(
 		id, repo, update_ref, author, committer,
 		message_encoding, message, git_tree_id(tree),
-		commit_parent_from_array, &data);
+		commit_parent_from_array, &data, false);
 }
 
 static const git_oid *commit_parent_for_amend(size_t curr, void *payload)
@@ -290,9 +316,9 @@ int git_commit_amend(
 		}
 	}
 
-	error = git_commit_create_from_callback(
+	error = git_commit__create_internal(
 		id, repo, NULL, author, committer, message_encoding, message,
-		&tree_id, commit_parent_for_amend, (void *)commit_to_amend);
+		&tree_id, commit_parent_for_amend, (void *)commit_to_amend, false);
 
 	if (!error && update_ref) {
 		error = git_reference__update_for_commit(
diff --git a/tests/commit/write.c b/tests/commit/write.c
index 176965cbd..303d1ce58 100644
--- a/tests/commit/write.c
+++ b/tests/commit/write.c
@@ -1,10 +1,12 @@
 #include "clar_libgit2.h"
+#include "git2/sys/commit.h"
 
 static const char *committer_name = "Vicent Marti";
 static const char *committer_email = "vicent@github.com";
 static const char *commit_message = "This commit has been created in memory\n\
    This is a commit created in memory and it will be written back to disk\n";
-static const char *tree_oid = "1810dff58d8a660512d4832e740f692884338ccd";
+static const char *tree_id_str = "1810dff58d8a660512d4832e740f692884338ccd";
+static const char *parent_id_str = "8496071c1b46c854b31185ea97743be6a8774479";
 static const char *root_commit_message = "This is a root commit\n\
    This is a root commit and should be the only one in this branch\n";
 static const char *root_reflog_message = "commit (initial): This is a root commit \
@@ -35,6 +37,8 @@ void test_commit_write__cleanup(void)
 	head_old = NULL;
 
 	cl_git_sandbox_cleanup();
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0));
 }
 
 
@@ -46,12 +50,11 @@ void test_commit_write__from_memory(void)
    const git_signature *author1, *committer1;
    git_commit *parent;
    git_tree *tree;
-   const char *commit_id_str = "8496071c1b46c854b31185ea97743be6a8774479";
 
-   git_oid_fromstr(&tree_id, tree_oid);
+   git_oid_fromstr(&tree_id, tree_id_str);
    cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id));
 
-   git_oid_fromstr(&parent_id, commit_id_str);
+   git_oid_fromstr(&parent_id, parent_id_str);
    cl_git_pass(git_commit_lookup(&parent, g_repo, &parent_id));
 
    /* create signatures */
@@ -106,7 +109,7 @@ void test_commit_write__root(void)
 	git_reflog *log;
 	const git_reflog_entry *entry;
 
-	git_oid_fromstr(&tree_id, tree_oid);
+	git_oid_fromstr(&tree_id, tree_id_str);
 	cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id));
 
 	/* create signatures */
@@ -158,3 +161,101 @@ void test_commit_write__root(void)
 	git_signature_free(committer);
 	git_reflog_free(log);
 }
+
+static int create_commit_from_ids(
+	git_oid *result,
+	const git_oid *tree_id,
+	const git_oid *parent_id)
+{
+	git_signature *author, *committer;
+	const git_oid *parent_ids[1];
+	int ret;
+
+	cl_git_pass(git_signature_new(
+		&committer, committer_name, committer_email, 123456789, 60));
+	cl_git_pass(git_signature_new(
+		&author, committer_name, committer_email, 987654321, 90));
+
+	parent_ids[0] = parent_id;
+
+	ret = git_commit_create_from_ids(
+		result,
+		g_repo,
+		NULL,
+		author,
+		committer,
+		NULL,
+		root_commit_message,
+		tree_id,
+		1,
+		parent_ids);
+
+	git_signature_free(committer);
+	git_signature_free(author);
+
+	return ret;
+}
+
+void test_commit_write__doesnt_validate_objects_by_default(void)
+{
+	git_oid expected_id, tree_id, parent_id, commit_id;
+
+	/* this is a valid tree and parent */
+	git_oid_fromstr(&tree_id, tree_id_str);
+	git_oid_fromstr(&parent_id, parent_id_str);
+
+	git_oid_fromstr(&expected_id, "c8571bbec3a72c4bcad31648902e5a453f1adece");
+	cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+	cl_assert_equal_oid(&expected_id, &commit_id);
+
+	/* this is a wholly invented tree id */
+	git_oid_fromstr(&tree_id, "1234567890123456789012345678901234567890");
+	git_oid_fromstr(&parent_id, parent_id_str);
+
+	git_oid_fromstr(&expected_id, "996008340b8e68d69bf3c28d7c57fb7ec3c8e202");
+	cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+	cl_assert_equal_oid(&expected_id, &commit_id);
+
+	/* this is a wholly invented parent id */
+	git_oid_fromstr(&tree_id, tree_id_str);
+	git_oid_fromstr(&parent_id, "1234567890123456789012345678901234567890");
+
+	git_oid_fromstr(&expected_id, "d78f660cab89d9791ca6714b57978bf2a7e709fd");
+	cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+	cl_assert_equal_oid(&expected_id, &commit_id);
+
+	/* these are legitimate objects, but of the wrong type */
+	git_oid_fromstr(&tree_id, parent_id_str);
+	git_oid_fromstr(&parent_id, tree_id_str);
+
+	git_oid_fromstr(&expected_id, "5d80c07414e3f18792949699dfcacadf7748f361");
+	cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+	cl_assert_equal_oid(&expected_id, &commit_id);
+}
+
+void test_commit_write__can_validate_objects(void)
+{
+	git_oid tree_id, parent_id, commit_id;
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1));
+
+	/* this is a valid tree and parent */
+	git_oid_fromstr(&tree_id, tree_id_str);
+	git_oid_fromstr(&parent_id, parent_id_str);
+	cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+
+	/* this is a wholly invented tree id */
+	git_oid_fromstr(&tree_id, "1234567890123456789012345678901234567890");
+	git_oid_fromstr(&parent_id, parent_id_str);
+	cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+
+	/* this is a wholly invented parent id */
+	git_oid_fromstr(&tree_id, tree_id_str);
+	git_oid_fromstr(&parent_id, "1234567890123456789012345678901234567890");
+	cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+
+	/* these are legitimate objects, but of the wrong type */
+	git_oid_fromstr(&tree_id, parent_id_str);
+	git_oid_fromstr(&parent_id, tree_id_str);
+	cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id));
+}