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

index: preserve extension parsing errors

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`.

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;
 		}
 

tests/index/splitindex.c

+#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'");
+}

tests/resources/splitindex/.gitted/HEAD

+ref: refs/heads/master

tests/resources/splitindex/.gitted/config

+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = true
+	precomposeunicode = true
+	splitIndex = true

tests/resources/splitindex/.gitted/info/exclude

+# 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]
+# *~