fix(commit): read tree and parents the way git does (GO-005)

The tree must be the first header, else ErrCorrupt. Only the parent lines
that directly follow it are parents; later tree and parent lines are
ordinary headers and are ignored, as in git log --format=%P. The object path
now agrees with the commit-graph path. Conformance tests against hostile
commits written with hash-object --literally, and fuzz seeds.

commit: b1a375f22bbe2324dc83d057a14a032aaa8f5226
author: brice · 11d ago · 2026-10-10 22:15 UTC
parent: f0b398e
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +129 -24

M commit.go +35 -18


@@ -63,16 +63,25 @@ func (r Repo) ReadCommit(id ObjectID, c Commit) error {
63	63		}
64	64
65	65		// ParseCommit parses the content of a commit object into c. The fields of c
66		-	// point into data, which must stay unchanged while they are used. It returns
67		-	// ErrCorrupt if the commit has no valid tree or a malformed parent.
	66	+	// point into data, which must stay unchanged while they are used.
	67	+	//
	68	+	// It reads headers the way Git does: the first line must be the tree, the
	69	+	// parent lines are the ones that immediately follow it, and any tree or
	70	+	// parent line further down is an ordinary, ignored header. It returns
	71	+	// ErrCorrupt if the first line is not a valid tree or a parent is malformed.
68	72		func ParseCommit(data []byte, c *Commit) error {
69	73		c.ID = ObjectID{}
70	74		c.Parents = c.Parents[:0]
71	75		c.Author, c.Committer = Signature{}, Signature{}
72	76		c.Encoding, c.Message = nil, nil
73		-	haveTree := false
	77	+	const (
	78	+	stTree = iota
	79	+	stParents
	80	+	stRest
	81	+	)
	82	+	state := stTree
74	83		for len(data) > 0 {
75		-	if data[0] == '\n' {
	84	+	if data[0] == '\n' && state != stTree {
76	85		c.Message = data[1:]
77	86		break
78	87		}

@@ -82,23 +91,31 @@ func ParseCommit(data []byte, c *Commit) error {
82	91		} else {
83	92		line, data = data, nil
84	93		}
85		-	if line[0] == ' ' {
86		-	continue // continuation of a multi-line header such as gpgsig
87		-	}
88	94		key, val, _ := bytes.Cut(line, []byte{' '})
89		-	switch string(key) {
90		-	case "tree":
	95	+	switch state {
	96	+	case stTree:
91	97		id, ok := parseIDBytes(val)
92		-	if !ok \|\| haveTree {
93		-	return corrupt("commit: bad tree header")
	98	+	if string(key) != "tree" \|\| !ok {
	99	+	return corrupt("commit: the first header is not a valid tree")
94	100		}
95		-	c.Tree, haveTree = id, true
96		-	case "parent":
97		-	id, ok := parseIDBytes(val)
98		-	if !ok {
99		-	return corrupt("commit: bad parent header")
	101	+	c.Tree = id
	102	+	state = stParents
	103	+	continue
	104	+	case stParents:
	105	+	if string(key) == "parent" {
	106	+	id, ok := parseIDBytes(val)
	107	+	if !ok {
	108	+	return corrupt("commit: bad parent header")
	109	+	}
	110	+	c.Parents = append(c.Parents, id)
	111	+	continue
100	112		}
101		-	c.Parents = append(c.Parents, id)
	113	+	state = stRest
	114	+	}
	115	+	if line[0] == ' ' {
	116	+	continue // continuation of a multi-line header such as gpgsig
	117	+	}
	118	+	switch string(key) {
102	119		case "author":
103	120		c.Author = parseSignature(val)
104	121		case "committer":

@@ -107,7 +124,7 @@ func ParseCommit(data []byte, c *Commit) error {
107	124		c.Encoding = val
108	125		}
109	126		}
110		-	if !haveTree {
	127	+	if state == stTree {
111	128		return corrupt("commit: no tree")
112	129		}
113	130		return nil

 }
 // ParseCommit parses the content of a commit object into c. The fields of c
-// point into data, which must stay unchanged while they are used. It returns
+// point into data, which must stay unchanged while they are used.
-// ErrCorrupt if the commit has no valid tree or a malformed parent.
+//
+// It reads headers the way Git does: the first line must be the tree, the
+// parent lines are the ones that immediately follow it, and any tree or
+// parent line further down is an ordinary, ignored header. It returns
+// ErrCorrupt if the first line is not a valid tree or a parent is malformed.
 func ParseCommit(data []byte, c *Commit) error {
 	c.ID = ObjectID{}
 	c.Parents = c.Parents[:0]
 	c.Author, c.Committer = Signature{}, Signature{}
 	c.Encoding, c.Message = nil, nil
-	haveTree := false
+	const (
+		stTree = iota
+		stParents
+		stRest
+	)
+	state := stTree
 	for len(data) > 0 {
-		if data[0] == '\n' {
+		if data[0] == '\n' && state != stTree {
 			c.Message = data[1:]
 			break
 		}
 		} else {
 			line, data = data, nil
 		}
-		if line[0] == ' ' {
-			continue // continuation of a multi-line header such as gpgsig
-		}
 		key, val, _ := bytes.Cut(line, []byte{' '})
-		switch string(key) {
+		switch state {
-		case "tree":
+		case stTree:
 			id, ok := parseIDBytes(val)
-			if !ok || haveTree {
+			if string(key) != "tree" || !ok {
-				return corrupt("commit: bad tree header")
+				return corrupt("commit: the first header is not a valid tree")
 			}
-			c.Tree, haveTree = id, true
+			c.Tree = id
-		case "parent":
+			state = stParents
-			id, ok := parseIDBytes(val)
+			continue
-			if !ok {
+		case stParents:
-				return corrupt("commit: bad parent header")
+			if string(key) == "parent" {
+				id, ok := parseIDBytes(val)
+				if !ok {
+					return corrupt("commit: bad parent header")
+				}
+				c.Parents = append(c.Parents, id)
+				continue
 			}
-			c.Parents = append(c.Parents, id)
+			state = stRest
+		}
+		if line[0] == ' ' {
+			continue // continuation of a multi-line header such as gpgsig
+		}
+		switch string(key) {
 		case "author":
 			c.Author = parseSignature(val)
 		case "committer":
 			c.Encoding = val
 		}
 	}
-	if !haveTree {
+	if state == stTree {
 		return corrupt("commit: no tree")
 	}
 	return nil

A commit_hostile_test.go +68 -0

@@ -0,0 +1,68 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package git
	4	+
	5	+	import (
	6	+	"errors"
	7	+	"strings"
	8	+	"testing"
	9	+	)
	10	+
	11	+	// Headers after the first line are read the way Git reads them: the tree
	12	+	// must come first, the parents follow it directly, and later tree or parent
	13	+	// lines are ordinary headers that nothing uses.
	14	+	func TestParseCommitHostileHeadersMatchGit(t *testing.T) {
	15	+	f := newFx(t)
	16	+	f.write("a", []byte("a"), 0o644)
	17	+	p1 := f.commit("one")
	18	+	f.write("a", []byte("b"), 0o644)
	19	+	p2 := f.commit("two")
	20	+	tree := f.git("rev-parse", "HEAD^{tree}")
	21	+	sig := "author A <a@x> 1 +0000\ncommitter C <c@x> 2 +0000\n"
	22	+	cases := []struct {
	23	+	name, body string
	24	+	corrupt bool
	25	+	}{
	26	+	{"second tree", "tree " + tree + "\ntree " + p1 + "\nparent " + p1 + "\n" + sig + "\nmsg\n", false},
	27	+	{"parents then tree", "tree " + tree + "\nparent " + p1 + "\nparent " + p2 + "\ntree " + p1 + "\n" + sig + "\nmsg\n", false},
	28	+	{"parent after author", "tree " + tree + "\nparent " + p1 + "\n" + sig + "parent " + p2 + "\n\nmsg\n", false},
	29	+	{"parent after other header", "tree " + tree + "\n" + "author A <a@x> 1 +0000\nparent " + p1 + "\ncommitter C <c@x> 2 +0000\n\nmsg\n", false},
	30	+	{"bad parent after author", "tree " + tree + "\n" + sig + "parent zzz\n\nmsg\n", false},
	31	+	{"tree after author", "author A <a@x> 1 +0000\ntree " + tree + "\n\nmsg\n", true},
	32	+	{"parent before tree", "parent " + p1 + "\ntree " + tree + "\n" + sig + "\nmsg\n", true},
	33	+	{"no tree", sig + "\nmsg\n", true},
	34	+	{"bad parent", "tree " + tree + "\nparent zzz\n" + sig + "\nmsg\n", true},
	35	+	{"parent with junk", "tree " + tree + "\nparent " + p1 + " junk\n" + sig + "\nmsg\n", true},
	36	+	}
	37	+	r := f.open()
	38	+	for _, c := range cases {
	39	+	t.Run(c.name, func(t *testing.T) {
	40	+	id := mustID(t, f.gitIn([]byte(c.body), "hash-object", "-t", "commit", "--literally", "-w", "--stdin"))
	41	+	var got Commit
	42	+	err := r.ReadCommit(id, &got)
	43	+	want, gerr := f.gitErr("log", "-1", "--format=%P %T", id.String())
	44	+	if c.corrupt {
	45	+	if gerr == nil {
	46	+	t.Fatalf("git accepts it: %q", want)
	47	+	}
	48	+	if !errors.Is(err, ErrCorrupt) {
	49	+	t.Fatalf("ReadCommit = %v, want ErrCorrupt", err)
	50	+	}
	51	+	return
	52	+	}
	53	+	if gerr != nil {
	54	+	t.Fatalf("git rejects it: %v", gerr)
	55	+	}
	56	+	if err != nil {
	57	+	t.Fatal(err)
	58	+	}
	59	+	var ps []string
	60	+	for _, p := range got.Parents {
	61	+	ps = append(ps, p.String())
	62	+	}
	63	+	if have := strings.TrimSpace(strings.Join(ps, " ") + " " + got.Tree.String()); have != strings.TrimSpace(want) {
	64	+	t.Errorf("parents and tree = %q, git says %q", have, strings.TrimSpace(want))
	65	+	}
	66	+	})
	67	+	}
	68	+	}

@@ -0,0 +1,68 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package git
	4	+
	5	+	import (
	6	+	"errors"
	7	+	"strings"
	8	+	"testing"
	9	+	)
	10	+
	11	+	// Headers after the first line are read the way Git reads them: the tree
	12	+	// must come first, the parents follow it directly, and later tree or parent
	13	+	// lines are ordinary headers that nothing uses.
	14	+	func TestParseCommitHostileHeadersMatchGit(t *testing.T) {
	15	+	f := newFx(t)
	16	+	f.write("a", []byte("a"), 0o644)
	17	+	p1 := f.commit("one")
	18	+	f.write("a", []byte("b"), 0o644)
	19	+	p2 := f.commit("two")
	20	+	tree := f.git("rev-parse", "HEAD^{tree}")
	21	+	sig := "author A <a@x> 1 +0000\ncommitter C <c@x> 2 +0000\n"
	22	+	cases := []struct {
	23	+	name, body string
	24	+	corrupt bool
	25	+	}{
	26	+	{"second tree", "tree " + tree + "\ntree " + p1 + "\nparent " + p1 + "\n" + sig + "\nmsg\n", false},
	27	+	{"parents then tree", "tree " + tree + "\nparent " + p1 + "\nparent " + p2 + "\ntree " + p1 + "\n" + sig + "\nmsg\n", false},
	28	+	{"parent after author", "tree " + tree + "\nparent " + p1 + "\n" + sig + "parent " + p2 + "\n\nmsg\n", false},
	29	+	{"parent after other header", "tree " + tree + "\n" + "author A <a@x> 1 +0000\nparent " + p1 + "\ncommitter C <c@x> 2 +0000\n\nmsg\n", false},
	30	+	{"bad parent after author", "tree " + tree + "\n" + sig + "parent zzz\n\nmsg\n", false},
	31	+	{"tree after author", "author A <a@x> 1 +0000\ntree " + tree + "\n\nmsg\n", true},
	32	+	{"parent before tree", "parent " + p1 + "\ntree " + tree + "\n" + sig + "\nmsg\n", true},
	33	+	{"no tree", sig + "\nmsg\n", true},
	34	+	{"bad parent", "tree " + tree + "\nparent zzz\n" + sig + "\nmsg\n", true},
	35	+	{"parent with junk", "tree " + tree + "\nparent " + p1 + " junk\n" + sig + "\nmsg\n", true},
	36	+	}
	37	+	r := f.open()
	38	+	for _, c := range cases {
	39	+	t.Run(c.name, func(t *testing.T) {
	40	+	id := mustID(t, f.gitIn([]byte(c.body), "hash-object", "-t", "commit", "--literally", "-w", "--stdin"))
	41	+	var got Commit
	42	+	err := r.ReadCommit(id, &got)
	43	+	want, gerr := f.gitErr("log", "-1", "--format=%P %T", id.String())
	44	+	if c.corrupt {
	45	+	if gerr == nil {
	46	+	t.Fatalf("git accepts it: %q", want)
	47	+	}
	48	+	if !errors.Is(err, ErrCorrupt) {
	49	+	t.Fatalf("ReadCommit = %v, want ErrCorrupt", err)
	50	+	}
	51	+	return
	52	+	}
	53	+	if gerr != nil {
	54	+	t.Fatalf("git rejects it: %v", gerr)
	55	+	}
	56	+	if err != nil {
	57	+	t.Fatal(err)
	58	+	}
	59	+	var ps []string
	60	+	for _, p := range got.Parents {
	61	+	ps = append(ps, p.String())
	62	+	}
	63	+	if have := strings.TrimSpace(strings.Join(ps, " ") + " " + got.Tree.String()); have != strings.TrimSpace(want) {
	64	+	t.Errorf("parents and tree = %q, git says %q", have, strings.TrimSpace(want))
	65	+	}
	66	+	})
	67	+	}
	68	+	}

M fuzz_test.go +17 -0


@@ -93,6 +93,9 @@ func FuzzParseCommit(f *testing.F) {
93	93		f.Add([]byte("tree " + tree))
94	94		f.Add([]byte("tree " + tree + "\nauthor <> 99999999999999999999999 +9999\n\n"))
95	95		f.Add([]byte("tree\n\n"))
	96	+	f.Add([]byte("tree " + tree + "\ntree " + par + "\nparent " + par + "\n\n"))
	97	+	f.Add([]byte("tree " + tree + "\nparent " + par + "\nauthor A <a@x> 1 +0000\nparent " + tree + "\n\nm"))
	98	+	f.Add([]byte("parent " + par + "\ntree " + tree + "\n\n"))
96	99		f.Add([]byte{})
97	100		f.Add([]byte("\n\n\n"))
98	101		var c Commit

@@ -102,6 +105,20 @@ func FuzzParseCommit(f *testing.F) {
102	105		if err == nil && len(c.Message) > len(data) {
103	106		t.Fatal("message longer than input")
104	107		}
	108	+	if err == nil && !bytes.HasPrefix(data, []byte("tree ")) {
	109	+	t.Fatal("accepted a commit whose first header is not the tree")
	110	+	}
	111	+	if err == nil && len(c.Parents) > 0 {
	112	+	// The parents are the lines that follow the tree directly.
	113	+	rest := data[bytes.IndexByte(data, '\n')+1:]
	114	+	for _, p := range c.Parents {
	115	+	want := append([]byte("parent "), []byte(p.String())...)
	116	+	if len(rest) < len(want) \|\| !bytes.EqualFold(rest[:len(want)], want) {
	117	+	t.Fatalf("parent %s is not on the line after the tree", p)
	118	+	}
	119	+	rest = rest[bytes.IndexByte(rest, '\n')+1:]
	120	+	}
	121	+	}
105	122		var tg Tag
106	123		okOrCorrupt(t, ParseTag(data, &tg))
107	124		_ = parseSignature(data)


@@ -93,6 +93,9 @@ func FuzzParseCommit(f *testing.F) {
93	f.Add([]byte("tree " + tree))	93		f.Add([]byte("tree " + tree))
94	f.Add([]byte("tree " + tree + "\nauthor <> 99999999999999999999999 +9999\n\n"))	94		f.Add([]byte("tree " + tree + "\nauthor <> 99999999999999999999999 +9999\n\n"))
95	f.Add([]byte("tree\n\n"))	95		f.Add([]byte("tree\n\n"))
		96	+	f.Add([]byte("tree " + tree + "\ntree " + par + "\nparent " + par + "\n\n"))
		97	+	f.Add([]byte("tree " + tree + "\nparent " + par + "\nauthor A <a@x> 1 +0000\nparent " + tree + "\n\nm"))
		98	+	f.Add([]byte("parent " + par + "\ntree " + tree + "\n\n"))
96	f.Add([]byte{})	99		f.Add([]byte{})
97	f.Add([]byte("\n\n\n"))	100		f.Add([]byte("\n\n\n"))
98	var c Commit	101		var c Commit

@@ -102,6 +105,20 @@ func FuzzParseCommit(f *testing.F) {
102	if err == nil && len(c.Message) > len(data) {	105		if err == nil && len(c.Message) > len(data) {
103	t.Fatal("message longer than input")	106		t.Fatal("message longer than input")
104	}	107		}
		108	+	if err == nil && !bytes.HasPrefix(data, []byte("tree ")) {
		109	+	t.Fatal("accepted a commit whose first header is not the tree")
		110	+	}
		111	+	if err == nil && len(c.Parents) > 0 {
		112	+	// The parents are the lines that follow the tree directly.
		113	+	rest := data[bytes.IndexByte(data, '\n')+1:]
		114	+	for _, p := range c.Parents {
		115	+	want := append([]byte("parent "), []byte(p.String())...)
		116	+	if len(rest) < len(want) \|\| !bytes.EqualFold(rest[:len(want)], want) {
		117	+	t.Fatalf("parent %s is not on the line after the tree", p)
		118	+	}
		119	+	rest = rest[bytes.IndexByte(rest, '\n')+1:]
		120	+	}
		121	+	}
105	var tg Tag	122		var tg Tag
106	okOrCorrupt(t, ParseTag(data, &tg))	123		okOrCorrupt(t, ParseTag(data, &tg))
107	_ = parseSignature(data)	124		_ = parseSignature(data)

M parse_test.go +9 -6


@@ -223,12 +223,15 @@ func TestParseCommitEdgeCases(t *testing.T) {
223	223		}
224	224		}
225	225		for name, d := range map[string]string{
226		-	"empty": "",
227		-	"no tree": "author A <a@x> 1 +0000\n\nmsg",
228		-	"bad tree": "tree xyz\n\n",
229		-	"short tree": "tree " + tree[:39] + "\n\n",
230		-	"bad parent": "tree " + tree + "\nparent zz\n\n",
231		-	"double tree": "tree " + tree + "\ntree " + tree + "\n\n",
	226	+	"empty": "",
	227	+	"no tree": "author A <a@x> 1 +0000\n\nmsg",
	228	+	"bad tree": "tree xyz\n\n",
	229	+	"short tree": "tree " + tree[:39] + "\n\n",
	230	+	"bad parent": "tree " + tree + "\nparent zz\n\n",
	231	+	"tree second": "author A <a@x> 1 +0000\ntree " + tree + "\n\n",
	232	+	"parent first": "parent " + tree + "\ntree " + tree + "\n\n",
	233	+	"blank first": "\ntree " + tree + "\n",
	234	+	"bad parent after": "tree " + tree + "\nparent " + tree + "\nparent x\n\n",
232	235		} {
233	236		if err := ParseCommit([]byte(d), &c); !errors.Is(err, ErrCorrupt) {
234	237		t.Errorf("%s: %v", name, err)

 		}
 	}
 	for name, d := range map[string]string{
-		"empty":       "",
+		"empty":            "",
-		"no tree":     "author A <a@x> 1 +0000\n\nmsg",
+		"no tree":          "author A <a@x> 1 +0000\n\nmsg",
-		"bad tree":    "tree xyz\n\n",
+		"bad tree":         "tree xyz\n\n",
-		"short tree":  "tree " + tree[:39] + "\n\n",
+		"short tree":       "tree " + tree[:39] + "\n\n",
-		"bad parent":  "tree " + tree + "\nparent zz\n\n",
+		"bad parent":       "tree " + tree + "\nparent zz\n\n",
-		"double tree": "tree " + tree + "\ntree " + tree + "\n\n",
+		"tree second":      "author A <a@x> 1 +0000\ntree " + tree + "\n\n",
+		"parent first":     "parent " + tree + "\ntree " + tree + "\n\n",
+		"blank first":      "\ntree " + tree + "\n",
+		"bad parent after": "tree " + tree + "\nparent " + tree + "\nparent x\n\n",
 	} {
 		if err := ParseCommit([]byte(d), &c); !errors.Is(err, ErrCorrupt) {
 			t.Errorf("%s: %v", name, err)