Remove regexp usage (#3456)

This patch removes regexp usage in cacheControlHandler.ServeHTTP() and
server-main.checkEndpointsSyntax()

cmd/generic-handlers.go

@@ -19,7 +19,6 @@ package cmd
 import (
 	"net/http"
 	"path"
-	"regexp"
 	"strings"
 	"time"
 
@@ -142,21 +141,18 @@ func setBrowserCacheControlHandler(h http.Handler) http.Handler {
 func (h cacheControlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if r.Method == "GET" && guessIsBrowserReq(r) && globalIsBrowserEnabled {
 		// For all browser requests set appropriate Cache-Control policies
-		match, err := regexp.Match(reservedBucket+`/([^/]+\.js|favicon.ico)`, []byte(r.URL.Path))
-		if err != nil {
-			errorIf(err, "Unable to match incoming URL %s", r.URL)
-			writeErrorResponse(w, r, ErrInternalError, r.URL.Path)
-			return
-		}
-		if match {
-			// For assets set cache expiry of one year. For each release, the name
-			// of the asset name will change and hence it can not be served from cache.
-			w.Header().Set("Cache-Control", "max-age=31536000")
-		} else if strings.HasPrefix(r.URL.Path, reservedBucket+"/") {
-			// For non asset requests we serve index.html which will never be cached.
-			w.Header().Set("Cache-Control", "no-store")
+		if strings.HasPrefix(r.URL.Path, reservedBucket+"/") {
+			if strings.HasSuffix(r.URL.Path, ".js") || r.URL.Path == reservedBucket+"/favicon.ico" {
+				// For assets set cache expiry of one year. For each release, the name
+				// of the asset name will change and hence it can not be served from cache.
+				w.Header().Set("Cache-Control", "max-age=31536000")
+			} else {
+				// For non asset requests we serve index.html which will never be cached.
+				w.Header().Set("Cache-Control", "no-store")
+			}
 		}
 	}
+
 	h.handler.ServeHTTP(w, r)
 }
 

cmd/server-main.go

@@ -23,9 +23,9 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path"
 	"strings"
 
-	"regexp"
 	"runtime"
 
 	"github.com/minio/cli"
@@ -260,35 +260,46 @@ func isDistributedSetup(eps []*url.URL) bool {
 	return false
 }
 
-// Check if the endpoints are following expected syntax, i.e
-// valid scheme, valid path across all platforms.
-func checkEndpointsSyntax(eps []*url.URL, disks []string) error {
-	for i, u := range eps {
-		switch u.Scheme {
-		case "":
-			// "/" is not allowed.
-			if u.Path == "" || u.Path == "/" {
-				return fmt.Errorf("Root path is not allowed : %s (%s)", u.Path, disks[i])
+// Check if endpoint is in expected syntax by valid scheme/path across all platforms.
+func checkEndpointURL(endpointURL *url.URL) (err error) {
+	// applicable to all OS.
+	if endpointURL.Scheme == "" || endpointURL.Scheme == "http" || endpointURL.Scheme == "https" {
+		urlPath := path.Clean(endpointURL.Path)
+		if urlPath == "" || urlPath == "." || urlPath == "/" || urlPath == `\` {
+			err = fmt.Errorf("Empty or root path is not allowed")
+		}
+
+		return err
+	}
+
+	// Applicable to Windows only.
+	if runtime.GOOS == "windows" {
+		// On Windows, endpoint can be a path with drive eg. C:\Export and its URL.Scheme is 'C'.
+		// Check if URL.Scheme is a single letter alphabet to represent a drive.
+		// Note: URL.Parse() converts scheme into lower case always.
+		if len(endpointURL.Scheme) == 1 && endpointURL.Scheme[0] >= 'a' && endpointURL.Scheme[0] <= 'z' {
+			// If endpoint is C:\ or C:\export, URL.Path does not have path information like \ or \export
+			// hence we directly work with endpoint.
+			urlPath := strings.SplitN(path.Clean(endpointURL.String()), ":", 2)[1]
+			if urlPath == "" || urlPath == "." || urlPath == "/" || urlPath == `\` {
+				err = fmt.Errorf("Empty or root path is not allowed")
 			}
-		case "http", "https":
-			// "http://server1/" is not allowed
-			if u.Path == "" || u.Path == "/" || u.Path == "\\" {
-				return fmt.Errorf("Root path is not allowed : %s (%s)", u.Path, disks[i])
-			}
-		default:
-			if runtime.GOOS == "windows" {
-				// On windows for "C:\export" scheme will be "C"
-				matched, err := regexp.MatchString("^[a-zA-Z]$", u.Scheme)
-				if err != nil {
-					return fmt.Errorf("Invalid scheme : %s (%s), ERROR %s", u.Scheme, disks[i], err)
-				}
-				if matched {
-					break
-				}
-			}
-			return fmt.Errorf("Invalid scheme : %s (%s)", u.Scheme, disks[i])
+
+			return err
 		}
 	}
+
+	return fmt.Errorf("Invalid scheme")
+}
+
+// Check if endpoints are in expected syntax by valid scheme/path across all platforms.
+func checkEndpointsSyntax(eps []*url.URL, disks []string) error {
+	for i, u := range eps {
+		if err := checkEndpointURL(u); err != nil {
+			return fmt.Errorf("%s: %s (%s)", err.Error(), u.Path, disks[i])
+		}
+	}
+
 	return nil
 }
 

cmd/server-main_test.go

@@ -200,48 +200,52 @@ func TestParseStorageEndpoints(t *testing.T) {
 // Test check endpoints syntax function for syntax verification
 // across various scenarios of inputs.
 func TestCheckEndpointsSyntax(t *testing.T) {
-	var testCases []string
-	if runtime.GOOS == "windows" {
-		testCases = []string{
-			"\\export",
-			"D:\\export",
-			"D:\\",
-			"D:",
-			"\\",
-		}
-	} else {
-		testCases = []string{
-			"/export",
-		}
-	}
-	testCasesCommon := []string{
+	successCases := []string{
 		"export",
+		"/export",
 		"http://localhost/export",
 		"https://localhost/export",
 	}
-	testCases = append(testCases, testCasesCommon...)
-	for _, disk := range testCases {
+
+	failureCases := []string{
+		"/",
+		"http://localhost",
+		"http://localhost/",
+		"ftp://localhost/export",
+		"server:/export",
+	}
+
+	if runtime.GOOS == "windows" {
+		successCases = append(successCases,
+			`\export`,
+			`D:\export`,
+		)
+
+		failureCases = append(failureCases,
+			"D:",
+			`D:\`,
+			`\`,
+		)
+	}
+
+	for _, disk := range successCases {
 		eps, err := parseStorageEndpoints([]string{disk})
 		if err != nil {
 			t.Fatalf("Unable to parse %s, error %s", disk, err)
 		}
 		if err = checkEndpointsSyntax(eps, []string{disk}); err != nil {
-			t.Errorf("Invalid endpoints %s", err)
+			t.Errorf("expected: <nil>, got: %s", err)
 		}
 	}
-	eps, err := parseStorageEndpoints([]string{"/"})
-	if err != nil {
-		t.Fatalf("Unable to parse /, error %s", err)
-	}
-	if err = checkEndpointsSyntax(eps, []string{"/"}); err == nil {
-		t.Error("Should fail, passed instead")
-	}
-	eps, err = parseStorageEndpoints([]string{"http://localhost/"})
-	if err != nil {
-		t.Fatalf("Unable to parse http://localhost/, error %s", err)
-	}
-	if err = checkEndpointsSyntax(eps, []string{"http://localhost/"}); err == nil {
-		t.Error("Should fail, passed instead")
+
+	for _, disk := range failureCases {
+		eps, err := parseStorageEndpoints([]string{disk})
+		if err != nil {
+			t.Fatalf("Unable to parse %s, error %s", disk, err)
+		}
+		if err = checkEndpointsSyntax(eps, []string{disk}); err == nil {
+			t.Errorf("expected: <error>, got: <nil>")
+		}
 	}
 }