Merge pull request #1123 from carlosmn/lax-tree

tree: relax the filemode parser

3 files changed, 48 insertions, 36 deletions

diff --git a/src/tree.c b/src/tree.c
index fedf4b604..efb991df1 100644
--- a/src/tree.c
+++ b/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);
 
diff --git a/tests-clar/object/tree/attributes.c b/tests-clar/object/tree/attributes.c
index 054f67137..b5319d30e 100644
--- a/tests-clar/object/tree/attributes.c
+++ b/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();
+}
diff --git a/tests-clar/resources/deprecated-mode.git/objects/08/10fb7818088ff5ac41ee49199b51473b1bd6c7 b/tests-clar/resources/deprecated-mode.git/objects/08/10fb7818088ff5ac41ee49199b51473b1bd6c7
new file mode 100644
index 000000000..52d56936a
--- /dev/null
+++ b/tests-clar/resources/deprecated-mode.git/objects/08/10fb7818088ff5ac41ee49199b51473b1bd6c7