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

[PATCH v2] reflog-walk: don't segfault on non-commit sha1's in the reflog

From
Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date
Dec 30, 2015, 22:17 UTC
Message-ID
<20151230221705.GA4025@spirit>
In-Reply-To
<1451512152.9251.23.camel@kaarsemaker.net>

Use lookup_commit instead of parse_object to look up commits mentioned in the reflog. This avoids a segfault in save_parents if somehow a sha1 for something other than a commit ends up in the reflog.

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
---
 reflog-walk.c     |  7 +++++--
 t/t1410-reflog.sh | 13 +++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

This version implements Junio's way of doing this, to avoid spurious warnings and to handle broken commit objects better.

I also added a failing test to show that git reflog still fails to display the full reflog when it encounters such non-commit objects (it just stops when it seems them). I don't know how to fix that.

The really correct way of fixing this bug may actually be a level higher, and making git reflog not rely on information about parent commits, whether they are fake or not.

diff --git a/reflog-walk.c b/reflog-walk.c
index 85b8a54..861d7c4 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -221,6 +221,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	struct commit_info *commit_info =
 		get_commit_info(commit, &info->reflogs, 0);
 	struct commit_reflog *commit_reflog;
+	struct object *logobj;
 	struct reflog_info *reflog;
 
 	info->last_commit_reflog = NULL;
@@ -236,11 +237,13 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
-	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
-	if (!commit_info->commit) {
+	logobj = parse_object(reflog->osha1);
+	if (!logobj || logobj->type != OBJ_COMMIT) {
+		commit_info->commit = NULL;
 		commit->parents = NULL;
 		return;
 	}
+	commit_info->commit = (struct commit *)logobj;
 
 	commit->parents = xcalloc(1, sizeof(struct commit_list));
 	commit->parents->item = commit_info->commit;
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index b79049f..130d671 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -325,4 +325,17 @@ test_expect_success 'parsing reverse reflogs at BUFSIZ boundaries' '
 	test_cmp expect actual
 '
 
+test_expect_success 'no segfaults for reflog containing non-commit sha1s' '
+	git update-ref --create-reflog -m "Creating ref" \
+		refs/tests/tree-in-reflog HEAD &&
+	git update-ref -m "Forcing tree" refs/tests/tree-in-reflog HEAD^{tree} &&
+	git update-ref -m "Restoring to commit" refs/tests/tree-in-reflog HEAD &&
+	git reflog refs/tests/tree-in-reflog
+'
+
+test_expect_failure 'reflog containing non-commit sha1s displays fully' '
+	git reflog refs/tests/tree-in-reflog > actual &&
+	test_line_count = 3 actual
+'
+
 test_done
-- 
2.7.0-rc1-207-ga35084c
Previous: Dennis KaarsemakerNext: Junio C Hamano
Message 13 of 25 in “Segfault in git reflog”
  1. Dennis KaarsemakerDec 30, 2015
  2. Duy NguyenDec 30, 2015
  3. Dennis KaarsemakerDec 30, 2015
  4. Duy NguyenDec 30, 2015
  5. Duy NguyenDec 30, 2015
  6. Dennis KaarsemakerDec 30, 2015
  7. Duy NguyenDec 30, 2015
  8. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  9. Junio C HamanoDec 30, 2015
  10. Dennis KaarsemakerDec 30, 2015
  11. Junio C HamanoDec 30, 2015
  12. Dennis KaarsemakerDec 30, 2015
  13. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  14. Junio C HamanoDec 30, 2015
  15. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  16. Junio C HamanoDec 31, 2015
  17. Dennis KaarsemakerDec 31, 2015
  18. Dennis KaarsemakerDec 31, 2015
  19. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Jan 5, 2016
  20. Eric SunshineJan 6, 2016
  21. Dennis KaarsemakerJan 6, 2016
  22. Eric SunshineJan 6, 2016
  23. Eric SunshineJan 6, 2016
  24. Dennis KaarsemakerJan 6, 2016
  25. Duy NguyenJan 6, 2016

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.