Validate External Cache Host (prebid#1422)

* first draft

* Little tweaks

* Scott's review part 1

* Scott's review corrections part 2

* Scotts refactor

* correction in config_test.go

* Correction and refactor

* Multiple return statements

* Test case refactor

Co-authored-by: Gus Carreon <[email protected]>
Co-authored-by: Gus Carreon <[email protected]>
Co-authored-by: Gus Carreon <[email protected]>

Author: 4 people

config/config.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"net/url"
 	"reflect"
@@ -111,6 +112,7 @@ func (cfg *Configuration) validate() configErrors {
 	errs = cfg.CurrencyConverter.validate(errs)
 	errs = validateAdapters(cfg.Adapters, errs)
 	errs = cfg.Debug.validate(errs)
+	errs = cfg.ExtCacheURL.validate(errs)
 	return errs
 }
 
@@ -128,6 +130,40 @@ func (cfg *AuctionTimeouts) validate(errs configErrors) configErrors {
 	return errs
 }
 
+func (data *ExternalCache) validate(errs configErrors) configErrors {
+	if data.Host == "" && data.Path == "" {
+		// Both host and path can be blank. No further validation needed
+		return errs
+	}
+
+	// Either host or path or both not empty, validate.
+	if data.Host == "" && data.Path != "" || data.Host != "" && data.Path == "" {
+		return append(errs, errors.New("External cache Host and Path must both be specified"))
+	}
+	if strings.HasSuffix(data.Host, "/") {
+		return append(errs, errors.New(fmt.Sprintf("External cache Host '%s' must not end with a path separator", data.Host)))
+	}
+	if strings.ContainsAny(data.Host, "://") {
+		return append(errs, errors.New(fmt.Sprintf("External cache Host must not specify a protocol. '%s'", data.Host)))
+	}
+	if !strings.HasPrefix(data.Path, "/") {
+		return append(errs, errors.New(fmt.Sprintf("External cache Path '%s' must begin with a path separator", data.Path)))
+	}
+
+	urlObj, err := url.Parse("https://" + data.Host + data.Path)
+	if err != nil {
+		return append(errs, errors.New(fmt.Sprintf("External cache Path validation error: %s ", err.Error())))
+	}
+	if urlObj.Host != data.Host {
+		return append(errs, errors.New(fmt.Sprintf("External cache Host '%s' is invalid", data.Host)))
+	}
+	if urlObj.Path != data.Path {
+		return append(errs, errors.New("External cache Path is invalid"))
+	}
+
+	return errs
+}
+
 // LimitAuctionTimeout returns the min of requested or cfg.MaxAuctionTimeout.
 // Both values treat "0" as "infinite".
 func (cfg *AuctionTimeouts) LimitAuctionTimeout(requested time.Duration) time.Duration {

config/config_test.go

@@ -14,6 +14,91 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestExternalCacheURLValidate(t *testing.T) {
+	testCases := []struct {
+		desc      string
+		data      ExternalCache
+		expErrors int
+	}{
+		{
+			desc:      "With http://",
+			data:      ExternalCache{Host: "http://www.google.com", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "Without http://",
+			data:      ExternalCache{Host: "www.google.com", Path: "/path/v1"},
+			expErrors: 0,
+		},
+		{
+			desc:      "No scheme but '//' prefix",
+			data:      ExternalCache{Host: "//www.google.com", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "// appears twice",
+			data:      ExternalCache{Host: "//www.google.com//", Path: "path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "Host has an only // value",
+			data:      ExternalCache{Host: "//", Path: "path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "only scheme host, valid path",
+			data:      ExternalCache{Host: "http://", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "No host, path only",
+			data:      ExternalCache{Host: "", Path: "path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "No host, nor path",
+			data:      ExternalCache{Host: "", Path: ""},
+			expErrors: 0,
+		},
+		{
+			desc:      "Invalid http at the end",
+			data:      ExternalCache{Host: "www.google.com", Path: "http://"},
+			expErrors: 1,
+		},
+		{
+			desc:      "Host has an unknown scheme",
+			data:      ExternalCache{Host: "unknownscheme://host", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "Wrong colon side in scheme",
+			data:      ExternalCache{Host: "http//:www.appnexus.com", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "Missing '/' in scheme",
+			data:      ExternalCache{Host: "http:/www.appnexus.com", Path: "/path/v1"},
+			expErrors: 1,
+		},
+		{
+			desc:      "host with scheme, no path",
+			data:      ExternalCache{Host: "http://www.appnexus.com", Path: ""},
+			expErrors: 1,
+		},
+		{
+			desc:      "scheme, no host nor path",
+			data:      ExternalCache{Host: "http://", Path: ""},
+			expErrors: 1,
+		},
+	}
+	for _, test := range testCases {
+		var errs configErrors
+		errs = test.data.validate(errs)
+
+		assert.Equal(t, test.expErrors, len(errs), "Test case threw unexpected number of errors. Desc: %s errMsg = %v \n", test.desc, errs)
+	}
+}
+
 func TestDefaults(t *testing.T) {
 	v := viper.New()
 	SetupViper(v, "")
@@ -66,7 +151,7 @@ cache:
   query: uuid=%PBS_CACHE_UUID%
 external_cache:
   host: www.externalprebidcache.net
-  path: endpoints/cache
+  path: /endpoints/cache
 http_client:
   max_connections_per_host: 10
   max_idle_connections: 500
@@ -223,7 +308,7 @@ func TestFullConfig(t *testing.T) {
 	cmpStrings(t, "cache.host", cfg.CacheURL.Host, "prebidcache.net")
 	cmpStrings(t, "cache.query", cfg.CacheURL.Query, "uuid=%PBS_CACHE_UUID%")
 	cmpStrings(t, "external_cache.host", cfg.ExtCacheURL.Host, "www.externalprebidcache.net")
-	cmpStrings(t, "external_cache.path", cfg.ExtCacheURL.Path, "endpoints/cache")
+	cmpStrings(t, "external_cache.path", cfg.ExtCacheURL.Path, "/endpoints/cache")
 	cmpInts(t, "http_client.max_connections_per_host", cfg.Client.MaxConnsPerHost, 10)
 	cmpInts(t, "http_client.max_idle_connections", cfg.Client.MaxIdleConns, 500)
 	cmpInts(t, "http_client.max_idle_connections_per_host", cfg.Client.MaxIdleConnsPerHost, 20)