enhance health validataion (#16549)

Signed-off-by: Wang Yan <wangyan@vmware.com>

src/lib/endpoint.go

@@ -23,7 +23,7 @@ import (
 )
 
 // ValidateHTTPURL checks whether the provided string is a valid HTTP URL.
-// If it it, return the URL in format "scheme://host:port" to avoid the SSRF
+// If it is, return the URL in format "scheme://host:port" to avoid the SSRF
 func ValidateHTTPURL(s string) (string, error) {
 	s = strings.Trim(s, " ")
 	s = strings.TrimRight(s, "/")
@@ -33,8 +33,7 @@ func ValidateHTTPURL(s string) (string, error) {
 	if !strings.Contains(s, "://") {
 		s = "http://" + s
 	}
-
-	url, err := url.ParseRequestURI(s)
+	url, err := url.Parse(s)
 	if err != nil {
 		return "", errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("invalid URL: %s", err.Error())
 	}

src/lib/endpoint_test.go

@@ -0,0 +1,48 @@
+package lib
+
+import (
+	"testing"
+)
+
+var testcases = []struct {
+	url         string
+	expectedUrl string
+	valid       bool
+}{
+	{"http://harbor.foo.com", "http://harbor.foo.com", true},
+	{"http://harbor.foo.com/", "http://harbor.foo.com", true},
+	{"http://harbor.foo.com/path", "http://harbor.foo.com/path", true},
+	{"/", "", false},
+	{"foo.html", "http://foo.html", true},
+	{"*", "http://*", true},
+	{"http://127.0.0.1/", "http://127.0.0.1", true},
+	{"http://127.0.0.1:8080/", "http://127.0.0.1:8080", true},
+	{"http://[fe80::1]/", "http://[fe80::1]", true},
+	{"http://[fe80::1]:8080/", "http://[fe80::1]:8080", true},
+
+	{"http://[fe80::1%25en0]/", "http://[fe80::1%en0]", true},
+	{"http://[fe80::1%25en0]:8080/", "http://[fe80::1%en0]:8080", true},
+	{"http://[fe80::1%25%65%6e%301-._~]/", "http://[fe80::1%en01-._~]", true},
+	{"http://[fe80::1%25%65%6e%301-._~]:8080/", "http://[fe80::1%en01-._~]:8080", true},
+
+	{"http://127.0.0.%31/", "", false},
+	{"http://127.0.0.%31:8080/", "", false},
+	{"http://10.0.0.1/test.txt#/api/version", "http://10.0.0.1/test.txt", true},
+}
+
+func TestValidateHTTPURL(t *testing.T) {
+	for _, test := range testcases {
+		url, err := ValidateHTTPURL(test.url)
+		if test.valid {
+			if err != nil {
+				t.Errorf("ValidateHTTPURL:%q gave err %v; want no error", test.url, err)
+			}
+			if url != test.expectedUrl {
+				t.Errorf("ValidateHTTPURL:%q gave %s; want %s", test.url, url, test.expectedUrl)
+			}
+		} else if !test.valid && err == nil {
+			t.Errorf("ValidateHTTPURL:%q gave <nil> error; want some error", test.url)
+		}
+	}
+
+}

src/pkg/p2p/preheat/provider/dragonfly.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	common_http "github.com/goharbor/harbor/src/common/http"
+	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/pkg/p2p/preheat/models/provider"
 	"github.com/goharbor/harbor/src/pkg/p2p/preheat/provider/auth"
 	"github.com/goharbor/harbor/src/pkg/p2p/preheat/provider/client"
@@ -56,7 +57,11 @@ func (dd *DragonflyDriver) GetHealth() (*DriverStatus, error) {
 	}
 
 	url := fmt.Sprintf("%s%s", strings.TrimSuffix(dd.instance.Endpoint, "/"), healthCheckEndpoint)
-	_, err := client.GetHTTPClient(dd.instance.Insecure).Get(url, dd.getCred(), nil, nil)
+	url, err := lib.ValidateHTTPURL(url)
+	if err != nil {
+		return nil, err
+	}
+	_, err = client.GetHTTPClient(dd.instance.Insecure).Get(url, dd.getCred(), nil, nil)
 	if err != nil {
 		// Unhealthy
 		return nil, err

src/pkg/p2p/preheat/provider/kraken.go

@@ -3,6 +3,7 @@ package provider
 import (
 	"errors"
 	"fmt"
+	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/pkg/p2p/preheat/models/notification"
 	"strings"
 	"time"
@@ -44,7 +45,11 @@ func (kd *KrakenDriver) GetHealth() (*DriverStatus, error) {
 	}
 
 	url := fmt.Sprintf("%s%s", strings.TrimSuffix(kd.instance.Endpoint, "/"), krakenHealthPath)
-	_, err := client.GetHTTPClient(kd.instance.Insecure).Get(url, kd.getCred(), nil, nil)
+	url, err := lib.ValidateHTTPURL(url)
+	if err != nil {
+		return nil, err
+	}
+	_, err = client.GetHTTPClient(kd.instance.Insecure).Get(url, kd.getCred(), nil, nil)
 	if err != nil {
 		// Unhealthy
 		return nil, err

src/pkg/scan/dao/scanner/model.go

@@ -16,10 +16,9 @@ package scanner
 
 import (
 	"encoding/json"
-	"net/url"
-	"strings"
 	"time"
 
+	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/scan/rest/auth"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
@@ -105,10 +104,11 @@ func (r *Registration) Validate(checkUUID bool) error {
 		return errors.New("missing registration name")
 	}
 
-	err := checkURL(r.URL)
+	url, err := lib.ValidateHTTPURL(r.URL)
 	if err != nil {
 		return errors.Wrap(err, "scanner registration validate")
 	}
+	r.URL = url
 
 	if len(r.Auth) > 0 &&
 		r.Auth != auth.Basic &&
@@ -199,19 +199,3 @@ func (r *Registration) GetRegistryAuthorizationType() string {
 
 	return auth
 }
-
-// Check the registration URL with url package
-func checkURL(u string) error {
-	if len(strings.TrimSpace(u)) == 0 {
-		return errors.New("empty url")
-	}
-
-	uri, err := url.Parse(u)
-	if err == nil {
-		if uri.Scheme != "http" && uri.Scheme != "https" {
-			err = errors.New("invalid scheme")
-		}
-	}
-
-	return err
-}

src/pkg/scan/dao/scanner/model_test.go

@@ -70,7 +70,7 @@ func (suite *ModelTestSuite) TestValidate() {
 
 	r.URL = "a.b.c"
 	err = r.Validate(true)
-	require.Error(suite.T(), err)
+	require.NoError(suite.T(), err)
 
 	r.URL = "http://a.b.c"
 	err = r.Validate(true)