Fix regex denial-of-service

Makefile

@@ -1,16 +1,19 @@
 default: test lint
+LINT=golangci-lint run
 
 test:
 	go test -v ./...
 	go test -covermode=count -coverprofile=profile.cov .
 
 lint:
-	gometalinter ./...
+	$(LINT) ./...
 
 install:
 	go get -d -v ./... && go build -v ./...
-	gometalinter --install --update
+	$(LINT) ./...
 
 deps:
-	go get github.com/alecthomas/gometalinter
-	go get golang.org/x/tools/cmd/cover
+	# binary will be $(go env GOPATH)/bin/golangci-lint
+	go mod download -x
+	go mod verify
+	go mod tidy -v

go.mod

@@ -1,3 +1,5 @@
 module github.com/whilp/git-urls
 
 go 1.13
+
+require github.com/stretchr/testify v1.8.4

go.sum

@@ -0,0 +1,17 @@
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

urls.go

@@ -48,6 +48,8 @@ var (
 	)
 )
 
+const maxUrlLen = 8000
+
 // Parser converts a string into a URL.
 type Parser func(string) (*url.URL, error)
 
@@ -90,6 +92,15 @@ func ParseTransport(rawurl string) (*url.URL, error) {
 // ParseScp parses rawurl into a URL object. The rawurl must be
 // an SCP-like URL, otherwise ParseScp returns an error.
 func ParseScp(rawurl string) (*url.URL, error) {
+
+	// Did some research on https://stackoverflow.com/q/417142/31319
+	// and for best results, URLs should not be more than about 2000
+	// bytes. Some sites support up to 8000 bytes but that isn't
+	// widespread yet. We will try using 8000 first.
+	if len(rawurl) >= maxUrlLen {
+		return nil, fmt.Errorf("URL too long (%d >= %d)", len(rawurl), maxUrlLen)
+	}
+
 	match := scpSyntax.FindAllStringSubmatch(rawurl, -1)
 	if len(match) == 0 {
 		return nil, fmt.Errorf("no scp URL found in %q", rawurl)

urls_test.go

@@ -5,6 +5,9 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var tests []*Test
@@ -218,3 +221,33 @@ func TestParse(t *testing.T) {
 		}
 	}
 }
+
+// TestRegex tests to see if we have an excessively long URL
+func TestRegex(t *testing.T) {
+
+	// inner function for repeating tests
+	runTimingTest := func(url string, shouldError bool) {
+		begin := time.Now()
+
+		_, err := ParseScp(url)
+		if shouldError {
+			assert.Errorf(t, err, "len of %d should trigger error", len(url))
+		} else {
+			assert.Nilf(t, err, "unexpected error: %v", err)
+		}
+		elapsed := time.Since(begin)
+		t.Logf("url len is %d, function took %+v", len(url), elapsed)
+	}
+
+	// First case is 7909 bytes which should still be fast
+	runTimingTest(`https://=`+strings.Repeat(`/`, 7900), false)
+
+	// Second case is 190,000,000 bytes which should be too slow
+	runTimingTest(`https://=`+strings.Repeat(`/`, 190000000), true)
+
+	// Real URL
+	runTimingTest(`https://stackoverflow.com/q/417142/31319`, false)
+
+	// Another real URL
+	runTimingTest(`https://github.com/whilp/git-urls.git`, false)
+}