Commit fd490d3ed5812b930768071433882ffae14da468

Carlos Martín Nieto 2018-10-08T13:15:31

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.

diff --git a/src/tree.c b/src/tree.c
index e4a0547..d628aeb 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)) {