Commit 7968e90f79d4c209f26d6a6f48db2137b168906e
2019-10-18T12:33:07
refdb_fs: properly parse corrupted reflogs In previous versions, libgit2 could be coerced into writing reflog messages with embedded newlines into the reflog by using `git_stash_save` with a message containing newlines. While the root cause is fixed now, it was noticed that upstream git is in fact able to read such corrupted reflog messages just fine. Make the reflog parser more lenient in order to just skip over malformatted reflog lines to bring us in line with git. This requires us to change an existing test that verified that we do indeed _fail_ to parse such logs.
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
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index 1bf300c..77b72dc 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1653,32 +1653,31 @@ static int reflog_alloc(git_reflog **reflog, const char *name)
static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
{
git_parse_ctx parser = GIT_PARSE_CTX_INIT;
- git_reflog_entry *entry = NULL;
if ((git_parse_ctx_init(&parser, buf, buf_size)) < 0)
return -1;
for (; parser.remain_len; git_parse_advance_line(&parser)) {
+ git_reflog_entry *entry;
const char *sig;
char c;
entry = git__calloc(1, sizeof(*entry));
GIT_ERROR_CHECK_ALLOC(entry);
-
entry->committer = git__calloc(1, sizeof(*entry->committer));
GIT_ERROR_CHECK_ALLOC(entry->committer);
if (git_parse_advance_oid(&entry->oid_old, &parser) < 0 ||
git_parse_advance_expected(&parser, " ", 1) < 0 ||
git_parse_advance_oid(&entry->oid_cur, &parser) < 0)
- goto error;
+ goto next;
sig = parser.line;
while (git_parse_peek(&c, &parser, 0) == 0 && c != '\t' && c != '\n')
git_parse_advance_chars(&parser, 1);
if (git_signature__parse(entry->committer, &sig, parser.line, NULL, 0) < 0)
- goto error;
+ goto next;
if (c == '\t') {
size_t len;
@@ -1692,16 +1691,18 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
GIT_ERROR_CHECK_ALLOC(entry->msg);
}
- if (git_vector_insert(&log->entries, entry) < 0)
- goto error;
- }
+ if ((git_vector_insert(&log->entries, entry)) < 0) {
+ git_reflog_entry__free(entry);
+ return -1;
+ }
- return 0;
+ continue;
-error:
- git_reflog_entry__free(entry);
+next:
+ git_reflog_entry__free(entry);
+ }
- return -1;
+ return 0;
}
static int create_new_reflog_file(const char *filepath)
diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c
index 9bd1af4..5cefc32 100644
--- a/tests/refs/reflog/reflog.c
+++ b/tests/refs/reflog/reflog.c
@@ -222,45 +222,45 @@ void test_refs_reflog_reflog__reading_the_reflog_from_a_reference_with_no_log_re
git_buf_dispose(&subtrees_log_path);
}
-void test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_returns_error(void)
+void test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_succeeds(void)
{
git_reflog *reflog;
- const git_error *error;
const char *refname = "refs/heads/newline";
const char *refmessage =
"Reflog*message with a newline and enough content after it to pass the GIT_REFLOG_SIZE_MIN check inside reflog_parse.";
+ const git_reflog_entry *entry;
git_reference *ref;
git_oid id;
git_buf logpath = GIT_BUF_INIT, logcontents = GIT_BUF_INIT;
char *star;
- git_oid_fromstr(&id, current_master_tip);
-
- /* create a new branch */
+ /* Create a new branch. */
+ cl_git_pass(git_oid_fromstr(&id, current_master_tip));
cl_git_pass(git_reference_create(&ref, g_repo, refname, &id, 1, refmessage));
- /* corrupt the branch reflog by introducing a newline inside the reflog message (we replace '*' with '\n') */
- git_buf_join_n(&logpath, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, refname);
+ /*
+ * Corrupt the branch reflog by introducing a newline inside the reflog message.
+ * We do this by replacing '*' with '\n'
+ */
+ cl_git_pass(git_buf_join_n(&logpath, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, refname));
cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath)));
cl_assert((star = strchr(git_buf_cstr(&logcontents), '*')) != NULL);
*star = '\n';
cl_git_rewritefile(git_buf_cstr(&logpath), git_buf_cstr(&logcontents));
- /* confirm that the file was rewritten successfully and now contains a '\n' in the expected location */
+ /*
+ * Confirm that the file was rewritten successfully
+ * and now contains a '\n' in the expected location
+ */
cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath)));
cl_assert(strstr(git_buf_cstr(&logcontents), "Reflog\nmessage") != NULL);
- /* clear the error state so we can capture the error generated by git_reflog_read */
- git_error_clear();
-
- cl_git_fail(git_reflog_read(&reflog, g_repo, refname));
-
- error = git_error_last();
-
- cl_assert(error != NULL);
- cl_assert_equal_s("unable to parse OID - contains invalid characters", error->message);
+ cl_git_pass(git_reflog_read(&reflog, g_repo, refname));
+ cl_assert(entry = git_reflog_entry_byindex(reflog, 0));
+ cl_assert_equal_s(git_reflog_entry_message(entry), "Reflog");
git_reference_free(ref);
+ git_reflog_free(reflog);
git_buf_dispose(&logpath);
git_buf_dispose(&logcontents);
}