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.

Author: pks-t

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)

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