fs: Do not return reservedBucket names in ListBuckets() (minio#3754)

Make sure to skip reserved bucket names in `ListBuckets()`
current code didn't skip this properly and also generalize
this behavior for both XL and FS.

cmd/admin-rpc-client.go

@@ -150,7 +150,7 @@ func makeAdminPeers(eps []*url.URL) adminPeers {
 				secretKey:       serverCred.SecretKey,
 				serverAddr:      ep.Host,
 				secureConn:      globalIsSSL,
-				serviceEndpoint: path.Join(reservedBucket, adminPath),
+				serviceEndpoint: path.Join(minioReservedBucketPath, adminPath),
 				serviceName:     "Admin",
 			}
 

cmd/admin-rpc-server.go

@@ -141,7 +141,7 @@ func registerAdminRPCRouter(mux *router.Router) error {
 	if err != nil {
 		return traceError(err)
 	}
-	adminRouter := mux.NewRoute().PathPrefix(reservedBucket).Subrouter()
+	adminRouter := mux.NewRoute().PathPrefix(minioReservedBucketPath).Subrouter()
 	adminRouter.Path(adminPath).Handler(adminRPCServer)
 	return nil
 }

cmd/browser-peer-rpc.go

@@ -111,7 +111,7 @@ func updateCredsOnPeers(creds credential) map[string]error {
 				secretKey:       serverCred.SecretKey,
 				serverAddr:      peers[ix],
 				secureConn:      globalIsSSL,
-				serviceEndpoint: path.Join(reservedBucket, browserPeerPath),
+				serviceEndpoint: path.Join(minioReservedBucketPath, browserPeerPath),
 				serviceName:     "BrowserPeer",
 			})
 

cmd/browser-peer-rpc_test.go

@@ -36,7 +36,7 @@ func (s *TestRPCBrowserPeerSuite) SetUpSuite(c *testing.T) {
 		serverAddr:      s.testServer.Server.Listener.Addr().String(),
 		accessKey:       s.testServer.AccessKey,
 		secretKey:       s.testServer.SecretKey,
-		serviceEndpoint: path.Join(reservedBucket, browserPeerPath),
+		serviceEndpoint: path.Join(minioReservedBucketPath, browserPeerPath),
 		serviceName:     "BrowserPeer",
 	}
 }

cmd/browser-rpc-router.go

@@ -45,7 +45,7 @@ func registerBrowserPeerRPCRouter(mux *router.Router) error {
 		return traceError(err)
 	}
 
-	bpRouter := mux.NewRoute().PathPrefix(reservedBucket).Subrouter()
+	bpRouter := mux.NewRoute().PathPrefix(minioReservedBucketPath).Subrouter()
 	bpRouter.Path(browserPeerPath).Handler(bpRPCServer)
 	return nil
 }

cmd/bucket-notification-utils.go

@@ -253,19 +253,19 @@ func unmarshalSqsARN(queueARN string) (mSqs arnSQS) {
 	}
 	sqsType := strings.TrimPrefix(queueARN, minioSqs+serverConfig.GetRegion()+":")
 	switch {
-	case strings.HasSuffix(sqsType, queueTypeAMQP):
+	case hasSuffix(sqsType, queueTypeAMQP):
 		mSqs.Type = queueTypeAMQP
-	case strings.HasSuffix(sqsType, queueTypeNATS):
+	case hasSuffix(sqsType, queueTypeNATS):
 		mSqs.Type = queueTypeNATS
-	case strings.HasSuffix(sqsType, queueTypeElastic):
+	case hasSuffix(sqsType, queueTypeElastic):
 		mSqs.Type = queueTypeElastic
-	case strings.HasSuffix(sqsType, queueTypeRedis):
+	case hasSuffix(sqsType, queueTypeRedis):
 		mSqs.Type = queueTypeRedis
-	case strings.HasSuffix(sqsType, queueTypePostgreSQL):
+	case hasSuffix(sqsType, queueTypePostgreSQL):
 		mSqs.Type = queueTypePostgreSQL
-	case strings.HasSuffix(sqsType, queueTypeKafka):
+	case hasSuffix(sqsType, queueTypeKafka):
 		mSqs.Type = queueTypeKafka
-	case strings.HasSuffix(sqsType, queueTypeWebhook):
+	case hasSuffix(sqsType, queueTypeWebhook):
 		mSqs.Type = queueTypeWebhook
 	} // Add more queues here.
 	mSqs.AccountID = strings.TrimSuffix(sqsType, ":"+mSqs.Type)

cmd/errors.go

@@ -86,10 +86,10 @@ func traceError(e error, errs ...error) error {
 		fn := runtime.FuncForPC(pc)
 		file, line := fn.FileLine(pc)
 		name := fn.Name()
-		if strings.HasSuffix(name, "ServeHTTP") {
+		if hasSuffix(name, "ServeHTTP") {
 			break
 		}
-		if strings.HasSuffix(name, "runtime.") {
+		if hasSuffix(name, "runtime.") {
 			break
 		}
 

cmd/fs-v1-multipart.go

@@ -278,7 +278,7 @@ func (fs fsObjects) listMultipartUploads(bucket, prefix, keyMarker, uploadIDMark
 			}
 
 			entry := strings.TrimPrefix(walkResult.entry, retainSlash(bucket))
-			if strings.HasSuffix(walkResult.entry, slashSeparator) {
+			if hasSuffix(walkResult.entry, slashSeparator) {
 				uploads = append(uploads, uploadMetadata{
 					Object: entry,
 				})
@@ -314,7 +314,7 @@ func (fs fsObjects) listMultipartUploads(bucket, prefix, keyMarker, uploadIDMark
 	for _, upload := range uploads {
 		var objectName string
 		var uploadID string
-		if strings.HasSuffix(upload.Object, slashSeparator) {
+		if hasSuffix(upload.Object, slashSeparator) {
 			// All directory entries are common prefixes.
 			uploadID = "" // Upload ids are empty for CommonPrefixes.
 			objectName = upload.Object

cmd/fs-v1.go

@@ -19,7 +19,6 @@ package cmd
 import (
 	"crypto/md5"
 	"encoding/hex"
-	"errors"
 	"fmt"
 	"hash"
 	"io"
@@ -291,12 +290,11 @@ func (fs fsObjects) ListBuckets() ([]BucketInfo, error) {
 		return nil, toObjectErr(traceError(errDiskNotFound))
 	}
 
-	var invalidBucketNames []string
 	for _, entry := range entries {
-		if entry == minioMetaBucket+"/" || !strings.HasSuffix(entry, slashSeparator) {
+		// Ignore all reserved bucket names and invalid bucket names.
+		if isReservedOrInvalidBucket(entry) {
 			continue
 		}
-
 		var fi os.FileInfo
 		fi, err = fsStatDir(pathJoin(fs.fsPath, entry))
 		if err != nil {
@@ -310,24 +308,13 @@ func (fs fsObjects) ListBuckets() ([]BucketInfo, error) {
 			return nil, err
 		}
 
-		if !IsValidBucketName(fi.Name()) {
-			invalidBucketNames = append(invalidBucketNames, fi.Name())
-			continue
-		}
-
 		bucketInfos = append(bucketInfos, BucketInfo{
 			Name: fi.Name(),
-			// As os.Stat() doesn't carry other than ModTime(), use ModTime() as CreatedTime.
+			// As os.Stat() doesnt carry CreatedTime, use ModTime() as CreatedTime.
 			Created: fi.ModTime(),
 		})
 	}
 
-	// Print a user friendly message if we indeed skipped certain directories which are
-	// incompatible with S3's bucket name restrictions.
-	if len(invalidBucketNames) > 0 {
-		errorIf(errors.New("One or more invalid bucket names found"), "Skipping %s", invalidBucketNames)
-	}
-
 	// Sort bucket infos by bucket name.
 	sort.Sort(byBucketName(bucketInfos))
 
@@ -780,7 +767,7 @@ func (fs fsObjects) ListObjects(bucket, prefix, marker, delimiter string, maxKey
 
 	// Convert entry to ObjectInfo
 	entryToObjectInfo := func(entry string) (objInfo ObjectInfo, err error) {
-		if strings.HasSuffix(entry, slashSeparator) {
+		if hasSuffix(entry, slashSeparator) {
 			// Object name needs to be full path.
 			objInfo.Name = entry
 			objInfo.IsDir = true
@@ -804,7 +791,7 @@ func (fs fsObjects) ListObjects(bucket, prefix, marker, delimiter string, maxKey
 			// bucket argument is unused as we don't need to StatFile
 			// to figure if it's a file, just need to check that the
 			// object string does not end with "/".
-			return !strings.HasSuffix(object, slashSeparator)
+			return !hasSuffix(object, slashSeparator)
 		}
 		listDir := fs.listDirFactory(isLeaf)
 		walkResultCh = startTreeWalk(bucket, prefix, marker, recursive, listDir, isLeaf, endWalkCh)

cmd/generic-handlers.go

@@ -68,7 +68,8 @@ func (h requestSizeLimitHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 
 // Reserved bucket.
 const (
-	reservedBucket = "/minio"
+	minioReservedBucket     = "minio"
+	minioReservedBucketPath = "/" + minioReservedBucket
 )
 
 // Adds redirect rules for incoming requests.
@@ -86,16 +87,16 @@ func setBrowserRedirectHandler(h http.Handler) http.Handler {
 // serves only limited purpose on redirect-handler for
 // browser requests.
 func getRedirectLocation(urlPath string) (rLocation string) {
-	if urlPath == reservedBucket {
-		rLocation = reservedBucket + "/"
+	if urlPath == minioReservedBucketPath {
+		rLocation = minioReservedBucketPath + "/"
 	}
 	if contains([]string{
 		"/",
 		"/webrpc",
 		"/login",
 		"/favicon.ico",
 	}, urlPath) {
-		rLocation = reservedBucket + urlPath
+		rLocation = minioReservedBucketPath + urlPath
 	}
 	return rLocation
 }
@@ -143,8 +144,8 @@ func setBrowserCacheControlHandler(h http.Handler) http.Handler {
 func (h cacheControlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if r.Method == httpGET && guessIsBrowserReq(r) && globalIsBrowserEnabled {
 		// For all browser requests set appropriate Cache-Control policies
-		if hasPrefix(r.URL.Path, reservedBucket+"/") {
-			if hasSuffix(r.URL.Path, ".js") || r.URL.Path == reservedBucket+"/favicon.ico" {
+		if hasPrefix(r.URL.Path, minioReservedBucketPath+"/") {
+			if hasSuffix(r.URL.Path, ".js") || r.URL.Path == minioReservedBucketPath+"/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")
@@ -160,17 +161,17 @@ func (h cacheControlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 // Adds verification for incoming paths.
 type minioPrivateBucketHandler struct {
-	handler        http.Handler
-	reservedBucket string
+	handler            http.Handler
+	reservedBucketPath string
 }
 
 func setPrivateBucketHandler(h http.Handler) http.Handler {
-	return minioPrivateBucketHandler{handler: h, reservedBucket: reservedBucket}
+	return minioPrivateBucketHandler{h, minioReservedBucketPath}
 }
 
 func (h minioPrivateBucketHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// For all non browser requests, reject access to 'reservedBucket'.
-	if !guessIsBrowserReq(r) && path.Clean(r.URL.Path) == reservedBucket {
+	// For all non browser requests, reject access to 'reservedBucketPath'.
+	if !guessIsBrowserReq(r) && path.Clean(r.URL.Path) == h.reservedBucketPath {
 		writeErrorResponse(w, ErrAllAccessDisabled, r.URL)
 		return
 	}

cmd/generic-handlers_test.go

@@ -29,28 +29,28 @@ func TestRedirectLocation(t *testing.T) {
 	}{
 		{
 			// 1. When urlPath is '/minio'
-			urlPath:  reservedBucket,
-			location: reservedBucket + "/",
+			urlPath:  minioReservedBucketPath,
+			location: minioReservedBucketPath + "/",
 		},
 		{
 			// 2. When urlPath is '/'
 			urlPath:  "/",
-			location: reservedBucket + "/",
+			location: minioReservedBucketPath + "/",
 		},
 		{
 			// 3. When urlPath is '/webrpc'
 			urlPath:  "/webrpc",
-			location: reservedBucket + "/webrpc",
+			location: minioReservedBucketPath + "/webrpc",
 		},
 		{
 			// 4. When urlPath is '/login'
 			urlPath:  "/login",
-			location: reservedBucket + "/login",
+			location: minioReservedBucketPath + "/login",
 		},
 		{
 			// 5. When urlPath is '/favicon.ico'
 			urlPath:  "/favicon.ico",
-			location: reservedBucket + "/favicon.ico",
+			location: minioReservedBucketPath + "/favicon.ico",
 		},
 		{
 			// 6. When urlPath is '/unknown'

cmd/humanized-duration_test.go

@@ -17,7 +17,6 @@
 package cmd
 
 import (
-	"strings"
 	"testing"
 	"time"
 )
@@ -26,10 +25,10 @@ import (
 func TestHumanizedDuration(t *testing.T) {
 	duration := time.Duration(90487000000000)
 	humanDuration := timeDurationToHumanizedDuration(duration)
-	if !strings.HasSuffix(humanDuration.String(), "seconds") {
+	if !hasSuffix(humanDuration.String(), "seconds") {
 		t.Fatal("Stringer method for humanized duration should have seconds.", humanDuration.String())
 	}
-	if strings.HasSuffix(humanDuration.StringShort(), "seconds") {
+	if hasSuffix(humanDuration.StringShort(), "seconds") {
 		t.Fatal("StringShorter method for humanized duration should not have seconds.", humanDuration.StringShort())
 	}
 
@@ -42,9 +41,9 @@ func TestHumanizedDuration(t *testing.T) {
 		t.Fatalf("Expected %#v, got %#v incorrect conversion of duration to humanized form",
 			expectedHumanSecDuration, humanSecDuration)
 	}
-	if strings.HasSuffix(humanSecDuration.String(), "days") ||
-		strings.HasSuffix(humanSecDuration.String(), "hours") ||
-		strings.HasSuffix(humanSecDuration.String(), "minutes") {
+	if hasSuffix(humanSecDuration.String(), "days") ||
+		hasSuffix(humanSecDuration.String(), "hours") ||
+		hasSuffix(humanSecDuration.String(), "minutes") {
 		t.Fatal("Stringer method for humanized duration should have only seconds.", humanSecDuration.String())
 	}
 
@@ -57,7 +56,7 @@ func TestHumanizedDuration(t *testing.T) {
 		t.Fatalf("Expected %#v, got %#v incorrect conversion of duration to humanized form",
 			expectedHumanMinDuration, humanMinDuration)
 	}
-	if strings.HasSuffix(humanMinDuration.String(), "hours") {
+	if hasSuffix(humanMinDuration.String(), "hours") {
 		t.Fatal("Stringer method for humanized duration should have only minutes.", humanMinDuration.String())
 	}
 
@@ -70,7 +69,7 @@ func TestHumanizedDuration(t *testing.T) {
 		t.Fatalf("Expected %#v, got %#v incorrect conversion of duration to humanized form",
 			expectedHumanHourDuration, humanHourDuration)
 	}
-	if strings.HasSuffix(humanHourDuration.String(), "days") {
+	if hasSuffix(humanHourDuration.String(), "days") {
 		t.Fatal("Stringer method for humanized duration should have hours.", humanHourDuration.String())
 	}
 }

cmd/lock-rpc-server.go

@@ -30,7 +30,7 @@ import (
 
 const (
 	// Lock rpc server endpoint.
-	lockRPCPath = "/minio/lock"
+	lockRPCPath = "/lock"
 
 	// Lock maintenance interval.
 	lockMaintenanceInterval = 1 * time.Minute // 1 minute.
@@ -122,8 +122,8 @@ func registerStorageLockers(mux *router.Router, lockServers []*lockServer) error
 		if err := lockRPCServer.RegisterName("Dsync", lockServer); err != nil {
 			return traceError(err)
 		}
-		lockRouter := mux.PathPrefix(reservedBucket).Subrouter()
-		lockRouter.Path(path.Join("/lock", lockServer.rpcPath)).Handler(lockRPCServer)
+		lockRouter := mux.PathPrefix(minioReservedBucketPath).Subrouter()
+		lockRouter.Path(path.Join(lockRPCPath, lockServer.rpcPath)).Handler(lockRPCServer)
 	}
 	return nil
 }

cmd/object-api-common.go

@@ -20,7 +20,6 @@ import (
 	"net"
 	"net/url"
 	"runtime"
-	"strings"
 	"sync"
 	"time"
 
@@ -59,7 +58,7 @@ func isRemoteDisk(disk StorageAPI) bool {
 // if size == 0 and object ends with slashSeparator then
 // returns true.
 func isObjectDir(object string, size int64) bool {
-	return strings.HasSuffix(object, slashSeparator) && size == 0
+	return hasSuffix(object, slashSeparator) && size == 0
 }
 
 // Converts just bucket, object metadata into ObjectInfo datatype.
@@ -284,7 +283,7 @@ func cleanupDir(storage StorageAPI, volume, dirPath string) error {
 	var delFunc func(string) error
 	// Function to delete entries recursively.
 	delFunc = func(entryPath string) error {
-		if !strings.HasSuffix(entryPath, slashSeparator) {
+		if !hasSuffix(entryPath, slashSeparator) {
 			// Delete the file entry.
 			return traceError(storage.DeleteFile(volume, entryPath))
 		}

cmd/object-api-utils.go

@@ -129,7 +129,7 @@ func retainSlash(s string) string {
 func pathJoin(elem ...string) string {
 	trailingSlash := ""
 	if len(elem) > 0 {
-		if strings.HasSuffix(elem[len(elem)-1], slashSeparator) {
+		if hasSuffix(elem[len(elem)-1], slashSeparator) {
 			trailingSlash = "/"
 		}
 	}
@@ -180,6 +180,15 @@ func hasSuffix(s string, suffix string) bool {
 	return strings.HasSuffix(s, suffix)
 }
 
+// Ignores all reserved bucket names or invalid bucket names.
+func isReservedOrInvalidBucket(bucketEntry string) bool {
+	bucketEntry = strings.TrimSuffix(bucketEntry, slashSeparator)
+	if !IsValidBucketName(bucketEntry) {
+		return true
+	}
+	return bucketEntry == minioMetaBucket || bucketEntry == minioReservedBucket
+}
+
 // byBucketName is a collection satisfying sort.Interface.
 type byBucketName []BucketInfo