Commit d8233feb78d10ade39fd64b85044fea84f2fc293

Patrick Steinhardt 2019-10-18T09:24:14

reflog: allow adding entries with newlines in their message Currently, the reflog disallows any entries that have a message with newlines, as that would effectively break the reflog format, which may contain a single line per entry, only. Upstream git behaves a bit differently, though, especially when considering stashes: instead of rejecting any reflog entry with newlines, git will simply replace newlines with spaces. E.g. executing 'git stash push -m "foo\nbar"' will create a reflog entry with "foo bar" as entry message. This commit adjusts our own logic to stop rejecting commit messages with newlines. Previously, this logic was part of `git_reflog_append`, only. There is a second place though where we add reflog entries, which is the serialization code in the filesystem refdb. As it didn't contain any sanity checks whatsoever, the refdb would have been perfectly happy to write malformatted reflog entries to the disk. This is being fixed with the same logic as for the reflog itself.

diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index c48afb9..449dedf 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1856,8 +1856,15 @@ static int serialize_reflog_entry(
 	git_buf_rtrim(buf);
 
 	if (msg) {
+		size_t i;
+
 		git_buf_putc(buf, '\t');
 		git_buf_puts(buf, msg);
+
+		for (i = 0; i < buf->size - 2; i++)
+			if (buf->ptr[i] == '\n')
+				buf->ptr[i] = ' ';
+		git_buf_rtrim(buf);
 	}
 
 	git_buf_putc(buf, '\n');
diff --git a/src/reflog.c b/src/reflog.c
index 1834a27..24dada0 100644
--- a/src/reflog.c
+++ b/src/reflog.c
@@ -74,9 +74,8 @@ int git_reflog_write(git_reflog *reflog)
 
 int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_signature *committer, const char *msg)
 {
-	git_reflog_entry *entry;
 	const git_reflog_entry *previous;
-	const char *newline;
+	git_reflog_entry *entry;
 
 	assert(reflog && new_oid && committer);
 
@@ -87,19 +86,18 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_sign
 		goto cleanup;
 
 	if (msg != NULL) {
-		if ((entry->msg = git__strdup(msg)) == NULL)
-			goto cleanup;
+		size_t i, msglen = strlen(msg);
 
-		newline = strchr(msg, '\n');
-
-		if (newline) {
-			if (newline[1] != '\0') {
-				git_error_set(GIT_ERROR_INVALID, "reflog message cannot contain newline");
-				goto cleanup;
-			}
+		if ((entry->msg = git__strndup(msg, msglen)) == NULL)
+			goto cleanup;
 
-			entry->msg[newline - msg] = '\0';
-		}
+		/*
+		 * Replace all newlines with spaces, except for
+		 * the final trailing newline.
+		 */
+		for (i = 0; i < msglen; i++)
+			if (entry->msg[i] == '\n')
+				entry->msg[i] = ' ';
 	}
 
 	previous = git_reflog_entry_byindex(reflog, 0);
diff --git a/tests/refs/reflog/messages.c b/tests/refs/reflog/messages.c
index f8acd23..43f59a8 100644
--- a/tests/refs/reflog/messages.c
+++ b/tests/refs/reflog/messages.c
@@ -281,6 +281,27 @@ void test_refs_reflog_messages__creating_a_direct_reference(void)
 	git_reference_free(reference);
 }
 
+void test_refs_reflog_messages__newline_gets_replaced(void)
+{
+	const git_reflog_entry *entry;
+	git_signature *signature;
+	git_reflog *reflog;
+	git_oid oid;
+
+	cl_git_pass(git_signature_now(&signature, "me", "foo@example.com"));
+	cl_git_pass(git_oid_fromstr(&oid, "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"));
+
+	cl_git_pass(git_reflog_read(&reflog, g_repo, "HEAD"));
+	cl_assert_equal_sz(7, git_reflog_entrycount(reflog));
+	cl_git_pass(git_reflog_append(reflog, &oid, signature, "inner\nnewline"));
+	cl_assert_equal_sz(8, git_reflog_entrycount(reflog));
+
+	cl_assert(entry = git_reflog_entry_byindex(reflog, 0));
+	cl_assert_equal_s(git_reflog_entry_message(entry), "inner newline");
+
+	git_signature_free(signature);
+	git_reflog_free(reflog);
+}
 
 void test_refs_reflog_messages__renaming_ref(void)
 {
diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c
index 7e4b1ef..9bd1af4 100644
--- a/tests/refs/reflog/reflog.c
+++ b/tests/refs/reflog/reflog.c
@@ -87,15 +87,13 @@ void test_refs_reflog_reflog__append_then_read(void)
 	cl_git_pass(git_signature_now(&committer, "foo", "foo@bar"));
 
 	cl_git_pass(git_reflog_read(&reflog, g_repo, new_ref));
-
-	cl_git_fail(git_reflog_append(reflog, &oid, committer, "no inner\nnewline"));
 	cl_git_pass(git_reflog_append(reflog, &oid, committer, NULL));
 	cl_git_pass(git_reflog_append(reflog, &oid, committer, commit_msg "\n"));
 	cl_git_pass(git_reflog_write(reflog));
-	git_reflog_free(reflog);
 
 	assert_appends(committer, &oid);
 
+	git_reflog_free(reflog);
 	git_signature_free(committer);
 }
 
diff --git a/tests/stash/save.c b/tests/stash/save.c
index 362c704..d568567 100644
--- a/tests/stash/save.c
+++ b/tests/stash/save.c
@@ -283,6 +283,26 @@ void test_stash_save__stashing_updates_the_reflog(void)
 	assert_object_oid("refs/stash@{1}", NULL, GIT_OBJECT_COMMIT);
 }
 
+void test_stash_save__multiline_message(void)
+{
+	const char *msg = "This\n\nis a multiline message\n";
+	const git_reflog_entry *entry;
+	git_reflog *reflog;
+
+	assert_object_oid("refs/stash@{0}", NULL, GIT_OBJECT_COMMIT);
+
+	cl_git_pass(git_stash_save(&stash_tip_oid, repo, signature, msg, GIT_STASH_DEFAULT));
+
+	cl_git_pass(git_reflog_read(&reflog, repo, "refs/stash"));
+	cl_assert(entry = git_reflog_entry_byindex(reflog, 0));
+	cl_assert_equal_s(git_reflog_entry_message(entry), "On master: This  is a multiline message");
+
+	assert_object_oid("refs/stash@{0}", git_oid_tostr_s(&stash_tip_oid), GIT_OBJECT_COMMIT);
+	assert_commit_message_contains("refs/stash@{0}", msg);
+
+	git_reflog_free(reflog);
+}
+
 void test_stash_save__cannot_stash_when_there_are_no_local_change(void)
 {
 	git_index *index;