Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

plumbing: diff, fix crash when a small ending equal-chunk #749

Merged
merged 2 commits into from
Feb 17, 2018
Merged

plumbing: diff, fix crash when a small ending equal-chunk #749

merged 2 commits into from
Feb 17, 2018

Conversation

irias
Copy link

@irias irias commented Feb 14, 2018

Signed-off-by: Mechiel Lukkien [email protected]

note: i'm not familiar with the code base. please review fix, and whether needs fixing in other places.

to reproduce, first create a git repo with a commit that has a small trailing equals-chunk:

mkdir git-test-repo
cd git-test-repo/
git init
yes | head -n10 >test.txt
git add test.txt
git commit -m test
(yes | head -n8; echo n; echo y) >test.txt
git commit -m test test.txt

now run the test program at the bottom, and observe the crash:

$ go run main.go -- git-test-repo/
2018/02/14 18:22:25 patch between commit
commit beccfaa9484487c5adad075ac96308ed9ab897d2
Author: Mechiel Lukkien <[email protected]>
Date:   Wed Feb 14 18:10:24 2018 +0100

    test


and
commit c408553d8c54d91bf197fad054541722d16d471e
Author: Mechiel Lukkien <[email protected]>
Date:   Wed Feb 14 18:10:24 2018 +0100

    test


panic: runtime error: slice bounds out of range

goroutine 1 [running]:
gopkg.in/src-d/go-git.v4/plumbing/format/diff.(*hunksGenerator).processEqualsLines(0xc4200edd00, 0xc4201b4540, 0x1, 0x2, 0x3)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/format/diff/unified_encoder.go:265 +0x59d
gopkg.in/src-d/go-git.v4/plumbing/format/diff.(*hunksGenerator).Generate(0xc4200edd00, 0xc42005bdc0, 0x4, 0x4)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/format/diff/unified_encoder.go:180 +0x3f2
gopkg.in/src-d/go-git.v4/plumbing/format/diff.(*UnifiedEncoder).encodeFilePatch(0xc4200d6400, 0xc420124770, 0x1, 0x1, 0x1, 0xc4200d6400)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/format/diff/unified_encoder.go:85 +0x208
gopkg.in/src-d/go-git.v4/plumbing/format/diff.(*UnifiedEncoder).Encode(0xc4200d6400, 0x1612a00, 0xc42013e7b0, 0x13e40e0, 0x101)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/format/diff/unified_encoder.go:68 +0x65
gopkg.in/src-d/go-git.v4/plumbing/object.(*Patch).Encode(0xc42013e7b0, 0x160e980, 0xc4201b23f0, 0xc4200ede70, 0x10cb200)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/object/patch.go:107 +0x7c
gopkg.in/src-d/go-git.v4/plumbing/object.(*Patch).String(0xc42013e7b0, 0x1f, 0xc4200edf50)
	/Users/mjl/go/src/gopkg.in/src-d/go-git.v4/plumbing/object/patch.go:116 +0x5a
main.main()
	/Users/mjl/go/src/test/go-git-diff-bug/main.go:51 +0x3d8
exit status 2

then apply the patch in this pull request, and see the expected patch:

$ go run main.go -- git-test-repo/
2018/02/14 18:12:35 patch between commit
commit beccfaa9484487c5adad075ac96308ed9ab897d2
Author: Mechiel Lukkien <[email protected]>
Date:   Wed Feb 14 18:10:24 2018 +0100

    test


and
commit c408553d8c54d91bf197fad054541722d16d471e
Author: Mechiel Lukkien <[email protected]>
Date:   Wed Feb 14 18:10:24 2018 +0100

    test


2018/02/14 18:12:35 diff --git a/test.txt b/test.txt
index db7cbfc682f6343bb0e71e0cf774ba7a1ca1a37b..67c82c94116850ddfcf70fd43ae4ab2eb2242503 100644
--- a/test.txt
+++ b/test.txt
@@ -6,5 +6,5 @@ y
 y
 y
 y
-y
+n
 y

and the attached program that triggers the bug:

package main

import (
	"flag"
	"log"

	"gopkg.in/src-d/go-git.v4"
	"gopkg.in/src-d/go-git.v4/plumbing/object"
)


func check(err error, msg string) {
	if err != nil {
		log.Fatalf("%s: %s\n", msg, err)
	}
}

func main() {
	flag.Parse()
	args := flag.Args()
	if len(args) != 1 {
		log.Fatalf("pass git repo directory as parameter")
	}
	dir := args[0]
	repo, err := git.PlainOpen(dir)
	check(err, "plainopen")

	iter, err := repo.CommitObjects()
	check(err, "commitobjects")

	c0, err := iter.Next()
	check(err, "commit1")
	c1, err := iter.Next()
	check(err, "commit2")

	commits := []*object.Commit{c0, c1}
	if len(commits[0].ParentHashes) != 0 {
		commits = []*object.Commit{c1, c0}
	}

	t0, err := repo.TreeObject(commits[0].TreeHash)
	check(err, "treeobject first commit")
	t1, err := repo.TreeObject(commits[1].TreeHash)
	check(err, "treeobject second commit")

	p, err := t0.Patch(t1)
	check(err, "patch")

	log.Printf("patch between commit\n%s\nand\n%s\n", commits[0], commits[1])

	diff := p.String() // crashes

	log.Print(diff)
}

@mcuadros mcuadros requested a review from ajnavarro February 15, 2018 10:22
Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for this case please?

this reuses an existing patch, setting context to 6 triggers the
bug, becuase of a 5-line trailing equals chunk.

Signed-off-by: Mechiel Lukkien <[email protected]>
@irias
Copy link
Author

irias commented Feb 16, 2018

test has been added.

@mjl-
Copy link
Contributor

mjl- commented Feb 16, 2018

fyi, PR and comments are showing up as created by "ghost" because i turned my company github user account i was using into an organization.

@mcuadros mcuadros changed the title fix crash when generating a unified diff with a small ending equal-chunk plumbing: diff, fix crash when a small ending equal-chunk Feb 17, 2018
@mcuadros mcuadros merged commit 886dc83 into src-d:master Feb 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants