Skip to content

Reflog/Parse skip newlines at the end of last message. - #5266

Closed
lolgear wants to merge 8 commits into
libgit2:masterfrom
lolgear:reflog/parse_skip_newlines_at_the_end_of_last_message
Closed

Reflog/Parse skip newlines at the end of last message.#5266
lolgear wants to merge 8 commits into
libgit2:masterfrom
lolgear:reflog/parse_skip_newlines_at_the_end_of_last_message

Conversation

@lolgear

@lolgear lolgear commented Oct 13, 2019

Copy link
Copy Markdown
Contributor

It is done by seek forward to a character that following after invalid character.

if (get_oid_old < 0) {
  // get invalid character position
  int position = get_position();
  seek_forward(position + 1);
}

@lolgear lolgear mentioned this pull request Oct 13, 2019
Open
@pks-t

pks-t commented Oct 18, 2019

Copy link
Copy Markdown
Member

Thanks a lot for your pull request! I've taken a deeper look at the code to try to find out the root cause. The issue is that we do not sanitize entries that we're adding to the reflog ".git/logs/refs/stash". As a result, if the user specifies multi-line reflog messages, the reflog will end up with these exact newlines and thus break the format.

So two things that need to be done:

  1. libgit2 needs to sanitize reflog messages in all places properly
  2. We should just skip over non-parseable lines in the reflog. Git itself does so, we should do the same.

I'll open a PR in a few minutes that tackles both these issues. I hope you don't mind me effectively taking over the issue, but I think that the deeper underlying issues need to be fixed. Anyway, thanks a lot for bringing this to our attention and proposing a fix!

@pks-t pks-t mentioned this pull request Oct 18, 2019
Merged
@pks-t

pks-t commented Oct 18, 2019

Copy link
Copy Markdown
Member

Please see #5275

@lolgear lolgear mentioned this pull request Nov 2, 2019
Merged
2 tasks
@ethomson

ethomson commented Nov 2, 2019

Copy link
Copy Markdown
Member

Closed via #5275. Thanks for reporting this issue and the collaboration to fix it.

@lolgear lolgear closed this Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lolgear @pks-t @ethomson