tree: relax the filemode parser

There are many different broken filemodes in the wild so we need to
protect against them and give something useful up the chain. Don't
fail when reading a tree from the ODB but normalize the mode as best
we can.

As 664 is no longer a mode that we consider to be valid and gets
normalized to 644, we can stop accepting it in the treebuilder. The
library won't expose it to the user, so any invalid modes are a bug.

src/tree.c

@@ -18,12 +18,33 @@ static bool valid_filemode(const int filemode)
 {
 	return (filemode == GIT_FILEMODE_TREE
 		|| filemode == GIT_FILEMODE_BLOB
-		|| filemode == GIT_FILEMODE_BLOB_GROUP_WRITABLE
 		|| filemode == GIT_FILEMODE_BLOB_EXECUTABLE
 		|| filemode == GIT_FILEMODE_LINK
 		|| filemode == GIT_FILEMODE_COMMIT);
 }
 
+GIT_INLINE(git_filemode_t) normalize_filemode(git_filemode_t filemode)
+{
+	/* Tree bits set, but it's not a commit */
+	if (filemode & GIT_FILEMODE_TREE && !(filemode & 0100000))
+		return GIT_FILEMODE_TREE;
+
+	/* If any of the x bits is set */
+	if (filemode & 0111)
+		return GIT_FILEMODE_BLOB_EXECUTABLE;
+
+	/* 16XXXX means commit */
+	if ((filemode & GIT_FILEMODE_COMMIT) == GIT_FILEMODE_COMMIT)
+		return GIT_FILEMODE_COMMIT;
+
+	/* 12XXXX means commit */
+	if ((filemode & GIT_FILEMODE_LINK) == GIT_FILEMODE_LINK)
+		return GIT_FILEMODE_LINK;
+
+	/* Otherwise, return a blob */
+	return GIT_FILEMODE_BLOB;
+}
+
 static int valid_entry_name(const char *filename)
 {
 	return *filename != '\0' &&
@@ -320,10 +341,11 @@ static int tree_parse_buffer(git_tree *tree, const char *buffer, const char *buf
 		git_tree_entry *entry;
 		int attr;
 
-		if (git__strtol32(&attr, buffer, &buffer, 8) < 0 ||
-			!buffer || !valid_filemode(attr))
+		if (git__strtol32(&attr, buffer, &buffer, 8) < 0 || !buffer)
 			return tree_error("Failed to parse tree. Can't parse filemode", NULL);
 
+		attr = normalize_filemode(attr); /* make sure to normalize the filemode */
+
 		if (*buffer++ != ' ')
 			return tree_error("Failed to parse tree. Object is corrupted", NULL);
 
@@ -529,19 +551,6 @@ static void sort_entries(git_treebuilder *bld)
 	git_vector_sort(&bld->entries);
 }
 
-GIT_INLINE(git_filemode_t) normalize_filemode(git_filemode_t filemode)
-{
-	/* 100664 mode is an early design mistake. Tree entries may bear
-	 * this mode in some old git repositories, but it's now deprecated.
-	 * We silently normalize while inserting new entries in a tree 
-	 * being built.
-	 */
-	if (filemode == GIT_FILEMODE_BLOB_GROUP_WRITABLE)
-		return GIT_FILEMODE_BLOB;
-
-	return filemode;
-}
-
 int git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source)
 {
 	git_treebuilder *bld;
@@ -565,7 +574,7 @@ int git_treebuilder_create(git_treebuilder **builder_p, const git_tree *source)
 			if (append_entry(
 				bld, entry_src->filename,
 				&entry_src->oid,
-				normalize_filemode((git_filemode_t)entry_src->attr)) < 0)
+				entry_src->attr) < 0)
 				goto on_error;
 		}
 	}
@@ -593,8 +602,6 @@ int git_treebuilder_insert(
 	if (!valid_filemode(filemode))
 		return tree_error("Failed to insert entry. Invalid filemode for file", filename);
 
-	filemode = normalize_filemode(filemode);
-
 	if (!valid_entry_name(filename))
 		return tree_error("Failed to insert entry. Invalid name for a tree entry", filename);
 

tests-clar/object/tree/attributes.c

@@ -34,14 +34,14 @@ void test_object_tree_attributes__group_writable_tree_entries_created_with_an_an
 
 	entry = git_tree_entry_byname(tree, "old_mode.txt");
 	cl_assert_equal_i(
-		GIT_FILEMODE_BLOB_GROUP_WRITABLE,
+		GIT_FILEMODE_BLOB,
 		git_tree_entry_filemode(entry));
 
 	git_tree_free(tree);
 	git_repository_free(repo);
 }
 
-void test_object_tree_attributes__normalize_attributes_when_inserting_in_a_new_tree(void)
+void test_object_tree_attributes__treebuilder_reject_invalid_filemode(void)
 {
 	git_repository *repo;
 	git_treebuilder *builder;
@@ -55,28 +55,14 @@ void test_object_tree_attributes__normalize_attributes_when_inserting_in_a_new_t
 
 	cl_git_pass(git_treebuilder_create(&builder, NULL));
 
-	cl_git_pass(git_treebuilder_insert(
+	cl_git_fail(git_treebuilder_insert(
 		&entry,
 		builder,
 		"normalized.txt",
 		&bid,
 		GIT_FILEMODE_BLOB_GROUP_WRITABLE));
 
-	cl_assert_equal_i(
-		GIT_FILEMODE_BLOB,
-		git_tree_entry_filemode(entry));
-	
-	cl_git_pass(git_treebuilder_write(&tid, repo, builder));
 	git_treebuilder_free(builder);
-
-	cl_git_pass(git_tree_lookup(&tree, repo, &tid));
-
-	entry = git_tree_entry_byname(tree, "normalized.txt");
-	cl_assert_equal_i(
-		GIT_FILEMODE_BLOB,
-		git_tree_entry_filemode(entry));
-
-	git_tree_free(tree);
 	cl_git_sandbox_cleanup();
 }
 
@@ -113,3 +99,22 @@ void test_object_tree_attributes__normalize_attributes_when_creating_a_tree_from
 	git_tree_free(tree);
 	cl_git_sandbox_cleanup();
 }
+
+void test_object_tree_attributes__normalize_600(void)
+{
+	git_oid id;
+	git_tree *tree;
+	git_repository *repo;
+	const git_tree_entry *entry;
+
+	repo = cl_git_sandbox_init("deprecated-mode.git");
+
+	git_oid_fromstr(&id, "0810fb7818088ff5ac41ee49199b51473b1bd6c7");
+	cl_git_pass(git_tree_lookup(&tree, repo, &id));
+
+	entry = git_tree_entry_byname(tree, "ListaTeste.xml");
+	cl_assert_equal_i(entry->attr, GIT_FILEMODE_BLOB);
+
+	git_tree_free(tree);
+	cl_git_sandbox_cleanup();
+}