Merge pull request #1255 from hackintoshrao/list-object-prefix-response

api: ListObject - Changing to empty response when prefixDir doesn't exist

pkg/fs/fs-bucket-listobjects.go

@@ -39,8 +39,9 @@ func (fs Filesystem) ListObjects(bucket, prefix, marker, delimiter string, maxKe
 	}
 
 	bucket = fs.denormalizeBucket(bucket)
-
+	bucketDir := filepath.Join(fs.path, bucket)
-	if status, err := isDirExist(filepath.Join(fs.path, bucket)); !status {
+	// Verify if bucket exists.
+	if status, err := isDirExist(bucketDir); !status {
 		if err == nil {
 			// File exists, but its not a directory.
 			return result, probe.NewError(BucketNotFound{Bucket: bucket})
@@ -51,11 +52,16 @@ func (fs Filesystem) ListObjects(bucket, prefix, marker, delimiter string, maxKe
 			return result, probe.NewError(err)
 		}
 	}
+	if !IsValidObjectPrefix(prefix) {
+		return result, probe.NewError(ObjectNameInvalid{Bucket: bucket, Object: prefix})
+	}
 
+	// Verify if delimiter is anything other than '/', which we do not support.
 	if delimiter != "" && delimiter != "/" {
 		return result, probe.NewError(fmt.Errorf("delimiter '%s' is not supported", delimiter))
 	}
 
+	// Marker is set unescape.
 	if marker != "" {
 		if markerUnescaped, err := url.QueryUnescape(marker); err == nil {
 			marker = markerUnescaped
@@ -74,11 +80,24 @@ func (fs Filesystem) ListObjects(bucket, prefix, marker, delimiter string, maxKe
 		return result, nil
 	}
 
+	// Over flowing maxkeys - reset to listObjectsLimit.
 	if maxKeys < 0 || maxKeys > listObjectsLimit {
 		maxKeys = listObjectsLimit
 	}
 
-	bucketDir := filepath.Join(fs.path, bucket)
+	// Verify if prefix exists.
+	prefixDir := filepath.Dir(filepath.FromSlash(prefix))
+	rootDir := filepath.Join(bucketDir, prefixDir)
+	_, err := isDirExist(rootDir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			// Prefix does not exist, not an error just respond empty
+			// list response.
+			return result, nil
+		}
+		// Rest errors should be treated as failure.
+		return result, probe.NewError(err)
+	}
 
 	recursive := true
 	skipDir := true
@@ -87,9 +106,6 @@ func (fs Filesystem) ListObjects(bucket, prefix, marker, delimiter string, maxKe
 		recursive = false
 	}
 
-	prefixDir := filepath.Dir(filepath.FromSlash(prefix))
-	rootDir := filepath.Join(bucketDir, prefixDir)
-
 	// Maximum 1000 objects returned in a single to listObjects.
 	// Further calls will set right marker value to continue reading the rest of the objectList.
 	// popListObjectCh returns nil if the call to ListObject is done for the first time.

pkg/fs/fs-bucket-listobjects_test.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -459,8 +458,8 @@ func TestListObjects(t *testing.T) {
 		// The prefix and marker combination to be valid it should satisy strings.HasPrefix(marker, prefix).
 		{"test-bucket-list-object", "asia", "europe-object", "", 0, ListObjectsResult{}, fmt.Errorf("Invalid combination of marker '%s' and prefix '%s'", "europe-object", "asia"), false},
 		// Setting a non-existing directory to be prefix (14-15).
-		{"empty-bucket", "europe/france/", "", "", 1, ListObjectsResult{}, fmt.Errorf("%s", filepath.FromSlash("/empty-bucket/europe/france")), false},
+		{"empty-bucket", "europe/france/", "", "", 1, ListObjectsResult{}, nil, true},
-		{"empty-bucket", "europe/tunisia/", "", "", 1, ListObjectsResult{}, fmt.Errorf("%s", filepath.FromSlash("/empty-bucket/europe/tunisia")), false},
+		{"empty-bucket", "europe/tunisia/", "", "", 1, ListObjectsResult{}, nil, true},
 		// Testing on empty bucket, that is, bucket without any objects in it (16).
 		{"empty-bucket", "", "", "", 0, ListObjectsResult{}, nil, true},
 		// Setting maxKeys to negative value (17-18).

pkg/fs/fs-utils.go

@@ -51,3 +51,15 @@ func IsValidObjectName(object string) bool {
 	}
 	return true
 }
+
+// IsValidObjectPrefix verifies whether the prefix is a valid object name.
+// Its valid to have a empty prefix.
+func IsValidObjectPrefix(object string) bool {
+	// Prefix can be empty.
+	if object == "" {
+		return true
+	}
+	// Verify if prefix is a valid object name.
+	return IsValidObjectName(object)
+
+}