index: preserve extension parsing errors Previously, we would clobber any extension-specific error message with an "extension is truncated" message. This makes `read_extension` correctly preserve those errors, takes responsibility for truncation errors, and adds a new message with the actual extension signature for unsupported mandatory extensions.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171
diff --git a/docs/changelog.md b/docs/changelog.md
index 26899a5..bf31ec9 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 677faea..8b75323 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 0000000..d32ed10
--- /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 0000000..cb089cd
--- /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 0000000..e9d0b6d
--- /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 0000000..ff34488
Binary files /dev/null and b/tests/resources/splitindex/.gitted/index differ
diff --git a/tests/resources/splitindex/.gitted/info/exclude b/tests/resources/splitindex/.gitted/info/exclude
new file mode 100644
index 0000000..a5196d1
--- /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 0000000..e69de29
--- /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 0000000..e69de29
--- /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 0000000..3330d71
Binary files /dev/null and b/tests/resources/splitindex/.gitted/sharedindex.39d890139ee5356c7ef572216cebcd27aa41f9df differ