Skip to content

Commit 1c67093

Browse files
chojs23stefanhaller
andcommitted
Fix index out of bounds panic when repository has massive tag lists
Co-authored-by: Stefan Haller <[email protected]>
1 parent c54232e commit 1c67093

File tree

2 files changed

+214
-1
lines changed

2 files changed

+214
-1
lines changed

pkg/commands/git_commands/commit_loader.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,12 @@ func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commi
202202
func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line string, showDivergence bool) *models.Commit {
203203
split := strings.SplitN(line, "\x00", 8)
204204

205+
// Ensure we have the minimum required fields (at least 7 for basic functionality)
206+
if len(split) < 7 {
207+
self.Log.Warnf("Malformed git log line: expected at least 7 fields, got %d. Line: %s", len(split), line)
208+
return nil
209+
}
210+
205211
hash := split[0]
206212
unixTimestamp := split[1]
207213
authorName := split[2]
@@ -212,7 +218,12 @@ func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line
212218
divergence = lo.Ternary(split[5] == "<", models.DivergenceLeft, models.DivergenceRight)
213219
}
214220
extraInfo := strings.TrimSpace(split[6])
215-
message := split[7]
221+
222+
// message (and the \x00 before it) might not be present if extraInfo is extremely long
223+
message := ""
224+
if len(split) > 7 {
225+
message = split[7]
226+
}
216227

217228
var tags []string
218229

pkg/commands/git_commands/commit_loader_test.go

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,3 +588,205 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) {
588588
})
589589
}
590590
}
591+
592+
func TestCommitLoader_extractCommitFromLine(t *testing.T) {
593+
common := common.NewDummyCommon()
594+
hashPool := &utils.StringPool{}
595+
596+
loader := &CommitLoader{
597+
Common: common,
598+
}
599+
600+
scenarios := []struct {
601+
testName string
602+
line string
603+
showDivergence bool
604+
expectedCommit *models.Commit
605+
}{
606+
{
607+
testName: "normal commit line with all fields",
608+
line: "0eea75e8c631fba6b58135697835d58ba4c18dbc\x001640826609\x00Jesse Duffield\x00[email protected]\x00b21997d6b4cbdf84b149\x00>\x00HEAD -> better-tests\x00better typing for rebase mode",
609+
showDivergence: false,
610+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
611+
Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc",
612+
Name: "better typing for rebase mode",
613+
Tags: nil,
614+
ExtraInfo: "(HEAD -> better-tests)",
615+
UnixTimestamp: 1640826609,
616+
AuthorName: "Jesse Duffield",
617+
AuthorEmail: "[email protected]",
618+
Parents: []string{"b21997d6b4cbdf84b149"},
619+
Divergence: models.DivergenceNone,
620+
}),
621+
},
622+
{
623+
testName: "normal commit line with left divergence",
624+
line: "hash123\x001234567890\x00John Doe\x00[email protected]\x00parent1 parent2\x00<\x00origin/main\x00commit message",
625+
showDivergence: true,
626+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
627+
Hash: "hash123",
628+
Name: "commit message",
629+
Tags: nil,
630+
ExtraInfo: "(origin/main)",
631+
UnixTimestamp: 1234567890,
632+
AuthorName: "John Doe",
633+
AuthorEmail: "[email protected]",
634+
Parents: []string{"parent1", "parent2"},
635+
Divergence: models.DivergenceLeft,
636+
}),
637+
},
638+
{
639+
testName: "commit line with tags in extraInfo",
640+
line: "abc123\x001640000000\x00Jane Smith\x00[email protected]\x00parenthash\x00>\x00tag: v1.0, tag: release\x00tagged release",
641+
showDivergence: true,
642+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
643+
Hash: "abc123",
644+
Name: "tagged release",
645+
Tags: []string{"v1.0", "release"},
646+
ExtraInfo: "(tag: v1.0, tag: release)",
647+
UnixTimestamp: 1640000000,
648+
AuthorName: "Jane Smith",
649+
AuthorEmail: "[email protected]",
650+
Parents: []string{"parenthash"},
651+
Divergence: models.DivergenceRight,
652+
}),
653+
},
654+
{
655+
testName: "commit line with empty extraInfo",
656+
line: "def456\x001640000000\x00Bob Wilson\x00[email protected]\x00parenthash\x00>\x00\x00simple commit",
657+
showDivergence: true,
658+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
659+
Hash: "def456",
660+
Name: "simple commit",
661+
Tags: nil,
662+
ExtraInfo: "",
663+
UnixTimestamp: 1640000000,
664+
AuthorName: "Bob Wilson",
665+
AuthorEmail: "[email protected]",
666+
Parents: []string{"parenthash"},
667+
Divergence: models.DivergenceRight,
668+
}),
669+
},
670+
{
671+
testName: "commit line with no parents (root commit)",
672+
line: "root123\x001640000000\x00Alice Cooper\x00[email protected]\x00\x00>\x00\x00initial commit",
673+
showDivergence: true,
674+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
675+
Hash: "root123",
676+
Name: "initial commit",
677+
Tags: nil,
678+
ExtraInfo: "",
679+
UnixTimestamp: 1640000000,
680+
AuthorName: "Alice Cooper",
681+
AuthorEmail: "[email protected]",
682+
Parents: nil,
683+
Divergence: models.DivergenceRight,
684+
}),
685+
},
686+
{
687+
testName: "malformed line with only 3 fields",
688+
line: "hash\x00timestamp\x00author",
689+
showDivergence: false,
690+
expectedCommit: nil,
691+
},
692+
{
693+
testName: "malformed line with only 4 fields",
694+
line: "hash\x00timestamp\x00author\x00email",
695+
showDivergence: false,
696+
expectedCommit: nil,
697+
},
698+
{
699+
testName: "malformed line with only 5 fields",
700+
line: "hash\x00timestamp\x00author\x00email\x00parents",
701+
showDivergence: false,
702+
expectedCommit: nil,
703+
},
704+
{
705+
testName: "malformed line with only 6 fields",
706+
line: "hash\x00timestamp\x00author\x00email\x00parents\x00<",
707+
showDivergence: true,
708+
expectedCommit: nil,
709+
},
710+
{
711+
testName: "minimal valid line with 7 fields (no message)",
712+
line: "hash\x00timestamp\x00author\x00email\x00parents\x00>\x00extraInfo",
713+
showDivergence: true,
714+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
715+
Hash: "hash",
716+
Name: "",
717+
Tags: nil,
718+
ExtraInfo: "(extraInfo)",
719+
UnixTimestamp: 0,
720+
AuthorName: "author",
721+
AuthorEmail: "email",
722+
Parents: []string{"parents"},
723+
Divergence: models.DivergenceRight,
724+
}),
725+
},
726+
{
727+
testName: "minimal valid line with 7 fields (empty extraInfo)",
728+
line: "hash\x00timestamp\x00author\x00email\x00parents\x00>\x00",
729+
showDivergence: true,
730+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
731+
Hash: "hash",
732+
Name: "",
733+
Tags: nil,
734+
ExtraInfo: "",
735+
UnixTimestamp: 0,
736+
AuthorName: "author",
737+
AuthorEmail: "email",
738+
Parents: []string{"parents"},
739+
Divergence: models.DivergenceRight,
740+
}),
741+
},
742+
{
743+
testName: "valid line with 8 fields (complete)",
744+
line: "hash\x00timestamp\x00author\x00email\x00parents\x00<\x00extraInfo\x00message",
745+
showDivergence: true,
746+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
747+
Hash: "hash",
748+
Name: "message",
749+
Tags: nil,
750+
ExtraInfo: "(extraInfo)",
751+
UnixTimestamp: 0,
752+
AuthorName: "author",
753+
AuthorEmail: "email",
754+
Parents: []string{"parents"},
755+
Divergence: models.DivergenceLeft,
756+
}),
757+
},
758+
{
759+
testName: "empty line",
760+
line: "",
761+
showDivergence: false,
762+
expectedCommit: nil,
763+
},
764+
{
765+
testName: "line with special characters in commit message",
766+
line: "special123\x001640000000\x00Dev User\x00[email protected]\x00parenthash\x00>\x00\x00fix: handle \x00 null bytes and 'quotes'",
767+
showDivergence: true,
768+
expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
769+
Hash: "special123",
770+
Name: "fix: handle \x00 null bytes and 'quotes'",
771+
Tags: nil,
772+
ExtraInfo: "",
773+
UnixTimestamp: 1640000000,
774+
AuthorName: "Dev User",
775+
AuthorEmail: "[email protected]",
776+
Parents: []string{"parenthash"},
777+
Divergence: models.DivergenceRight,
778+
}),
779+
},
780+
}
781+
782+
for _, scenario := range scenarios {
783+
t.Run(scenario.testName, func(t *testing.T) {
784+
result := loader.extractCommitFromLine(hashPool, scenario.line, scenario.showDivergence)
785+
if scenario.expectedCommit == nil {
786+
assert.Nil(t, result)
787+
} else {
788+
assert.Equal(t, scenario.expectedCommit, result)
789+
}
790+
})
791+
}
792+
}

0 commit comments

Comments
 (0)