Added listen path validation

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Unreleased
 
-- TODO
+## Added
+
+- Added Proxy Listen Path validation to prevent `chi` from panicking in case of invalid listen path
 
 # 3.3.0
 

cmd/janus/init.go

@@ -5,8 +5,6 @@ import (
 	"os"
 	"path/filepath"
 
-	mgo "gopkg.in/mgo.v2"
-
 	"github.com/hellofresh/janus/pkg/config"
 	tracerfactory "github.com/hellofresh/janus/pkg/opentracing"
 	"github.com/hellofresh/janus/pkg/store"
@@ -15,6 +13,7 @@ import (
 	"github.com/hellofresh/stats-go/hooks"
 	"github.com/opentracing/opentracing-go"
 	log "github.com/sirupsen/logrus"
+	"gopkg.in/mgo.v2"
 )
 
 var (

cmd/janus/server.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/hellofresh/janus/pkg/api"
 	"github.com/hellofresh/janus/pkg/errors"
 	"github.com/hellofresh/janus/pkg/middleware"
 	"github.com/hellofresh/janus/pkg/notifier"
@@ -35,7 +34,6 @@ import (
 )
 
 var (
-	repo   api.Repository
 	server *http.Server
 )
 

pkg/api/api.go

@@ -21,7 +21,7 @@ type Plugin struct {
 type Definition struct {
 	Name        string            `bson:"name" json:"name" valid:"required"`
 	Active      bool              `bson:"active" json:"active"`
-	Proxy       *proxy.Definition `bson:"proxy" json:"proxy"`
+	Proxy       *proxy.Definition `bson:"proxy" json:"proxy" valid:"required"`
 	Plugins     []Plugin          `bson:"plugins" json:"plugins"`
 	HealthCheck HealthCheck       `bson:"health_check" json:"health_check"`
 }

pkg/api/api_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/hellofresh/janus/pkg/api"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewInstanceOfDefinition(t *testing.T) {
@@ -17,9 +18,11 @@ func TestNewInstanceOfDefinition(t *testing.T) {
 func TestSuccessfulValidation(t *testing.T) {
 	instance := api.NewDefinition()
 	instance.Name = "Test"
-	isValid, err := instance.Validate()
+	instance.Proxy.ListenPath = "/"
+	instance.Proxy.UpstreamURL = "http://example.com"
 
-	assert.NoError(t, err)
+	isValid, err := instance.Validate()
+	require.NoError(t, err)
 	assert.True(t, isValid)
 }
 

pkg/api/file_repository_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/hellofresh/janus/pkg/proxy"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func newRepo(t *testing.T) *FileSystemRepository {
@@ -42,17 +43,23 @@ func TestNewFileSystemRepository(t *testing.T) {
 	assertFindByFindByListenPath(t, fsRepo)
 	assertExists(t, fsRepo)
 
-	defToAdd := &Definition{Name: "foo-bar", Proxy: &proxy.Definition{ListenPath: "/foo/bar/*"}}
+	defToAdd := &Definition{
+		Name: "foo-bar",
+		Proxy: &proxy.Definition{
+			ListenPath:  "/foo/bar/*",
+			UpstreamURL: "http://example.com/foo/bar/",
+		},
+	}
 	err = fsRepo.Add(defToAdd)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	def, err := fsRepo.FindByName(defToAdd.Name)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, defToAdd.Name, def.Name)
 	assert.Equal(t, defToAdd.Proxy.ListenPath, def.Proxy.ListenPath)
 
 	def, err = fsRepo.FindByListenPath(defToAdd.Proxy.ListenPath)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, defToAdd.Name, def.Name)
 	assert.Equal(t, defToAdd.Proxy.ListenPath, def.Proxy.ListenPath)
 
@@ -68,7 +75,7 @@ func TestNewFileSystemRepository(t *testing.T) {
 	assert.Equal(t, ErrAPIListenPathExists, err)
 
 	err = fsRepo.Remove(defToAdd.Name)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	err = fsRepo.Remove(defToAdd.Name)
 	assert.Equal(t, ErrAPIDefinitionNotFound, err)
@@ -80,8 +87,8 @@ func TestNewFileSystemRepository(t *testing.T) {
 	assert.Equal(t, ErrAPIDefinitionNotFound, err)
 
 	exists, err = fsRepo.Exists(defToAdd)
+	require.NoError(t, err)
 	assert.False(t, exists)
-	assert.NoError(t, err)
 }
 
 func assertFindByName(t *testing.T, fsRepo *FileSystemRepository) {

pkg/proxy/definition.go

@@ -2,6 +2,7 @@ package proxy
 
 import (
 	"encoding/json"
+	"strings"
 
 	"github.com/asaskevich/govalidator"
 	"github.com/hellofresh/janus/pkg/router"
@@ -10,7 +11,7 @@ import (
 // Definition defines proxy rules for a route
 type Definition struct {
 	PreserveHost        bool     `bson:"preserve_host" json:"preserve_host" mapstructure:"preserve_host"`
-	ListenPath          string   `bson:"listen_path" json:"listen_path" mapstructure:"listen_path" valid:"required"`
+	ListenPath          string   `bson:"listen_path" json:"listen_path" mapstructure:"listen_path" valid:"required,urlpath"`
 	UpstreamURL         string   `bson:"upstream_url" json:"upstream_url" mapstructure:"upstream_url" valid:"url,required"`
 	InsecureSkipVerify  bool     `bson:"insecure_skip_verify" json:"insecure_skip_verify" mapstructure:"insecure_skip_verify"`
 	StripPath           bool     `bson:"strip_path" json:"strip_path" mapstructure:"strip_path"`
@@ -81,3 +82,15 @@ func JSONUnmarshalRoute(rawRoute []byte) (*Route, error) {
 	}
 	return NewRoute(proxyRoute.Proxy), nil
 }
+
+func init() {
+	// initializes custom validators
+	govalidator.CustomTypeTagMap.Set("urlpath", func(i interface{}, o interface{}) bool {
+		s, ok := i.(string)
+		if !ok {
+			return false
+		}
+
+		return strings.Index(s, "/") == 0
+	})
+}

pkg/proxy/register.go

@@ -106,11 +106,16 @@ func (p *Register) doRegister(listenPath string, handler http.HandlerFunc, metho
 		"listen_path": listenPath,
 	}).Debug("Registering a route")
 
-	for _, method := range methods {
-		if strings.ToUpper(method) == methodAll {
-			p.Router.Any(listenPath, handler, handlers...)
-		} else {
-			p.Router.Handle(strings.ToUpper(method), listenPath, handler, handlers...)
+	if strings.Index(listenPath, "/") != 0 {
+		log.WithField("listen_path", listenPath).
+			Error("Route listen path must begin with '/'.Skipping invalid route.")
+	} else {
+		for _, method := range methods {
+			if strings.ToUpper(method) == methodAll {
+				p.Router.Any(listenPath, handler, handlers...)
+			} else {
+				p.Router.Handle(strings.ToUpper(method), listenPath, handler, handlers...)
+			}
 		}
 	}
 }
@@ -134,13 +139,13 @@ func singleJoiningSlash(a, b string) string {
 	a = cleanSlashes(a)
 	b = cleanSlashes(b)
 
-	aslash := strings.HasSuffix(a, "/")
-	bslash := strings.HasPrefix(b, "/")
+	aSlash := strings.HasSuffix(a, "/")
+	bSlash := strings.HasPrefix(b, "/")
 
 	switch {
-	case aslash && bslash:
+	case aSlash && bSlash:
 		return a + b[1:]
-	case !aslash && !bslash:
+	case !aSlash && !bSlash:
 		if len(b) > 0 {
 			return a + "/" + b
 		}