diff --git a/object-api-putobject_test.go b/object-api-putobject_test.go index 149941509..818bf6ce2 100644 --- a/object-api-putobject_test.go +++ b/object-api-putobject_test.go @@ -18,7 +18,12 @@ package main import ( "bytes" + "crypto/md5" + "encoding/hex" "fmt" + "io/ioutil" + "os" + "path" "testing" ) @@ -229,3 +234,114 @@ func testObjectAPIPutObjectDiskNotFOund(obj ObjectLayer, instanceType string, di } } } + +// Wrapper for calling PutObject tests for both XL multiple disks and single node setup. +func TestObjectAPIPutObjectStaleFiles(t *testing.T) { + ExecObjectLayerStaleFilesTest(t, testObjectAPIPutObjectStaleFiles) +} + +// Tests validate correctness of PutObject. +func testObjectAPIPutObjectStaleFiles(obj ObjectLayer, instanceType string, disks []string, t *testing.T) { + // Generating cases for which the PutObject fails. + bucket := "minio-bucket" + object := "minio-object" + + // Create bucket. + err := obj.MakeBucket(bucket) + if err != nil { + // Failed to create newbucket, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + data := []byte("hello, world") + // Create object. + _, err = obj.PutObject(bucket, object, int64(len(data)), bytes.NewReader(data), nil) + if err != nil { + // Failed to create object, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + for _, disk := range disks { + tmpMetaDir := path.Join(disk, minioMetaBucket, tmpMetaPrefix) + if !isDirEmpty(tmpMetaDir) { + t.Fatalf("%s: expected: empty, got: non-empty", tmpMetaDir) + } + } +} + +// Wrapper for calling Multipart PutObject tests for both XL multiple disks and single node setup. +func TestObjectAPIMultipartPutObjectStaleFiles(t *testing.T) { + ExecObjectLayerStaleFilesTest(t, testObjectAPIMultipartPutObjectStaleFiles) +} + +// Tests validate correctness of PutObject. +func testObjectAPIMultipartPutObjectStaleFiles(obj ObjectLayer, instanceType string, disks []string, t *testing.T) { + // Generating cases for which the PutObject fails. + bucket := "minio-bucket" + object := "minio-object" + + // Create bucket. + err := obj.MakeBucket(bucket) + if err != nil { + // Failed to create newbucket, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + // Initiate Multipart Upload on the above created bucket. + uploadID, err := obj.NewMultipartUpload(bucket, object, nil) + if err != nil { + // Failed to create NewMultipartUpload, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + // Upload part1. + fiveMBBytes := bytes.Repeat([]byte("a"), 5*1024*1024) + md5Writer := md5.New() + md5Writer.Write(fiveMBBytes) + etag1 := hex.EncodeToString(md5Writer.Sum(nil)) + _, err = obj.PutObjectPart(bucket, object, uploadID, 1, int64(len(fiveMBBytes)), bytes.NewReader(fiveMBBytes), etag1) + if err != nil { + // Failed to upload object part, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + // Upload part2. + data := []byte("hello, world") + md5Writer = md5.New() + md5Writer.Write(data) + etag2 := hex.EncodeToString(md5Writer.Sum(nil)) + _, err = obj.PutObjectPart(bucket, object, uploadID, 2, int64(len(data)), bytes.NewReader(data), etag2) + if err != nil { + // Failed to upload object part, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + // Complete multipart. + parts := []completePart{ + {ETag: etag1, PartNumber: 1}, + {ETag: etag2, PartNumber: 2}, + } + _, err = obj.CompleteMultipartUpload(bucket, object, uploadID, parts) + if err != nil { + // Failed to complete multipart upload, abort. + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + for _, disk := range disks { + tmpMetaDir := path.Join(disk, minioMetaBucket, tmpMetaPrefix) + files, err := ioutil.ReadDir(tmpMetaDir) + if err != nil { + // Its OK to have non-existen tmpMetaDir. + if os.IsNotExist(err) { + continue + } + + // Print the error + t.Errorf("%s", err) + } + + if len(files) != 0 { + t.Fatalf("%s: expected: empty, got: non-empty. content: %s", tmpMetaDir, files) + } + } +} diff --git a/posix-utils_nix.go b/posix-utils_nix.go index 7086ea2fd..c71ff8e73 100644 --- a/posix-utils_nix.go +++ b/posix-utils_nix.go @@ -18,19 +18,12 @@ package main -import ( - "os" - "strings" -) +import "os" // isValidVolname verifies a volname name in accordance with object // layer requirements. func isValidVolname(volname string) bool { - if len(volname) < 3 || len(volname) > 63 { - return false - } - // Volname shouldn't have '/' in it. - return !strings.ContainsAny(volname, "/") + return !(len(volname) < 3 || len(volname) > 63) } // mkdirAll creates a directory named path, diff --git a/posix-utils_test.go b/posix-utils_test.go index 87db13da8..33d0bc6fd 100644 --- a/posix-utils_test.go +++ b/posix-utils_test.go @@ -56,7 +56,7 @@ func TestIsValidVolname(t *testing.T) { {"/", false}, {"a", false}, {"ab", false}, - {"ab/", false}, + {"ab/", true}, {"......", true}, {"lalalallalallalalalallalallalala-theString-size-is-greater-than-64", false}, } diff --git a/posix-utils_windows.go b/posix-utils_windows.go index c938fd3a7..9d9656865 100644 --- a/posix-utils_windows.go +++ b/posix-utils_windows.go @@ -33,7 +33,7 @@ func isValidVolname(volname string) bool { return false } // Volname shouldn't have reserved characters on windows in it. - return !strings.ContainsAny(volname, "/\\:*?\"<>|") + return !strings.ContainsAny(volname, `\:*?\"<>|`) } // mkdirAll creates a directory named path, diff --git a/test-utils_test.go b/test-utils_test.go index b8eb21b02..698c344f6 100644 --- a/test-utils_test.go +++ b/test-utils_test.go @@ -592,3 +592,18 @@ func ExecObjectLayerDiskNotFoundTest(t *testing.T, objTest objTestDiskNotFoundTy objTest(objLayer, xLTestStr, fsDirs, t) defer removeRoots(fsDirs) } + +// Special object test type for stale files situations. +type objTestStaleFilesType func(obj ObjectLayer, instanceType string, dirs []string, t *testing.T) + +// ExecObjectLayerStaleFilesTest - executes object layer tests those leaves stale +// files/directories under .minio/tmp. Creates XL ObjectLayer instance and runs test for XL layer. +func ExecObjectLayerStaleFilesTest(t *testing.T, objTest objTestStaleFilesType) { + objLayer, fsDirs, err := getXLObjectLayer() + if err != nil { + t.Fatalf("Initialization of object layer failed for XL setup: %s", err.Error()) + } + // Executing the object layer tests for XL. + objTest(objLayer, xLTestStr, fsDirs, t) + defer removeRoots(fsDirs) +} diff --git a/xl-v1-object.go b/xl-v1-object.go index 7c74a4593..c2ea20105 100644 --- a/xl-v1-object.go +++ b/xl-v1-object.go @@ -305,7 +305,8 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io. uniqueID := getUUID() tempErasureObj := path.Join(tmpMetaPrefix, uniqueID, "part.1") - tempObj := path.Join(tmpMetaPrefix, uniqueID) + minioMetaTmpBucket := path.Join(minioMetaBucket, tmpMetaPrefix) + tempObj := uniqueID // Initialize xl meta. xlMeta := newXLMetaV1(xl.dataBlocks, xl.parityBlocks) @@ -372,7 +373,7 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io. if md5Hex != "" { if newMD5Hex != md5Hex { // MD5 mismatch, delete the temporary object. - xl.deleteObject(minioMetaBucket, tempObj) + xl.deleteObject(minioMetaTmpBucket, tempObj) // Returns md5 mismatch. return "", BadDigest{md5Hex, newMD5Hex} } @@ -387,7 +388,7 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io. // Rename if an object already exists to temporary location. newUniqueID := getUUID() if xl.isObject(bucket, object) { - err = xl.renameObject(bucket, object, minioMetaBucket, path.Join(tmpMetaPrefix, newUniqueID)) + err = xl.renameObject(bucket, object, minioMetaTmpBucket, newUniqueID) if err != nil { return "", toObjectErr(err, bucket, object) } @@ -408,18 +409,18 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io. } // Write unique `xl.json` for each disk. - if err = xl.writeUniqueXLMetadata(minioMetaBucket, tempObj, partsMetadata); err != nil { + if err = xl.writeUniqueXLMetadata(minioMetaTmpBucket, tempObj, partsMetadata); err != nil { return "", toObjectErr(err, bucket, object) } // Rename the successfully written temporary object to final location. - err = xl.renameObject(minioMetaBucket, tempObj, bucket, object) + err = xl.renameObject(minioMetaTmpBucket, tempObj, bucket, object) if err != nil { return "", toObjectErr(err, bucket, object) } // Delete the temporary object. - xl.deleteObject(minioMetaBucket, path.Join(tmpMetaPrefix, newUniqueID)) + xl.deleteObject(minioMetaTmpBucket, newUniqueID) // Return md5sum, successfully wrote object. return newMD5Hex, nil