Merge pull request #4858 from tiennou/fix/index-ext-read

index: preserve extension parsing errors

10 files changed, 54 insertions, 14 deletions

diff --git a/docs/changelog.md b/docs/changelog.md
index f97f04842..bc139cd65 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -71,6 +71,9 @@ v0.27 + 1
 * Revision walks are now more efficient when the output is unsorted;
   we now avoid walking all the way to the beginning of history unnecessarily.
 
+* Error-handling around index extension loading has been fixed. We were
+  previously always misreporting a truncated index (#4858).
+
 ### API additions
 
 * The index may now be iterated atomically using `git_index_iterator`.
diff --git a/src/index.c b/src/index.c
index 677faea47..8b753234f 100644
--- a/src/index.c
+++ b/src/index.c
@@ -138,7 +138,7 @@ struct reuc_entry_internal {
 bool git_index__enforce_unsaved_safety = false;
 
 /* local declarations */
-static size_t read_extension(git_index *index, const char *buffer, size_t buffer_size);
+static int read_extension(size_t *read_len, git_index *index, const char *buffer, size_t buffer_size);
 static int read_header(struct index_header *dest, const void *buffer);
 
 static int parse_index(git_index *index, const char *buffer, size_t buffer_size);
@@ -2526,7 +2526,7 @@ static int read_header(struct index_header *dest, const void *buffer)
 	return 0;
 }
 
-static size_t read_extension(git_index *index, const char *buffer, size_t buffer_size)
+static int read_extension(size_t *read_len, git_index *index, const char *buffer, size_t buffer_size)
 {
 	struct index_extension dest;
 	size_t total_size;
@@ -2539,31 +2539,36 @@ static size_t read_extension(git_index *index, const char *buffer, size_t buffer
 
 	if (dest.extension_size > total_size ||
 		buffer_size < total_size ||
-		buffer_size - total_size < INDEX_FOOTER_SIZE)
-		return 0;
+		buffer_size - total_size < INDEX_FOOTER_SIZE) {
+		index_error_invalid("extension is truncated");
+		return -1;
+	}
 
 	/* optional extension */
 	if (dest.signature[0] >= 'A' && dest.signature[0] <= 'Z') {
 		/* tree cache */
 		if (memcmp(dest.signature, INDEX_EXT_TREECACHE_SIG, 4) == 0) {
 			if (git_tree_cache_read(&index->tree, buffer + 8, dest.extension_size, &index->tree_pool) < 0)
-				return 0;
+				return -1;
 		} else if (memcmp(dest.signature, INDEX_EXT_UNMERGED_SIG, 4) == 0) {
 			if (read_reuc(index, buffer + 8, dest.extension_size) < 0)
-				return 0;
+				return -1;
 		} else if (memcmp(dest.signature, INDEX_EXT_CONFLICT_NAME_SIG, 4) == 0) {
 			if (read_conflict_names(index, buffer + 8, dest.extension_size) < 0)
-				return 0;
+				return -1;
 		}
 		/* else, unsupported extension. We cannot parse this, but we can skip
 		 * it by returning `total_size */
 	} else {
 		/* we cannot handle non-ignorable extensions;
 		 * in fact they aren't even defined in the standard */
-		return 0;
+		git_error_set(GIT_ERROR_INDEX, "unsupported mandatory extension: '%.4s'", dest.signature);
+		return -1;
 	}
 
-	return total_size;
+	*read_len = total_size;
+
+	return 0;
 }
 
 static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
@@ -2645,11 +2650,7 @@ static int parse_index(git_index *index, const char *buffer, size_t buffer_size)
 	while (buffer_size > INDEX_FOOTER_SIZE) {
 		size_t extension_size;
 
-		extension_size = read_extension(index, buffer, buffer_size);
-
-		/* see if we have read any bytes from the extension */
-		if (extension_size == 0) {
-			error = index_error_invalid("extension is truncated");
+		if ((error = read_extension(&extension_size, index, buffer, buffer_size)) < 0) {
 			goto done;
 		}
 
diff --git a/tests/index/splitindex.c b/tests/index/splitindex.c
new file mode 100644
index 000000000..d32ed1022
--- /dev/null
+++ b/tests/index/splitindex.c
@@ -0,0 +1,21 @@
+#include "clar_libgit2.h"
+#include "index.h"
+
+static git_repository *g_repo;
+
+void test_index_splitindex__initialize(void)
+{
+	g_repo = cl_git_sandbox_init("splitindex");
+}
+
+void test_index_splitindex__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_index_splitindex__fail_on_open(void)
+{
+	git_index *idx;
+	cl_git_fail_with(-1, git_repository_index(&idx, g_repo));
+	cl_assert_equal_s(git_error_last()->message, "unsupported mandatory extension: 'link'");
+}
diff --git a/tests/resources/splitindex/.gitted/HEAD b/tests/resources/splitindex/.gitted/HEAD
new file mode 100644
index 000000000..cb089cd89
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/master
diff --git a/tests/resources/splitindex/.gitted/config b/tests/resources/splitindex/.gitted/config
new file mode 100644
index 000000000..e9d0b6d3b
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/config
@@ -0,0 +1,8 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+	splitIndex = true
diff --git a/tests/resources/splitindex/.gitted/index b/tests/resources/splitindex/.gitted/index
new file mode 100644
index 000000000..ff3448847
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/index
diff --git a/tests/resources/splitindex/.gitted/info/exclude b/tests/resources/splitindex/.gitted/info/exclude
new file mode 100644
index 000000000..a5196d1be
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/info/exclude
@@ -0,0 +1,6 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~
diff --git a/tests/resources/splitindex/.gitted/objects/.gitkeep b/tests/resources/splitindex/.gitted/objects/.gitkeep
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/objects/.gitkeep
diff --git a/tests/resources/splitindex/.gitted/refs/.gitkeep b/tests/resources/splitindex/.gitted/refs/.gitkeep
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/refs/.gitkeep
diff --git a/tests/resources/splitindex/.gitted/sharedindex.39d890139ee5356c7ef572216cebcd27aa41f9df b/tests/resources/splitindex/.gitted/sharedindex.39d890139ee5356c7ef572216cebcd27aa41f9df
new file mode 100644
index 000000000..3330d716f
--- /dev/null
+++ b/tests/resources/splitindex/.gitted/sharedindex.39d890139ee5356c7ef572216cebcd27aa41f9df