[bug] - Fix the starting index value for plus line check.

[ahrav]

[bill-rich]

Which repo are you running against to get this crash?

I think a better place to fix this would be to check the slice length at:

trufflehog/pkg/gitparse/gitparse.go

Line 229 in 70a40cd

if len(line) > 3 && bytes.Equal(line[:3], []byte("+++")) {

Starting at :3 will include the b/ in the path which we want to exclude

[bill-rich]

Nevermind on the repo question. I see it in the output.

[ahrav]

Which repo are you running against to get this crash?

I think a better place to fix this would be to check the slice length at:

trufflehog/pkg/gitparse/gitparse.go

Line 229 in 70a40cd

if len(line) > 3 && bytes.Equal(line[:3], []byte("+++")) {

Starting at :3 will include the b/ in the path which we want to exclude

I ran it against the udacity org. I've been using that org for my testing lately. Oh I see you would want it to be :4 since we want to exclude +++. Makes sense. Was the original 6 just a typo here?

Oh nvm we do want it to be 6 but you are suggesting checking that len first.

[bill-rich]

Which repo are you running against to get this crash?
I think a better place to fix this would be to check the slice length at:

trufflehog/pkg/gitparse/gitparse.go

Line 229 in 70a40cd

if len(line) > 3 && bytes.Equal(line[:3], []byte("+++")) {

Starting at :3 will include the b/ in the path which we want to exclude

I ran it against the udacity org. I've been using that org for my testing lately. Oh I see you would want it to be :4 since we want to exclude +++. Makes sense. Was the original 6 just a typo here?

Oh nvm we do want it to be 6 but you are suggesting checking that len first.

I'm curious what line is getting caught that is too short, but the line length check should be 6. That just got overlooked and no cases came up to break it.

I was thinking something like this:

diff --git a/pkg/gitparse/gitparse.go b/pkg/gitparse/gitparse.go
index 67388075..925da99c 100644
--- a/pkg/gitparse/gitparse.go
+++ b/pkg/gitparse/gitparse.go
@@ -120,7 +120,7 @@ func RepoPath(source string, head string) (chan Commit, error) {
                        case isIndexLine(line):
                                // NoOp
                        case isPlusFileLine(line):
-                               currentDiff.PathB = strings.TrimRight(string(line[3:]), "\n")
+                               currentDiff.PathB = strings.TrimRight(string(line[6:]), "\n")
                        case isMinusFileLine(line):
                                // NoOp
                        case isPlusDiffLine(line):
@@ -218,7 +218,7 @@ func isModeLine(line []byte) bool {
 
 // --- a/internal/addrs/move_endpoint_module.go
 func isMinusFileLine(line []byte) bool {
-       if len(line) > 3 && bytes.Equal(line[:3], []byte("---")) {
+       if len(line) >= 6 && bytes.Equal(line[:3], []byte("---")) {
                return true
        }
        return false
@@ -226,7 +226,7 @@ func isMinusFileLine(line []byte) bool {
 
 // +++ b/internal/addrs/move_endpoint_module.go
 func isPlusFileLine(line []byte) bool {
-       if len(line) > 3 && bytes.Equal(line[:3], []byte("+++")) {
+       if len(line) >= 6 && bytes.Equal(line[:3], []byte("+++")) {
                return true
        }
        return false

[mcastorina]

Tangent, but the bytes package has almost all the same functions as strings, including bytes.TrimRight and bytes.HasPrefix.

That should be more performant and convenient than converting to strings or checking slice lengths and comparing.

(Something I learned recently but not necessarily applicable here).

[mcastorina]

I know it's a small change, but would it be possible to add a test to prevent future regressions?

Maybe in a follow-up PR if this is urgent.

[bill-rich]

Looks good. I like the additional tests.