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
master
Harshavardhana 7 years ago committed by Nitish Tiwari
parent 954142a98f
commit ccdb7bc286
  1. 2
      cmd/bucket-handlers_test.go
  2. 16
      cmd/bucket-policy-handlers_test.go
  3. 14
      cmd/fs-v1.go
  4. 19
      cmd/fs-v1_test.go
  5. 18
      cmd/gateway/azure/gateway-azure.go
  6. 5
      cmd/gateway/oss/gateway-oss.go
  7. 22
      cmd/gateway/s3/gateway-s3.go
  8. 8
      cmd/object-api-listobjects_test.go
  9. 42
      cmd/object-api-multipart_test.go
  10. 8
      cmd/object-api-putobject_test.go
  11. 8
      cmd/posix.go
  12. 34
      cmd/posix_test.go
  13. 7
      cmd/web-handlers_test.go
  14. 11
      cmd/xl-v1-bucket.go

@ -313,7 +313,7 @@ func testListMultipartUploadsHandler(obj ObjectLayer, instanceType, bucketName s
maxUploads: "0", maxUploads: "0",
accessKey: credentials.AccessKey, accessKey: credentials.AccessKey,
secretKey: credentials.SecretKey, secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusBadRequest, expectedRespStatus: http.StatusNotFound,
shouldPass: false, shouldPass: false,
}, },
// Test case - 2. // Test case - 2.

@ -357,7 +357,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
// Test case - 8. // Test case - 8.
// non-existent bucket is used. // non-existent bucket is used.
// writing BucketPolicy should fail. // writing BucketPolicy should fail.
// should result is 500 InternalServerError. // should result is 404 StatusNotFound
{ {
bucketName: "non-existent-bucket", bucketName: "non-existent-bucket",
bucketPolicyReader: bytes.NewReader([]byte(fmt.Sprintf(bucketPolicyTemplate, "non-existent-bucket", "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, expectedRespStatus: http.StatusNotFound,
}, },
// Test case - 9. // Test case - 9.
// invalid bucket name is used. // non-existent bucket is used (with invalid bucket name)
// writing BucketPolicy should fail. // writing BucketPolicy should fail.
// should result is 400 StatusBadRequest. // should result is 404 StatusNotFound
{ {
bucketName: ".invalid-bucket", bucketName: ".invalid-bucket",
bucketPolicyReader: bytes.NewReader([]byte(fmt.Sprintf(bucketPolicyTemplate, ".invalid-bucket", ".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)), policyLen: len(fmt.Sprintf(bucketPolicyTemplate, bucketName, bucketName)),
accessKey: credentials.AccessKey, accessKey: credentials.AccessKey,
secretKey: credentials.SecretKey, secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusBadRequest, expectedRespStatus: http.StatusNotFound,
}, },
} }
@ -535,13 +535,13 @@ func testGetBucketPolicyHandler(obj ObjectLayer, instanceType, bucketName string
expectedRespStatus: http.StatusNotFound, expectedRespStatus: http.StatusNotFound,
}, },
// Test case - 3. // Test case - 3.
// Case with invalid bucket name. // Case with non-existent bucket name.
{ {
bucketName: ".invalid-bucket-name", bucketName: ".invalid-bucket-name",
accessKey: credentials.AccessKey, accessKey: credentials.AccessKey,
secretKey: credentials.SecretKey, secretKey: credentials.SecretKey,
expectedBucketPolicy: "", expectedBucketPolicy: "",
expectedRespStatus: http.StatusBadRequest, expectedRespStatus: http.StatusNotFound,
}, },
} }
// Iterating over the cases, fetching the policy and validating the response. // 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, expectedRespStatus: http.StatusNotFound,
}, },
// Test case - 3. // Test case - 3.
// Case with invalid bucket name. // Case with non-existent-bucket.
{ {
bucketName: ".invalid-bucket-name", bucketName: ".invalid-bucket-name",
accessKey: credentials.AccessKey, accessKey: credentials.AccessKey,
secretKey: credentials.SecretKey, secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusBadRequest, expectedRespStatus: http.StatusNotFound,
}, },
} }
// Iterating over the cases and deleting the bucket policy and then asserting response. // Iterating over the cases and deleting the bucket policy and then asserting response.

@ -195,13 +195,9 @@ func (fs *FSObjects) ClearLocks(ctx context.Context, info []VolumeLockInfo) erro
// corresponding valid bucket names on the backend in a platform // corresponding valid bucket names on the backend in a platform
// compatible way for all operating systems. // compatible way for all operating systems.
func (fs *FSObjects) getBucketDir(ctx context.Context, bucket string) (string, error) { func (fs *FSObjects) getBucketDir(ctx context.Context, bucket string) (string, error) {
// Verify if bucket is valid. if bucket == "" || bucket == "." || bucket == ".." {
if !IsValidBucketName(bucket) { return "", errVolumeNotFound
err := BucketNameInvalid{Bucket: bucket}
logger.LogIf(ctx, err)
return "", err
} }
bucketDir := pathJoin(fs.fsPath, bucket) bucketDir := pathJoin(fs.fsPath, bucket)
return bucketDir, nil return bucketDir, nil
} }
@ -226,6 +222,12 @@ func (fs *FSObjects) MakeBucketWithLocation(ctx context.Context, bucket, locatio
return err return err
} }
defer bucketLock.Unlock() 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) bucketDir, err := fs.getBucketDir(ctx, bucket)
if err != nil { if err != nil {
return toObjectErr(err, bucket) return toObjectErr(err, bucket)

@ -166,7 +166,15 @@ func TestFSGetBucketInfo(t *testing.T) {
fs := obj.(*FSObjects) fs := obj.(*FSObjects)
bucketName := "bucket" 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 // Test with valid parameters
info, err := fs.GetBucketInfo(context.Background(), bucketName) 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) 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") _, err = fs.GetBucketInfo(context.Background(), "a")
if !isSameType(err, BucketNameInvalid{}) { if !isSameType(err, BucketNotFound{}) {
t.Fatal("BucketNameInvalid error not returned") t.Fatal("BucketNotFound error not returned")
} }
// Check for buckets and should get disk not found. // Check for buckets and should get disk not found.
@ -319,9 +327,10 @@ func TestFSDeleteBucket(t *testing.T) {
} }
// Test with an invalid bucket name // 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) t.Fatal("Unexpected error: ", err)
} }
// Test with an inexistant bucket // Test with an inexistant bucket
if err = fs.DeleteBucket(context.Background(), "foobucket"); !isSameType(err, BucketNotFound{}) { if err = fs.DeleteBucket(context.Background(), "foobucket"); !isSameType(err, BucketNotFound{}) {
t.Fatal("Unexpected error: ", err) t.Fatal("Unexpected error: ", err)

@ -431,6 +431,15 @@ func (a *azureObjects) StorageInfo(ctx context.Context) (si minio.StorageInfo) {
// MakeBucketWithLocation - Create a new container on azure backend. // MakeBucketWithLocation - Create a new container on azure backend.
func (a *azureObjects) MakeBucketWithLocation(ctx context.Context, bucket, location string) error { 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) container := a.client.GetContainerReference(bucket)
err := container.Create(&storage.CreateContainerOptions{ err := container.Create(&storage.CreateContainerOptions{
Access: storage.ContainerAccessTypePrivate, Access: storage.ContainerAccessTypePrivate,
@ -441,15 +450,6 @@ func (a *azureObjects) MakeBucketWithLocation(ctx context.Context, bucket, locat
// GetBucketInfo - Get bucket metadata.. // GetBucketInfo - Get bucket metadata..
func (a *azureObjects) GetBucketInfo(ctx context.Context, bucket string) (bi minio.BucketInfo, e error) { 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 // Azure does not have an equivalent call, hence use
// ListContainers with prefix // ListContainers with prefix
resp, err := a.client.ListContainers(storage.ListContainersParameters{ resp, err := a.client.ListContainers(storage.ListContainersParameters{

@ -368,11 +368,6 @@ func (l *ossObjects) MakeBucketWithLocation(ctx context.Context, bucket, locatio
// ossGeBucketInfo gets bucket metadata. // ossGeBucketInfo gets bucket metadata.
func ossGeBucketInfo(ctx context.Context, client *oss.Client, bucket string) (bi minio.BucketInfo, err error) { 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) bgir, err := client.GetBucketInfo(bucket)
if err != nil { if err != nil {
logger.LogIf(ctx, err) logger.LogIf(ctx, err)

@ -172,16 +172,6 @@ func (l *s3Objects) StorageInfo(ctx context.Context) (si minio.StorageInfo) {
// MakeBucket creates a new container on S3 backend. // MakeBucket creates a new container on S3 backend.
func (l *s3Objects) MakeBucketWithLocation(ctx context.Context, bucket, location string) error { 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. // Verify if bucket name is valid.
// We are using a separate helper function here to validate bucket // We are using a separate helper function here to validate bucket
// names instead of IsValidBucketName() because there is a possibility // 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 // Ref - http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
if s3utils.CheckValidBucketName(bucket) != nil { if s3utils.CheckValidBucketName(bucket) != nil {
logger.LogIf(ctx, minio.BucketNameInvalid{Bucket: bucket}) 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() buckets, err := l.Client.ListBuckets()
if err != nil { if err != nil {
logger.LogIf(ctx, err) logger.LogIf(ctx, err)

@ -425,10 +425,10 @@ func testListObjects(obj ObjectLayer, instanceType string, t TestErrHandler) {
shouldPass bool shouldPass bool
}{ }{
// Test cases with invalid bucket names ( Test number 1-4 ). // Test cases with invalid bucket names ( Test number 1-4 ).
{".test", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, {".test", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: ".test"}, false},
{"Test", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, {"Test", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "Test"}, false},
{"---", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, {"---", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "---"}, false},
{"ad", "", "", "", 0, ListObjectsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, {"ad", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "ad"}, false},
// Using an existing file for bucket name, but its not a directory (5). // Using an existing file for bucket name, but its not a directory (5).
{"simple-file.txt", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "simple-file.txt"}, false}, {"simple-file.txt", "", "", "", 0, ListObjectsInfo{}, BucketNotFound{Bucket: "simple-file.txt"}, false},
// Valid bucket names, but they donot exist (6-8). // Valid bucket names, but they donot exist (6-8).

@ -111,7 +111,7 @@ func testObjectAbortMultipartUpload(obj ObjectLayer, instanceType string, t Test
uploadID string uploadID string
expectedErrType error expectedErrType error
}{ }{
{"--", object, uploadID, BucketNameInvalid{}}, {"--", object, uploadID, BucketNotFound{}},
{bucket, "\\", uploadID, ObjectNameInvalid{}}, {bucket, "\\", uploadID, ObjectNameInvalid{}},
{"foo", object, uploadID, BucketNotFound{}}, {"foo", object, uploadID, BucketNotFound{}},
{bucket, object, "foo-foo", InvalidUploadID{}}, {bucket, object, "foo-foo", InvalidUploadID{}},
@ -293,11 +293,11 @@ func testObjectAPIPutObjectPart(obj ObjectLayer, instanceType string, t TestErrH
}{ }{
// Test case 1-4. // Test case 1-4.
// Cases with invalid bucket name. // Cases with invalid bucket name.
{".test", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: .test")}, {".test", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: .test")},
{"------", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: ------")}, {"------", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: ------")},
{"$this-is-not-valid-too", "obj", "", 1, "", "", "", 0, false, "", {"$this-is-not-valid-too", "obj", "", 1, "", "", "", 0, false, "",
fmt.Errorf("%s", "Bucket name invalid: $this-is-not-valid-too")}, fmt.Errorf("%s", "Bucket not found: $this-is-not-valid-too")},
{"a", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: a")}, {"a", "obj", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: a")},
// Test case - 5. // Test case - 5.
// Case with invalid object names. // Case with invalid object names.
{bucket, "", "", 1, "", "", "", 0, false, "", fmt.Errorf("%s", "Object name invalid: minio-bucket#")}, {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 shouldPass bool
}{ }{
// Test cases with invalid bucket names ( Test number 1-4 ). // Test cases with invalid bucket names ( Test number 1-4 ).
{".test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, {".test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: ".test"}, false},
{"Test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, {"Test", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "Test"}, false},
{"---", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, {"---", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "---"}, false},
{"ad", "", "", "", "", 0, ListMultipartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, {"ad", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "ad"}, false},
// Valid bucket names, but they donot exist (Test number 5-7). // Valid bucket names, but they donot exist (Test number 5-7).
{"volatile-bucket-1", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, {"volatile-bucket-1", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false},
{"volatile-bucket-2", "", "", "", "", 0, ListMultipartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-2"}, 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 shouldPass bool
}{ }{
// Test cases with invalid bucket names (Test number 1-4). // Test cases with invalid bucket names (Test number 1-4).
{".test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, {".test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: ".test"}, false},
{"Test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "Test"}, false},
{"---", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, {"---", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "---"}, false},
{"ad", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "ad"}, false},
// Test cases for listing uploadID with single part. // Test cases for listing uploadID with single part.
// Valid bucket names, but they donot exist (Test number 5-7). // Valid bucket names, but they donot exist (Test number 5-7).
{"volatile-bucket-1", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, {"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 shouldPass bool
}{ }{
// Test cases with invalid bucket names (Test number 1-4). // Test cases with invalid bucket names (Test number 1-4).
{".test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: ".test"}, false}, {".test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: ".test"}, false},
{"Test", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "Test"}, false}, {"Test", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "Test"}, false},
{"---", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "---"}, false}, {"---", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "---"}, false},
{"ad", "", "", 0, 0, ListPartsInfo{}, BucketNameInvalid{Bucket: "ad"}, false}, {"ad", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "ad"}, false},
// Test cases for listing uploadID with single part. // Test cases for listing uploadID with single part.
// Valid bucket names, but they donot exist (Test number 5-7). // Valid bucket names, but they donot exist (Test number 5-7).
{"volatile-bucket-1", "", "", 0, 0, ListPartsInfo{}, BucketNotFound{Bucket: "volatile-bucket-1"}, false}, {"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 shouldPass bool
}{ }{
// Test cases with invalid bucket names (Test number 1-4). // Test cases with invalid bucket names (Test number 1-4).
{".test", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: ".test"}, false}, {".test", "", "", []CompletePart{}, "", BucketNotFound{Bucket: ".test"}, false},
{"Test", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "Test"}, false}, {"Test", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "Test"}, false},
{"---", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "---"}, false}, {"---", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "---"}, false},
{"ad", "", "", []CompletePart{}, "", BucketNameInvalid{Bucket: "ad"}, false}, {"ad", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "ad"}, false},
// Test cases for listing uploadID with single part. // Test cases for listing uploadID with single part.
// Valid bucket names, but they donot exist (Test number 5-7). // Valid bucket names, but they donot exist (Test number 5-7).
{"volatile-bucket-1", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "volatile-bucket-1"}, false}, {"volatile-bucket-1", "", "", []CompletePart{}, "", BucketNotFound{Bucket: "volatile-bucket-1"}, false},

@ -80,11 +80,11 @@ func testObjectAPIPutObject(obj ObjectLayer, instanceType string, t TestErrHandl
}{ }{
// Test case 1-4. // Test case 1-4.
// Cases with invalid bucket name. // Cases with invalid bucket name.
{".test", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: ".test"}}, {".test", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: ".test"}},
{"------", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: "------"}}, {"------", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: "------"}},
{"$this-is-not-valid-too", "obj", []byte(""), nil, "", 0, "", {"$this-is-not-valid-too", "obj", []byte(""), nil, "", 0, "",
BucketNameInvalid{Bucket: "$this-is-not-valid-too"}}, BucketNotFound{Bucket: "$this-is-not-valid-too"}},
{"a", "obj", []byte(""), nil, "", 0, "", BucketNameInvalid{Bucket: "a"}}, {"a", "obj", []byte(""), nil, "", 0, "", BucketNotFound{Bucket: "a"}},
// Test case - 5. // Test case - 5.
// Case with invalid object names. // Case with invalid object names.

@ -261,8 +261,8 @@ func (s *posix) DiskInfo() (info disk.Info, err error) {
// compatible way for all operating systems. If volume is not found // compatible way for all operating systems. If volume is not found
// an error is generated. // an error is generated.
func (s *posix) getVolDir(volume string) (string, error) { func (s *posix) getVolDir(volume string) (string, error) {
if !isValidVolname(volume) { if volume == "" || volume == "." || volume == ".." {
return "", errInvalidArgument return "", errVolumeNotFound
} }
volumeDir := pathJoin(s.diskPath, volume) volumeDir := pathJoin(s.diskPath, volume)
return volumeDir, nil return volumeDir, nil
@ -301,6 +301,10 @@ func (s *posix) MakeVol(volume string) (err error) {
return err return err
} }
if !isValidVolname(volume) {
return errInvalidArgument
}
volumeDir, err := s.getVolDir(volume) volumeDir, err := s.getVolDir(volume)
if err != nil { if err != nil {
return err return err

@ -231,7 +231,7 @@ func TestPosixReadAll(t *testing.T) {
{ {
volume: "ab", volume: "ab",
path: "as-file", path: "as-file",
err: errInvalidArgument, err: errVolumeNotFound,
}, },
} }
@ -477,7 +477,7 @@ func TestPosixDeleteVol(t *testing.T) {
{ {
volName: "ab", volName: "ab",
ioErrCount: 0, ioErrCount: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
} }
@ -588,7 +588,7 @@ func TestPosixStatVol(t *testing.T) {
{ {
volName: "ab", volName: "ab",
ioErrCount: 0, ioErrCount: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
} }
@ -756,7 +756,7 @@ func TestPosixPosixListDir(t *testing.T) {
srcVol: "ab", srcVol: "ab",
srcPath: "success-file", srcPath: "success-file",
ioErrCnt: 0, ioErrCnt: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
// TestPosix case - 4. // TestPosix case - 4.
// TestPosix case with io error count > max limit. // TestPosix case with io error count > max limit.
@ -902,7 +902,7 @@ func TestPosixDeleteFile(t *testing.T) {
srcVol: "my", srcVol: "my",
srcPath: "success-file", srcPath: "success-file",
ioErrCnt: 0, ioErrCnt: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
// TestPosix case - 5. // TestPosix case - 5.
// TestPosix case with non-existent volume. // TestPosix case with non-existent volume.
@ -1068,7 +1068,7 @@ func TestPosixReadFile(t *testing.T) {
}, },
// Empty volume name. - 12 // Empty volume name. - 12
{ {
"", "myobject", 14, 1, nil, errInvalidArgument, "", "myobject", 14, 1, nil, errVolumeNotFound,
}, },
// Empty filename name. - 13 // Empty filename name. - 13
{ {
@ -1374,7 +1374,7 @@ func TestPosixAppendFile(t *testing.T) {
// TestPosix case with invalid volume name. // TestPosix case with invalid volume name.
// A valid volume name should be atleast of size 3. // A valid volume name should be atleast of size 3.
err = posixStorage.AppendFile("bn", "yes", []byte("hello, world")) err = posixStorage.AppendFile("bn", "yes", []byte("hello, world"))
if err != errInvalidArgument { if err != errVolumeNotFound {
t.Fatalf("expected: \"Invalid argument error\", got: \"%s\"", err) 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. // TestPosix case with invalid volume name.
// A valid volume name should be atleast of size 3. // A valid volume name should be atleast of size 3.
err = posixStorage.PrepareFile("bn", "yes", 16) err = posixStorage.PrepareFile("bn", "yes", 16)
if err != errInvalidArgument { if err != errVolumeNotFound {
t.Fatalf("expected: \"Invalid argument error\", got: \"%s\"", err) 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. // TestPosix case with IO error count > max limit.
// setting ioErrCnt to 6. // setting ioErrCnt to 6.
@ -1681,7 +1681,7 @@ func TestPosixRenameFile(t *testing.T) {
srcPath: "file4", srcPath: "file4",
destPath: "new-path/", destPath: "new-path/",
ioErrCnt: 0, ioErrCnt: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
// TestPosix case - 14. // TestPosix case - 14.
// TestPosix case with invalid destination volume name. Length should be atleast 3. // TestPosix case with invalid destination volume name. Length should be atleast 3.
@ -1692,7 +1692,7 @@ func TestPosixRenameFile(t *testing.T) {
srcPath: "file4", srcPath: "file4",
destPath: "new-path/", destPath: "new-path/",
ioErrCnt: 0, ioErrCnt: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
// TestPosix case - 15. // TestPosix case - 15.
// TestPosix case with invalid destination volume name. Length should be atleast 3. // TestPosix case with invalid destination volume name. Length should be atleast 3.
@ -1703,7 +1703,7 @@ func TestPosixRenameFile(t *testing.T) {
srcPath: "file4", srcPath: "file4",
destPath: "new-path/", destPath: "new-path/",
ioErrCnt: 0, ioErrCnt: 0,
expectedErr: errInvalidArgument, expectedErr: errVolumeNotFound,
}, },
// TestPosix case - 16. // TestPosix case - 16.
// TestPosix case with the parent of the destination being a file. // TestPosix case with the parent of the destination being a file.

@ -355,10 +355,11 @@ func testDeleteBucketWebHandler(obj ObjectLayer, instanceType string, t TestErrH
// Empty string = no error // Empty string = no error
expect string expect string
}{ }{
{"", false, token, "Bucket Name is invalid"}, {"", false, token, "The specified bucket does not exist."},
{".", false, "auth", "Authentication failed"}, {".", false, "auth", "Authentication failed"},
{".", false, token, "Bucket Name . is invalid"}, {".", false, token, "The specified bucket . does not exist."},
{"ab", false, token, "Bucket Name ab is invalid"}, {"..", 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, "false token", "Authentication failed"},
{"minio", false, token, "specified bucket minio does not exist"}, {"minio", false, token, "specified bucket minio does not exist"},
{bucketName, false, token, ""}, {bucketName, false, token, ""},

@ -163,11 +163,6 @@ func (xl xlObjects) GetBucketInfo(ctx context.Context, bucket string) (bi Bucket
return bi, e return bi, e
} }
defer bucketLock.RUnlock() defer bucketLock.RUnlock()
// Verify if bucket is valid.
if !IsValidBucketName(bucket) {
return bi, BucketNameInvalid{Bucket: bucket}
}
bucketInfo, err := xl.getBucketInfo(ctx, bucket) bucketInfo, err := xl.getBucketInfo(ctx, bucket)
if err != nil { if err != nil {
return bi, toObjectErr(err, bucket) return bi, toObjectErr(err, bucket)
@ -228,18 +223,12 @@ func (xl xlObjects) ListBuckets(ctx context.Context) ([]BucketInfo, error) {
// DeleteBucket - deletes a bucket. // DeleteBucket - deletes a bucket.
func (xl xlObjects) DeleteBucket(ctx context.Context, bucket string) error { func (xl xlObjects) DeleteBucket(ctx context.Context, bucket string) error {
bucketLock := xl.nsMutex.NewNSLock(bucket, "") bucketLock := xl.nsMutex.NewNSLock(bucket, "")
if err := bucketLock.GetLock(globalObjectTimeout); err != nil { if err := bucketLock.GetLock(globalObjectTimeout); err != nil {
return err return err
} }
defer bucketLock.Unlock() defer bucketLock.Unlock()
// Verify if bucket is valid.
if !IsValidBucketName(bucket) {
return BucketNameInvalid{Bucket: bucket}
}
// Collect if all disks report volume not found. // Collect if all disks report volume not found.
var wg = &sync.WaitGroup{} var wg = &sync.WaitGroup{}
var dErrs = make([]error, len(xl.getDisks())) var dErrs = make([]error, len(xl.getDisks()))

Loading…
Cancel
Save