Skip to content

Commit

Permalink
reflog: allow adding entries with newlines in their message
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pks-t committed Oct 18, 2019
1 parent 2848160 commit d8233fe
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 16 deletions.
7 changes: 7 additions & 0 deletions src/refdb_fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
24 changes: 11 additions & 13 deletions src/reflog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions tests/refs/reflog/messages.c
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]"));
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)
{
Expand Down
4 changes: 1 addition & 3 deletions tests/refs/reflog/reflog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
20 changes: 20 additions & 0 deletions tests/stash/save.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit d8233fe

Please sign in to comment.