Merge pull request #181 from vmarkovtsev/master

Fix slice out of bounds in renames

internal/plumbing/renames.go

@@ -426,13 +426,22 @@ func (ra *RenameAnalysis) blobsAreClose(blob1 *CachedBlob, blob2 *CachedBlob) (b
 		}
 		// supposing that the rest of the lines are the same (they are not - too optimistic),
 		// estimate the maximum similarity and exit the loop if it lower than our threshold
-		maxCommon := common + internal.Min(
-			utf8.RuneCountInString(src[posSrc:]),
-			utf8.RuneCountInString(dst[posDst:]))
+		var srcPendingSize, dstPendingSize int
+		if posSrc < len(src) {
+			srcPendingSize = utf8.RuneCountInString(src[posSrc:])
+		}
+		if posDst < len(dst) {
+			dstPendingSize = utf8.RuneCountInString(dst[posDst:])
+		}
+		maxCommon := common + internal.Min(srcPendingSize, dstPendingSize)
 		similarity := (maxCommon * 100) / maxSize
 		if similarity < ra.SimilarityThreshold {
 			return false, nil
 		}
+		similarity = (common * 100) / maxSize
+		if similarity >= ra.SimilarityThreshold {
+			return true, nil
+		}
 	}
 	// the very last "overly optimistic" estimate was actually precise, so since we are still here
 	// the blobs are similar

internal/plumbing/renames_test.go

@@ -2,6 +2,10 @@ package plumbing
 
 import (
 	"bytes"
+	"compress/gzip"
+	"io/ioutil"
+	"os"
+	"path"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -259,3 +263,27 @@ func TestBlobsAreCloseBinary(t *testing.T) {
 	assert.Nil(t, err)
 	assert.False(t, result)
 }
+
+func TestBlobsAreCloseBug(t *testing.T) {
+	gzsource, err := os.Open(path.Join("..", "test_data", "rename_bug.xml.gz"))
+	defer gzsource.Close()
+	if err != nil {
+		t.Errorf("open ../test_data/rename_bug.xml.gz: %v", err)
+	}
+	gzreader, err := gzip.NewReader(gzsource)
+	if err != nil {
+		t.Errorf("gzip ../test_data/rename_bug.xml.gz: %v", err)
+	}
+	data, err := ioutil.ReadAll(gzreader)
+	if err != nil {
+		t.Errorf("gzip ../test_data/rename_bug.xml.gz: %v", err)
+	}
+	blob1 := &CachedBlob{Data: data}
+	blob2 := &CachedBlob{Data: data}
+	blob1.Size = int64(len(data))
+	blob2.Size = int64(len(data))
+	ra := fixtureRenameAnalysis()
+	result, err := ra.blobsAreClose(blob1, blob2)
+	assert.Nil(t, err)
+	assert.True(t, result)
+}