From ccdb7bc286d318cd39dd9ffe7204f0b749e4fe86 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 23 Apr 2018 20:27:33 -0700 Subject: [PATCH] Fix s3 compatibility fixes for getBucketLocation,headBucket,deleteBucket (#5842) - getBucketLocation - headBucket - deleteBucket Should return 404 or NoSuchBucket even for invalid bucket names, invalid bucket names are only validated during MakeBucket operation --- cmd/bucket-handlers_test.go | 2 +- cmd/bucket-policy-handlers_test.go | 16 ++++++------ cmd/fs-v1.go | 14 +++++----- cmd/fs-v1_test.go | 19 ++++++++++---- cmd/gateway/azure/gateway-azure.go | 18 ++++++------- cmd/gateway/oss/gateway-oss.go | 5 ---- cmd/gateway/s3/gateway-s3.go | 22 ++++++++-------- cmd/object-api-listobjects_test.go | 8 +++--- cmd/object-api-multipart_test.go | 42 +++++++++++++++--------------- cmd/object-api-putobject_test.go | 8 +++--- cmd/posix.go | 8 ++++-- cmd/posix_test.go | 34 ++++++++++++------------ cmd/web-handlers_test.go | 7 ++--- cmd/xl-v1-bucket.go | 11 -------- 14 files changed, 107 insertions(+), 107 deletions(-) diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go index 6d983b9c9..052b59c2a 100644 --- a/cmd/bucket-handlers_test.go +++ b/cmd/bucket-handlers_test.go @@ -313,7 +313,7 @@ func testListMultipartUploadsHandler(obj ObjectLayer, instanceType, bucketName s maxUploads: "0", accessKey: credentials.AccessKey, secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusNotFound, shouldPass: false, }, // Test case - 2. diff --git a/cmd/bucket-policy-handlers_test.go b/cmd/bucket-policy-handlers_test.go index bb6775995..387741a06 100644 --- a/cmd/bucket-policy-handlers_test.go +++ b/cmd/bucket-policy-handlers_test.go @@ -357,7 +357,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string // Test case - 8. // non-existent bucket is used. // writing BucketPolicy should fail. - // should result is 500 InternalServerError. + // should result is 404 StatusNotFound { bucketName: "non-existent-bucket", bucketPolicyReader: bytes.NewReader([]byte(fmt.Sprintf(bucketPolicyTemplate, "non-existent-bucket", "non-existent-bucket"))), @@ -368,9 +368,9 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string expectedRespStatus: http.StatusNotFound, }, // Test case - 9. - // invalid bucket name is used. + // non-existent bucket is used (with invalid bucket name) // writing BucketPolicy should fail. - // should result is 400 StatusBadRequest. + // should result is 404 StatusNotFound { bucketName: ".invalid-bucket", bucketPolicyReader: bytes.NewReader([]byte(fmt.Sprintf(bucketPolicyTemplate, ".invalid-bucket", ".invalid-bucket"))), @@ -378,7 +378,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string policyLen: len(fmt.Sprintf(bucketPolicyTemplate, bucketName, bucketName)), accessKey: credentials.AccessKey, secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusNotFound, }, } @@ -535,13 +535,13 @@ func testGetBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string expectedRespStatus: http.StatusNotFound, }, // Test case - 3. - // Case with invalid bucket name. + // Case with non-existent bucket name. { bucketName: ".invalid-bucket-name", accessKey: credentials.AccessKey, secretKey: credentials.SecretKey, expectedBucketPolicy: "", - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusNotFound, }, } // Iterating over the cases, fetching the policy and validating the response. @@ -742,12 +742,12 @@ func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName str expectedRespStatus: http.StatusNotFound, }, // Test case - 3. - // Case with invalid bucket name. + // Case with non-existent-bucket. { bucketName: ".invalid-bucket-name", accessKey: credentials.AccessKey, secretKey: credentials.SecretKey, - expectedRespStatus: http.StatusBadRequest, + expectedRespStatus: http.StatusNotFound, }, } // Iterating over the cases and deleting the bucket policy and then asserting response. diff --git a/cmd/fs-v1.go b/cmd/fs-v1.go index 76ec0d37a..9ccb61e19 100644 --- a/cmd/fs-v1.go +++ b/cmd/fs-v1.go @@ -195,13 +195,9 @@ func (fs *FSObjects) ClearLocks(ctx context.Context, info []VolumeLockInfo) erro // corresponding valid bucket names on the backend in a platform // compatible way for all operating systems. func (fs *FSObjects) getBucketDir(ctx context.Context, bucket string) (string, error) { - // Verify if bucket is valid. - if !IsValidBucketName(bucket) { - err := BucketNameInvalid{Bucket: bucket} - logger.LogIf(ctx, err) - return "", err + if bucket == "" || bucket == "." || bucket == ".." { + return "", errVolumeNotFound } - bucketDir := pathJoin(fs.fsPath, bucket) return bucketDir, nil } @@ -226,6 +222,12 @@ func (fs *FSObjects) MakeBucketWithLocation(ctx context.Context, bucket, locatio return err } defer bucketLock.Unlock() + // Verify if bucket is valid. + if !IsValidBucketName(bucket) { + err := BucketNameInvalid{Bucket: bucket} + logger.LogIf(ctx, err) + return err + } bucketDir, err := fs.getBucketDir(ctx, bucket) if err != nil { return toObjectErr(err, bucket) diff --git a/cmd/fs-v1_test.go b/cmd/fs-v1_test.go index 6f6dfecea..35291913d 100644 --- a/cmd/fs-v1_test.go +++ b/cmd/fs-v1_test.go @@ -166,7 +166,15 @@ func TestFSGetBucketInfo(t *testing.T) { fs := obj.(*FSObjects) bucketName := "bucket" - obj.MakeBucketWithLocation(context.Background(), bucketName, "") + err := obj.MakeBucketWithLocation(context.Background(), "a", "") + if !isSameType(err, BucketNameInvalid{}) { + t.Fatal("BucketNameInvalid error not returned") + } + + err = obj.MakeBucketWithLocation(context.Background(), bucketName, "") + if err != nil { + t.Fatal(err) + } // Test with valid parameters info, err := fs.GetBucketInfo(context.Background(), bucketName) @@ -177,10 +185,10 @@ func TestFSGetBucketInfo(t *testing.T) { t.Fatalf("wrong bucket name, expected: %s, found: %s", bucketName, info.Name) } - // Test with inexistant bucket + // Test with non-existent bucket _, err = fs.GetBucketInfo(context.Background(), "a") - if !isSameType(err, BucketNameInvalid{}) { - t.Fatal("BucketNameInvalid error not returned") + if !isSameType(err, BucketNotFound{}) { + t.Fatal("BucketNotFound error not returned") } // Check for buckets and should get disk not found. @@ -319,9 +327,10 @@ func TestFSDeleteBucket(t *testing.T) { } // Test with an invalid bucket name - if err = fs.DeleteBucket(context.Background(), "fo"); !isSameType(err, BucketNameInvalid{}) { + if err = fs.DeleteBucket(context.Background(), "fo"); !isSameType(err, BucketNotFound{}) { t.Fatal("Unexpected error: ", err) } + // Test with an inexistant bucket if err = fs.DeleteBucket(context.Background(), "foobucket"); !isSameType(err, BucketNotFound{}) { t.Fatal("Unexpected error: ", err) diff --git a/cmd/gateway/azure/gateway-azure.go b/cmd/gateway/azure/gateway-azure.go index 34d59864a..7c8f4b1b7 100644 --- a/cmd/gateway/azure/gateway-azure.go +++ b/cmd/gateway/azure/gateway-azure.go @@ -431,6 +431,15 @@ func (a *azureObjects) StorageInfo(ctx context.Context) (si minio.StorageInfo) { // MakeBucketWithLocation - Create a new container on azure backend. func (a *azureObjects) MakeBucketWithLocation(ctx context.Context, bucket, location string) error { + // Verify if bucket (container-name) is valid. + // IsValidBucketName has same restrictions as container names mentioned + // in azure documentation, so we will simply use the same function here. + // Ref - https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata + if !minio.IsValidBucketName(bucket) { + logger.LogIf(ctx, minio.BucketNameInvalid{Bucket: bucket}) + return minio.BucketNameInvalid{Bucket: bucket} + } + container := a.client.GetContainerReference(bucket) err := container.Create(&storage.CreateContainerOptions{ Access: storage.ContainerAccessTypePrivate, @@ -441,15 +450,6 @@ func (a *azureObjects) MakeBucketWithLocation(ctx context.Context, bucket, locat // GetBucketInfo - Get bucket metadata.. func (a *azureObjects) GetBucketInfo(ctx context.Context, bucket string) (bi minio.BucketInfo, e error) { - // Verify if bucket (container-name) is valid. - // IsValidBucketName has same restrictions as container names mentioned - // in azure documentation, so we will simply use the same function here. - // Ref - https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata - if !minio.IsValidBucketName(bucket) { - logger.LogIf(ctx, minio.BucketNameInvalid{Bucket: bucket}) - return bi, minio.BucketNameInvalid{Bucket: bucket} - } - // Azure does not have an equivalent call, hence use // ListContainers with prefix resp, err := a.client.ListContainers(storage.ListContainersParameters{ diff --git a/cmd/gateway/oss/gateway-oss.go b/cmd/gateway/oss/gateway-oss.go index 9362313fc..b7bd9100e 100644 --- a/cmd/gateway/oss/gateway-oss.go +++ b/cmd/gateway/oss/gateway-oss.go @@ -368,11 +368,6 @@ func (l *ossObjects) MakeBucketWithLocation(ctx context.Context, bucket, locatio // ossGeBucketInfo gets bucket metadata. func ossGeBucketInfo(ctx context.Context, client *oss.Client, bucket string) (bi minio.BucketInfo, err error) { - if !ossIsValidBucketName(bucket) { - logger.LogIf(ctx, minio.BucketNameInvalid{Bucket: bucket}) - return bi, minio.BucketNameInvalid{Bucket: bucket} - } - bgir, err := client.GetBucketInfo(bucket) if err != nil { logger.LogIf(ctx, err) diff --git a/cmd/gateway/s3/gateway-s3.go b/cmd/gateway/s3/gateway-s3.go index 34f37d629..1de1a26bc 100644 --- a/cmd/gateway/s3/gateway-s3.go +++ b/cmd/gateway/s3/gateway-s3.go @@ -172,16 +172,6 @@ func (l *s3Objects) StorageInfo(ctx context.Context) (si minio.StorageInfo) { // MakeBucket creates a new container on S3 backend. func (l *s3Objects) MakeBucketWithLocation(ctx context.Context, bucket, location string) error { - err := l.Client.MakeBucket(bucket, location) - if err != nil { - logger.LogIf(ctx, err) - return minio.ErrorRespToObjectError(err, bucket) - } - return err -} - -// GetBucketInfo gets bucket metadata.. -func (l *s3Objects) GetBucketInfo(ctx context.Context, bucket string) (bi minio.BucketInfo, e error) { // Verify if bucket name is valid. // We are using a separate helper function here to validate bucket // names instead of IsValidBucketName() because there is a possibility @@ -191,9 +181,19 @@ func (l *s3Objects) GetBucketInfo(ctx context.Context, bucket string) (bi minio. // Ref - http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html if s3utils.CheckValidBucketName(bucket) != nil { logger.LogIf(ctx, minio.BucketNameInvalid{Bucket: bucket}) - return bi, minio.BucketNameInvalid{Bucket: bucket} + return minio.BucketNameInvalid{Bucket: bucket} } + err := l.Client.MakeBucket(bucket, location) + if err != nil { + logger.LogIf(ctx, err) + return minio.ErrorRespToObjectError(err, bucket) + } + return err +} + +// GetBucketInfo gets bucket metadata.. +func (l *s3Objects) GetBucketInfo(ctx context.Context, bucket string) (bi minio.BucketInfo, e error) { buckets, err := l.Client.ListBuckets() if err != nil { logger.LogIf(ctx, err) diff --git a/cmd/object-api-listobjects_test.go b/cmd/object-api-listobjects_test.go index 9faabc1cc..2521df86a 100644 --- a/cmd/object-api-listobjects_test.go +++ b/cmd/object-api-listobjects_test.go @@ -425,10 +425,10 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) { shouldPass bool }{ // Test cases with invalid bucket names ( Test number 1-4 ). - {".test", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, + {".test", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: ".test"}, false}, + {"Test", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "Test"}, false}, + {"---", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "---"}, false}, + {"ad", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "ad"}, false}, // Using an existing file for bucket name, but its not a directory (5). {"simple-file.txt", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "simple-file.txt"}, false}, // Valid bucket names, but they donot exist (6-8). diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go index 05e065f9d..f01b6c7b4 100644 --- a/cmd/object-api-multipart_test.go +++ b/cmd/object-api-multipart_test.go @@ -111,7 +111,7 @@ func testObjectAbortMultipartUpload(obj ObjectLayer, instanceType string, t Test uploadID string expectedErrType error }{ - {"--", object, uploadID, BucketNameInvalid{}}, + {"--", object, uploadID, BucketNotFound{}}, {bucket, "\\", uploadID, ObjectNameInvalid{}}, {"foo", object, uploadID, BucketNotFound{}}, {bucket, object, "foo-foo", InvalidUploadID{}}, @@ -293,11 +293,11 @@ func testObjectAPIPutObjectPart(obj ObjectLayer, instanceType string, t TestErrH }{ // Test case 1-4. // Cases with invalid bucket name. - {".test", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: .test")}, - {"------", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: ------")}, + {".test", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: .test")}, + {"------", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: ------")}, {"$this-is-not-valid-too", "obj", "", 1, "", "", "", 0, false, "", - fmt.Errorf("%s", "Bucket name invalid: $this-is-not-valid-too")}, - {"a", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: a")}, + fmt.Errorf("%s", "Bucket not found: $this-is-not-valid-too")}, + {"a", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: a")}, // Test case - 5. // Case with invalid object names. {bucket, "", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Object name invalid: minio-bucket#")}, @@ -1103,10 +1103,10 @@ func testListMultipartUploads(obj ObjectLayer, instanceType string, t TestErrHan shouldPass bool }{ // Test cases with invalid bucket names ( Test number 1-4 ). - {".test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, + {".test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: ".test"}, false}, + {"Test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "Test"}, false}, + {"---", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "---"}, false}, + {"ad", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "ad"}, false}, // Valid bucket names, but they donot exist (Test number 5-7). {"volatile-bucket-1", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, {"volatile-bucket-2", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-2"}, false}, @@ -1404,10 +1404,10 @@ func testListObjectPartsDiskNotFound(obj ObjectLayer, instanceType string, disks shouldPass bool }{ // Test cases with invalid bucket names (Test number 1-4). - {".test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, + {".test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: ".test"}, false}, + {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "Test"}, false}, + {"---", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "---"}, false}, + {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "ad"}, false}, // Test cases for listing uploadID with single part. // Valid bucket names, but they donot exist (Test number 5-7). {"volatile-bucket-1", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, @@ -1642,10 +1642,10 @@ func testListObjectParts(obj ObjectLayer, instanceType string, t TestErrHandler) shouldPass bool }{ // Test cases with invalid bucket names (Test number 1-4). - {".test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, + {".test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: ".test"}, false}, + {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "Test"}, false}, + {"---", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "---"}, false}, + {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "ad"}, false}, // Test cases for listing uploadID with single part. // Valid bucket names, but they donot exist (Test number 5-7). {"volatile-bucket-1", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, @@ -1863,10 +1863,10 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T shouldPass bool }{ // Test cases with invalid bucket names (Test number 1-4). - {".test", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: ".test"}, false}, - {"Test", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "Test"}, false}, - {"---", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "---"}, false}, - {"ad", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "ad"}, false}, + {".test", "", "", []CompletePart{}, "", BucketNotFound{Bucket: ".test"}, false}, + {"Test", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "Test"}, false}, + {"---", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "---"}, false}, + {"ad", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "ad"}, false}, // Test cases for listing uploadID with single part. // Valid bucket names, but they donot exist (Test number 5-7). {"volatile-bucket-1", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "volatile-bucket-1"}, false}, diff --git a/cmd/object-api-putobject_test.go b/cmd/object-api-putobject_test.go index 6c63354f1..353556ac5 100644 --- a/cmd/object-api-putobject_test.go +++ b/cmd/object-api-putobject_test.go @@ -80,11 +80,11 @@ func testObjectAPIPutObject(obj ObjectLayer, instanceType string, t TestErrHandl }{ // Test case 1-4. // Cases with invalid bucket name. - {".test", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: ".test"}}, - {"------", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: "------"}}, + {".test", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: ".test"}}, + {"------", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: "------"}}, {"$this-is-not-valid-too", "obj", []byte(""), nil, "", 0, "", - BucketNameInvalid{Bucket: "$this-is-not-valid-too"}}, - {"a", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: "a"}}, + BucketNotFound{Bucket: "$this-is-not-valid-too"}}, + {"a", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: "a"}}, // Test case - 5. // Case with invalid object names. diff --git a/cmd/posix.go b/cmd/posix.go index 797913a64..650fc0841 100644 --- a/cmd/posix.go +++ b/cmd/posix.go @@ -261,8 +261,8 @@ func (s *posix) DiskInfo() (info disk.Info, err error) { // compatible way for all operating systems. If volume is not found // an error is generated. func (s *posix) getVolDir(volume string) (string, error) { - if !isValidVolname(volume) { - return "", errInvalidArgument + if volume == "" || volume == "." || volume == ".." { + return "", errVolumeNotFound } volumeDir := pathJoin(s.diskPath, volume) return volumeDir, nil @@ -301,6 +301,10 @@ func (s *posix) MakeVol(volume string) (err error) { return err } + if !isValidVolname(volume) { + return errInvalidArgument + } + volumeDir, err := s.getVolDir(volume) if err != nil { return err diff --git a/cmd/posix_test.go b/cmd/posix_test.go index 70a840375..b89178a12 100644 --- a/cmd/posix_test.go +++ b/cmd/posix_test.go @@ -231,7 +231,7 @@ func TestPosixReadAll(t *testing.T) { { volume: "ab", path: "as-file", - err: errInvalidArgument, + err: errVolumeNotFound, }, } @@ -477,7 +477,7 @@ func TestPosixDeleteVol(t *testing.T) { { volName: "ab", ioErrCount: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, } @@ -588,7 +588,7 @@ func TestPosixStatVol(t *testing.T) { { volName: "ab", ioErrCount: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, } @@ -756,7 +756,7 @@ func TestPosixPosixListDir(t *testing.T) { srcVol: "ab", srcPath: "success-file", ioErrCnt: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, // TestPosix case - 4. // TestPosix case with io error count > max limit. @@ -902,7 +902,7 @@ func TestPosixDeleteFile(t *testing.T) { srcVol: "my", srcPath: "success-file", ioErrCnt: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, // TestPosix case - 5. // TestPosix case with non-existent volume. @@ -1068,7 +1068,7 @@ func TestPosixReadFile(t *testing.T) { }, // Empty volume name. - 12 { - "", "myobject", 14, 1, nil, errInvalidArgument, + "", "myobject", 14, 1, nil, errVolumeNotFound, }, // Empty filename name. - 13 { @@ -1374,7 +1374,7 @@ func TestPosixAppendFile(t *testing.T) { // TestPosix case with invalid volume name. // A valid volume name should be atleast of size 3. err = posixStorage.AppendFile("bn", "yes", []byte("hello, world")) - if err != errInvalidArgument { + if err != errVolumeNotFound { t.Fatalf("expected: \"Invalid argument error\", got: \"%s\"", err) } @@ -1472,19 +1472,19 @@ func TestPosixPrepareFile(t *testing.T) { } } - // TestPosix case with invalid file size which should be strictly positive - err = posixStorage.PrepareFile("bn", "yes", -3) - if err != errInvalidArgument { - t.Fatalf("should fail: %v", err) - } - // TestPosix case with invalid volume name. // A valid volume name should be atleast of size 3. err = posixStorage.PrepareFile("bn", "yes", 16) - if err != errInvalidArgument { + if err != errVolumeNotFound { t.Fatalf("expected: \"Invalid argument error\", got: \"%s\"", err) } + // TestPosix case with invalid file size which should be strictly positive + err = posixStorage.PrepareFile("success-vol", "yes", -3) + if err != errInvalidArgument { + t.Fatalf("should fail: %v", err) + } + // TestPosix case with IO error count > max limit. // setting ioErrCnt to 6. @@ -1681,7 +1681,7 @@ func TestPosixRenameFile(t *testing.T) { srcPath: "file4", destPath: "new-path/", ioErrCnt: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, // TestPosix case - 14. // TestPosix case with invalid destination volume name. Length should be atleast 3. @@ -1692,7 +1692,7 @@ func TestPosixRenameFile(t *testing.T) { srcPath: "file4", destPath: "new-path/", ioErrCnt: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, // TestPosix case - 15. // TestPosix case with invalid destination volume name. Length should be atleast 3. @@ -1703,7 +1703,7 @@ func TestPosixRenameFile(t *testing.T) { srcPath: "file4", destPath: "new-path/", ioErrCnt: 0, - expectedErr: errInvalidArgument, + expectedErr: errVolumeNotFound, }, // TestPosix case - 16. // TestPosix case with the parent of the destination being a file. diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index 7bd1c93c9..4be9fa98f 100644 --- a/cmd/web-handlers_test.go +++ b/cmd/web-handlers_test.go @@ -355,10 +355,11 @@ func testDeleteBucketWebHandler(obj ObjectLayer, instanceType string, t TestErrH // Empty string = no error expect string }{ - {"", false, token, "Bucket Name is invalid"}, + {"", false, token, "The specified bucket does not exist."}, {".", false, "auth", "Authentication failed"}, - {".", false, token, "Bucket Name . is invalid"}, - {"ab", false, token, "Bucket Name ab is invalid"}, + {".", false, token, "The specified bucket . does not exist."}, + {"..", false, token, "The specified bucket .. does not exist."}, + {"ab", false, token, "The specified bucket ab does not exist."}, {"minio", false, "false token", "Authentication failed"}, {"minio", false, token, "specified bucket minio does not exist"}, {bucketName, false, token, ""}, diff --git a/cmd/xl-v1-bucket.go b/cmd/xl-v1-bucket.go index 6b58cdaa2..f1321d295 100644 --- a/cmd/xl-v1-bucket.go +++ b/cmd/xl-v1-bucket.go @@ -163,11 +163,6 @@ func (xl xlObjects) GetBucketInfo(ctx context.Context, bucket string) (bi Bucket return bi, e } defer bucketLock.RUnlock() - // Verify if bucket is valid. - if !IsValidBucketName(bucket) { - return bi, BucketNameInvalid{Bucket: bucket} - } - bucketInfo, err := xl.getBucketInfo(ctx, bucket) if err != nil { return bi, toObjectErr(err, bucket) @@ -228,18 +223,12 @@ func (xl xlObjects) ListBuckets(ctx context.Context) ([]BucketInfo, error) { // DeleteBucket - deletes a bucket. func (xl xlObjects) DeleteBucket(ctx context.Context, bucket string) error { - bucketLock := xl.nsMutex.NewNSLock(bucket, "") if err := bucketLock.GetLock(globalObjectTimeout); err != nil { return err } defer bucketLock.Unlock() - // Verify if bucket is valid. - if !IsValidBucketName(bucket) { - return BucketNameInvalid{Bucket: bucket} - } - // Collect if all disks report volume not found. var wg = &sync.WaitGroup{} var dErrs = make([]error, len(xl.getDisks()))