@matheusbsbs / matheus-bagliano / commits / 67f16e3d3f

sequencer: fix "rebase -i --root" corrupting author header timestamp

When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timestamp by prepending a "@": author A U Thor <author@example.com> @1112912773 -0700 The commit parser is very strict about the format of the "author" header, and does not allow a "@" in that position. The "@" comes from GIT_AUTHOR_DATE in "rebase-merge/author-script", signifying a Unix epoch-based timestamp, however, read_author_ident() incorrectly allows it to slip into the commit's "author" header, thus corrupting it. One possible fix would be simply to filter out the "@" when constructing the "author" header timestamp, however, a more correct fix is to parse the GIT_AUTHOR_DATE date (via parse_date()) and format the parsed result into the "author" header. Since "rebase-merge/author-script" may be edited by the user, this approach has the extra benefit of catching other potential timestamp corruption due to hand-editing. We can do better than calling parse_date() ourselves and constructing the "author" header manually, however, by instead taking advantage of fmt_ident() which does this work for us. The benefits of using fmt_ident() are twofold. First, it simplifies the logic considerably by allowing us to avoid the complexity of building the "author" header in parallel with and in the same buffer from which "rebase-merge/author-script" is being parsed. Instead, fmt_ident() is invoked to compose the header after parsing is complete. Second, fmt_ident() is careful to prevent "crud" from polluting the composed ident. As with validating GIT_AUTHOR_DATE, this "crud" avoidance prevents other (possibly hand-edited) bogus author information from "rebase-merge/author-script" from corrupting the commit object. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Eric Sunshine committed Jul 31, 2018 at 03:33 UTC 67f16e3d3fbd1eccda5d1cdb33d04cd087162057

2 files changed +10 -15

sequencer.c

+9 -14

@@ -706,14 +706,16 @@ static const char *read_author_ident(struct strbuf *buf)

706 const char *keys[] = {

707 "GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="

708 };

709 - char *in, *out, *eol;

710 - int i = 0, len;

709 + struct strbuf out = STRBUF_INIT;

710 + char *in, *eol;

711 + const char *val[3];

712 + int i = 0;

713

714 if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)

715 return NULL;

716

717 /* dequote values and construct ident line in-place */

716 - for (in = out = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {

718 + for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {

719 if (!skip_prefix(in, keys[i], (const char **)&in)) {

720 warning("could not parse '%s' (looking for '%s'",

721 rebase_path_author_script(), keys[i]);

@@ -727,16 +729,7 @@ static const char *read_author_ident(struct strbuf *buf)

729 keys[i], rebase_path_author_script());

730 return NULL;

731 }

730 - len = strlen(in);

731 -

732 - if (i > 0) /* separate values by spaces */

733 - *(out++) = ' ';

734 - if (i == 1) /* email needs to be surrounded by <...> */

735 - *(out++) = '<';

736 - memmove(out, in, len);

737 - out += len;

738 - if (i == 1) /* email needs to be surrounded by <...> */

739 - *(out++) = '>';

732 + val[i] = in;

733 in = eol + 1;

734 }

735

@@ -746,7 +739,9 @@ static const char *read_author_ident(struct strbuf *buf)

739 return NULL;

740 }

741

749 - strbuf_setlen(buf, out - buf->buf);

742 + strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));

743 + strbuf_swap(buf, &out);

744 + strbuf_release(&out);

745 return buf->buf;

746 }

747

t/t3404-rebase-interactive.sh

+1 -1

@@ -1384,7 +1384,7 @@ test_expect_success 'valid author header after --root swap' '

1384 set_fake_editor &&

1385 FAKE_LINES="2 1" git rebase -i --root &&

1386 git cat-file commit HEAD^ >out &&

1387 - grep "^author ..*> @[0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$" out

1387 + grep "^author ..*> [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$" out

1388 '

1389

1390 test_done