git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/4] parse_commit(): parse timestamp from end of line

From
Jeff King <peff@peff.net>
Date
Apr 27, 2023, 08:14 UTC
Message-ID
<20230427081409.GB1477912@coredump.intra.peff.net>
In-Reply-To
<20230427081330.GA1461786@coredump.intra.peff.net>

To find the committer timestamp, we parse left-to-right looking for the closing ">" of the email, and then expect the timestamp right after that. But we've seen some broken cases in the wild where this fails, but we _could_ find the timestamp with a little extra work. E.g.:

  Name <Name<email>> 123456789 -0500

This means that features that rely on the committer timestamp, like --since or --until, will treat the commit as happening at time 0 (i.e., 1970).

This is doubly confusing because the pretty-print parser learned to handle these in 03818a4a94 (split_ident: parse timestamp from end of line, 2013-10-14). So printing them via "git show", etc, makes everything look normal, but --until, etc are still broken (despite the fact that that commit explicitly mentioned --until!).

So let's use the same trick as 03818a4a94: find the end of the line, and parse back to the final ">". In theory we could use split_ident_line() here, but it's actually a bit more strict. In particular, it requires a valid time-zone token, too. That should be present, of course, but we wouldn't want to break --until for cases that are working currently.

We might want to teach split_ident_line() to become more lenient there, but it would require checking its many callers (since right now they can assume that if date_start is non-NULL, so is tz_start).

So for now we'll just reimplement the same trick in the commit parser.

The test is in t4212, which already covers similar cases, courtesy of 03818a4a94. We'll just adjust the broken commit to munge both the author and committer timestamps. Note that we could match (author|committer) here, but alternation can't be used portably in sed. Since we wouldn't expect to see ">" except as part of an ident line, we can just match that character on any line.

Signed-off-by: Jeff King <peff@peff.net>
---
 commit.c               | 24 ++++++++++++++++--------
 t/t4212-log-corrupt.sh |  7 ++++++-
 2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/commit.c b/commit.c
index 878b4473e4..04c20d9cc6 100644
--- a/commit.c
+++ b/commit.c
@@ -96,6 +96,7 @@ struct commit *lookup_commit_reference_by_name(const char *name)
 static timestamp_t parse_commit_date(const char *buf, const char *tail)
 {
 	const char *dateptr;
+	const char *eol;
 
 	if (buf + 6 >= tail)
 		return 0;
@@ -107,16 +108,23 @@ static timestamp_t parse_commit_date(const char *buf, const char *tail)
 		return 0;
 	if (memcmp(buf, "committer", 9))
 		return 0;
-	while (buf < tail && *buf++ != '>')
-		/* nada */;
-	if (buf >= tail)
+
+	/*
+	 * Jump to end-of-line so that we can walk backwards to find the
+	 * end-of-email ">". This is more forgiving of malformed cases
+	 * because unexpected characters tend to be in the name and email
+	 * fields.
+	 */
+	eol = memchr(buf, '\n', tail - buf);
+	if (!eol)
 		return 0;
-	dateptr = buf;
-	while (buf < tail && *buf++ != '\n')
-		/* nada */;
-	if (buf >= tail)
+	dateptr = eol;
+	while (dateptr > buf && dateptr[-1] != '>')
+		dateptr--;
+	if (dateptr == buf || dateptr == eol)
 		return 0;
-	/* dateptr < buf && buf[-1] == '\n', so parsing will stop at buf-1 */
+
+	/* dateptr < eol && *eol == '\n', so parsing will stop at eol */
 	return parse_timestamp(dateptr, NULL, 10);
 }
 
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index 8b5433ea74..af4b35ff56 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -9,7 +9,7 @@ test_expect_success 'setup' '
 	test_commit foo &&
 
 	git cat-file commit HEAD >ok.commit &&
-	sed "/^author /s/>/>-<>/" <ok.commit >broken_email.commit &&
+	sed "s/>/>-<>/" <ok.commit >broken_email.commit &&
 
 	git hash-object --literally -w -t commit broken_email.commit >broken_email.hash &&
 	git update-ref refs/heads/broken_email $(cat broken_email.hash)
@@ -44,6 +44,11 @@ test_expect_success 'git log --format with broken author email' '
 	test_must_be_empty actual.err
 '
 
+test_expect_success '--until handles broken email' '
+	git rev-list --until=1980-01-01 broken_email >actual &&
+	test_must_be_empty actual
+'
+
 munge_author_date () {
 	git cat-file commit "$1" >commit.orig &&
 	sed "s/^\(author .*>\) [0-9]*/\1 $2/" <commit.orig >commit.munge &&
-- 
2.40.1.663.g410c33770c.dirty
Previous: Jeff KingNext: Jeff King
Message 30 of 46 in “Weird behavior of 'git log --before' or 'git log --date-order': Commits from 2011 are treated to be before 1980”
  1. Thomas BockApr 14, 2023
  2. Jeff KingApr 15, 2023
  3. Jeff KingApr 15, 2023
  4. Kristoffer HaugsbakkApr 15, 2023
  5. Jeff KingApr 17, 2023
  6. Kristoffer HaugsbakkApr 17, 2023
  7. Jeff KingApr 17, 2023
  8. Kristoffer HaugsbakkApr 27, 2023
  9. Junio C HamanoApr 17, 2023
  10. Jeff KingApr 18, 2023
  11. Derrick StoleeApr 18, 2023
  12. Thomas BockApr 21, 2023
  13. 0/3 fixing some parse_commit() timestamp corner casesJeff King, Apr 22, 2023
  14. 1/3 t4212: avoid putting git on left-hand side of pipeJeff King, Apr 22, 2023
  15. 2/3 parse_commit(): parse timestamp from end of lineJeff King, Apr 22, 2023
  16. Junio C HamanoApr 24, 2023
  17. Jeff KingApr 25, 2023
  18. Junio C HamanoApr 24, 2023
  19. 0/3 fixing some parse_commit() timestamp corner casesJeff King, Apr 25, 2023
  20. Jeff KingApr 25, 2023
  21. 1/4 t4212: avoid putting git on left-hand side of pipeJeff King, Apr 25, 2023
  22. 2/4 parse_commit(): parse timestamp from end of lineJeff King, Apr 25, 2023
  23. 3/4 parse_commit(): handle broken whitespace-only timestampJeff King, Apr 25, 2023
  24. Phillip WoodApr 25, 2023
  25. Junio C HamanoApr 25, 2023
  26. Jeff KingApr 26, 2023
  27. Junio C HamanoApr 26, 2023
  28. 0/4 fixing some parse_commit() timestamp corner casesJeff King, Apr 27, 2023
  29. 1/4 t4212: avoid putting git on left-hand side of pipeJeff King, Apr 27, 2023
  30. 2/4 parse_commit(): parse timestamp from end of lineJeff King, Apr 27, 2023
  31. 3/4 parse_commit(): handle broken whitespace-only timestampJeff King, Apr 27, 2023
  32. Phillip WoodApr 27, 2023
  33. Phillip WoodApr 27, 2023
  34. Jeff KingApr 27, 2023
  35. Junio C HamanoApr 27, 2023
  36. Jeff KingApr 27, 2023
  37. Junio C HamanoApr 27, 2023
  38. Jeff KingApr 27, 2023
  39. 4/4 parse_commit(): describe more date-parsing failure modesJeff King, Apr 27, 2023
  40. Jeff KingApr 27, 2023
  41. Junio C HamanoApr 27, 2023
  42. Phillip WoodApr 26, 2023
  43. Andreas SchwabApr 26, 2023
  44. Phillip WoodApr 26, 2023
  45. 4/4 parse_commit(): describe more date-parsing failure modesJeff King, Apr 25, 2023
  46. Jeff KingApr 22, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.