sequencer: fix "rebase -i --root" corrupting author header timestamp
authorEric Sunshine <sunshine@sunshineco.com>
Tue, 31 Jul 2018 07:33:30 +0000 (03:33 -0400)
committerJunio C Hamano <gitster@pobox.com>
Tue, 31 Jul 2018 18:32:12 +0000 (11:32 -0700)
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>
sequencer.c
t/t3404-rebase-interactive.sh
index 2b66b85373a46bfaf6a3b0b5bc1af145b4fa98b8..580516959dac54aadec12a24b55d92c680ed090d 100644 (file)
@@ -706,14 +706,16 @@ static const char *read_author_ident(struct strbuf *buf)
        const char *keys[] = {
                "GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="
        };
-       char *in, *out, *eol;
-       int i = 0, len;
+       struct strbuf out = STRBUF_INIT;
+       char *in, *eol;
+       const char *val[3];
+       int i = 0;
 
        if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)
                return NULL;
 
        /* dequote values and construct ident line in-place */
-       for (in = out = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
+       for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
                if (!skip_prefix(in, keys[i], (const char **)&in)) {
                        warning("could not parse '%s' (looking for '%s'",
                                rebase_path_author_script(), keys[i]);
@@ -727,16 +729,7 @@ static const char *read_author_ident(struct strbuf *buf)
                                keys[i], rebase_path_author_script());
                        return NULL;
                }
-               len = strlen(in);
-
-               if (i > 0) /* separate values by spaces */
-                       *(out++) = ' ';
-               if (i == 1) /* email needs to be surrounded by <...> */
-                       *(out++) = '<';
-               memmove(out, in, len);
-               out += len;
-               if (i == 1) /* email needs to be surrounded by <...> */
-                       *(out++) = '>';
+               val[i] = in;
                in = eol + 1;
        }
 
@@ -746,7 +739,9 @@ static const char *read_author_ident(struct strbuf *buf)
                return NULL;
        }
 
-       strbuf_setlen(buf, out - buf->buf);
+       strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));
+       strbuf_swap(buf, &out);
+       strbuf_release(&out);
        return buf->buf;
 }
 
index 092139096eef048b4abd55f91f0b60146150a3db..b72167ecd5068648d11de4b0279a27cd5a3eb84a 100755 (executable)
@@ -1384,7 +1384,7 @@ test_expect_success 'valid author header after --root swap' '
        set_fake_editor &&
        FAKE_LINES="2 1" git rebase -i --root &&
        git cat-file commit HEAD^ >out &&
-       grep "^author ..*> @[0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$" out
+       grep "^author ..*> [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$" out
 '
 
 test_done