tree: unify the entry validity checks

We have two similar functions, `git_treebuilder_insert` and `append_entry` which
are used in different codepaths as part of creating a new tree. The former
learnt to check for object existence under strict object creation, but the
latter did not.

This allowed the creation of a tree from an unowned index to bypass some of the
checks and create a tree pointing to a nonexistent object.

Extract a single function which performs these checks and call it from both
codepaths. In `append_entry` we still do not validate when asked not to, as this
is data which is already in the tree and we want to allow users to deal with
repositories which already have some invalid data.

1 files changed, 34 insertions, 29 deletions

diff --git a/src/tree.c b/src/tree.c
index e4a0547a3..d628aeb64 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -456,6 +456,36 @@ static size_t find_next_dir(const char *dirname, git_index *index, size_t start)
 	return i;
 }
 
+static git_otype otype_from_mode(git_filemode_t filemode)
+{
+	switch (filemode) {
+	case GIT_FILEMODE_TREE:
+		return GIT_OBJ_TREE;
+	case GIT_FILEMODE_COMMIT:
+		return GIT_OBJ_COMMIT;
+	default:
+		return GIT_OBJ_BLOB;
+	}
+}
+
+static int check_entry(git_repository *repo, const char *filename, const git_oid *id, git_filemode_t filemode)
+{
+	if (!valid_filemode(filemode))
+		return tree_error("failed to insert entry: invalid filemode for file", filename);
+
+	if (!valid_entry_name(repo, filename))
+		return tree_error("failed to insert entry: invalid name for a tree entry", filename);
+
+	if (git_oid_iszero(id))
+		return tree_error("failed to insert entry: invalid null OID", filename);
+
+	if (filemode != GIT_FILEMODE_COMMIT &&
+	    !git_object__is_valid(repo, id, otype_from_mode(filemode)))
+		return tree_error("failed to insert entry: invalid object specified", filename);
+
+	return 0;
+}
+
 static int append_entry(
 	git_treebuilder *bld,
 	const char *filename,
@@ -466,11 +496,8 @@ static int append_entry(
 	git_tree_entry *entry;
 	int error = 0;
 
-	if (validate && !valid_entry_name(bld->repo, filename))
-		return tree_error("failed to insert entry: invalid name for a tree entry", filename);
-
-	if (validate && git_oid_iszero(id))
-		return tree_error("failed to insert entry: invalid null OID for a tree entry", filename);
+	if (validate && ((error = check_entry(bld->repo, filename, id, filemode)) < 0))
+		return error;
 
 	entry = alloc_entry(filename, strlen(filename), id);
 	GITERR_CHECK_ALLOC(entry);
@@ -684,18 +711,6 @@ on_error:
 	return -1;
 }
 
-static git_otype otype_from_mode(git_filemode_t filemode)
-{
-	switch (filemode) {
-	case GIT_FILEMODE_TREE:
-		return GIT_OBJ_TREE;
-	case GIT_FILEMODE_COMMIT:
-		return GIT_OBJ_COMMIT;
-	default:
-		return GIT_OBJ_BLOB;
-	}
-}
-
 int git_treebuilder_insert(
 	const git_tree_entry **entry_out,
 	git_treebuilder *bld,
@@ -709,18 +724,8 @@ int git_treebuilder_insert(
 
 	assert(bld && id && filename);
 
-	if (!valid_filemode(filemode))
-		return tree_error("failed to insert entry: invalid filemode for file", filename);
-
-	if (!valid_entry_name(bld->repo, filename))
-		return tree_error("failed to insert entry: invalid name for a tree entry", filename);
-
-	if (git_oid_iszero(id))
-		return tree_error("failed to insert entry: invalid null OID", filename);
-
-	if (filemode != GIT_FILEMODE_COMMIT &&
-	    !git_object__is_valid(bld->repo, id, otype_from_mode(filemode)))
-		return tree_error("failed to insert entry: invalid object specified", filename);
+	if ((error = check_entry(bld->repo, filename, id, filemode)) < 0)
+		return error;
 
 	pos = git_strmap_lookup_index(bld->map, filename);
 	if (git_strmap_valid_index(bld->map, pos)) {