list: Fix encoding in listing results (#1171)

Add unit tests for objects name which contains special
characters, such as, space, % and \x17..

api-list.go

@@ -248,15 +248,15 @@ func (c Client) listObjectsV2Query(bucketName, objectPrefix, continuationToken s
 		return listBucketResult, errors.New("Truncated response should have continuation token set")
 	}
 
-	for _, obj := range listBucketResult.Contents {
-		obj.Key, err = url.QueryUnescape(obj.Key)
+	for i, obj := range listBucketResult.Contents {
+		listBucketResult.Contents[i].Key, err = url.QueryUnescape(obj.Key)
 		if err != nil {
 			return listBucketResult, err
 		}
 	}
 
-	for _, obj := range listBucketResult.CommonPrefixes {
-		obj.Prefix, err = url.QueryUnescape(obj.Prefix)
+	for i, obj := range listBucketResult.CommonPrefixes {
+		listBucketResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix)
 		if err != nil {
 			return listBucketResult, err
 		}
@@ -433,15 +433,15 @@ func (c Client) listObjectsQuery(bucketName, objectPrefix, objectMarker, delimit
 		return listBucketResult, err
 	}
 
-	for _, obj := range listBucketResult.Contents {
-		obj.Key, err = url.QueryUnescape(obj.Key)
+	for i, obj := range listBucketResult.Contents {
+		listBucketResult.Contents[i].Key, err = url.QueryUnescape(obj.Key)
 		if err != nil {
 			return listBucketResult, err
 		}
 	}
 
-	for _, obj := range listBucketResult.CommonPrefixes {
-		obj.Prefix, err = url.QueryUnescape(obj.Prefix)
+	for i, obj := range listBucketResult.CommonPrefixes {
+		listBucketResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix)
 		if err != nil {
 			return listBucketResult, err
 		}
@@ -642,15 +642,15 @@ func (c Client) listMultipartUploadsQuery(bucketName, keyMarker, uploadIDMarker,
 		return listMultipartUploadsResult, err
 	}
 
-	for _, obj := range listMultipartUploadsResult.Uploads {
-		obj.Key, err = url.QueryUnescape(obj.Key)
+	for i, obj := range listMultipartUploadsResult.Uploads {
+		listMultipartUploadsResult.Uploads[i].Key, err = url.QueryUnescape(obj.Key)
 		if err != nil {
 			return listMultipartUploadsResult, err
 		}
 	}
 
-	for _, obj := range listMultipartUploadsResult.CommonPrefixes {
-		obj.Prefix, err = url.QueryUnescape(obj.Prefix)
+	for i, obj := range listMultipartUploadsResult.CommonPrefixes {
+		listMultipartUploadsResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix)
 		if err != nil {
 			return listMultipartUploadsResult, err
 		}

functional_tests.go

@@ -9969,7 +9969,7 @@ func testFGetObjectWithContextV2() {
 
 }
 
-// Test list object v1 and V2 storage class fields
+// Test list object v1 and V2
 func testListObjects() {
 	// initialize logging params
 	startTime := time.Now()
@@ -10012,75 +10012,71 @@ func testListObjects() {
 		return
 	}
 
-	bufSize := dataFileMap["datafile-33-kB"]
-	var reader = getDataReader("datafile-33-kB")
-	defer reader.Close()
-
-	// Save the data
-	objectName1 := randString(60, rand.NewSource(time.Now().UnixNano()), "")
+	testObjects := []struct {
+		name         string
+		storageClass string
+	}{
 
-	_, err = c.PutObject(bucketName, objectName1, reader, int64(bufSize), minio.PutObjectOptions{ContentType: "binary/octet-stream", StorageClass: "STANDARD"})
-	if err != nil {
-		logError(testName, function, args, startTime, "", "PutObject1 call failed", err)
-		return
+		// \x17 is a forbidden character in a xml document
+		{"foo\x17bar", "STANDARD"},
+		// Special characters
+		{"foo bar", "STANDARD"},
+		{"foo-%", "STANDARD"},
+		{"random-object-1", "STANDARD"},
+		{"random-object-2", "REDUCED_REDUNDANCY"},
+	}
+
+	for i, object := range testObjects {
+		bufSize := dataFileMap["datafile-33-kB"]
+		var reader = getDataReader("datafile-33-kB")
+		defer reader.Close()
+		_, err = c.PutObject(bucketName, object.name, reader, int64(bufSize),
+			minio.PutObjectOptions{ContentType: "binary/octet-stream", StorageClass: object.storageClass})
+		if err != nil {
+			logError(testName, function, args, startTime, "", fmt.Sprintf("PutObject %d call failed", i+1), err)
+			return
+		}
 	}
 
-	bufSize1 := dataFileMap["datafile-33-kB"]
-	var reader1 = getDataReader("datafile-33-kB")
-	defer reader1.Close()
-	objectName2 := randString(60, rand.NewSource(time.Now().UnixNano()), "")
+	testList := func(listFn func(string, string, bool, <-chan struct{}) <-chan minio.ObjectInfo, bucket string) {
+		// Create a done channel to control 'ListObjects' go routine.
+		doneCh := make(chan struct{})
+		// Exit cleanly upon return.
+		defer close(doneCh)
 
-	_, err = c.PutObject(bucketName, objectName2, reader1, int64(bufSize1), minio.PutObjectOptions{ContentType: "binary/octet-stream", StorageClass: "REDUCED_REDUNDANCY"})
-	if err != nil {
-		logError(testName, function, args, startTime, "", "PutObject2 call failed", err)
-		return
-	}
+		var objCursor int
 
-	// Create a done channel to control 'ListObjects' go routine.
-	doneCh := make(chan struct{})
-	// Exit cleanly upon return.
-	defer close(doneCh)
-
-	// check for storage-class from ListObjects result
-	for objInfo := range c.ListObjects(bucketName, "", true, doneCh) {
-		if objInfo.Err != nil {
-			logError(testName, function, args, startTime, "", "ListObjects failed unexpectedly", err)
-			return
-		}
-		if objInfo.Key == objectName1 && objInfo.StorageClass != "STANDARD" {
-			// Ignored as Gateways (Azure/GCS etc) wont return storage class
-			ignoredLog(testName, function, args, startTime, "ListObjects doesn't return expected storage class").Info()
-		}
-		if objInfo.Key == objectName2 && objInfo.StorageClass != "REDUCED_REDUNDANCY" {
-			// Ignored as Gateways (Azure/GCS etc) wont return storage class
-			ignoredLog(testName, function, args, startTime, "ListObjects doesn't return expected storage class").Info()
+		// check for object name and storage-class from listing object result
+		for objInfo := range listFn(bucket, "", true, doneCh) {
+			if objInfo.Err != nil {
+				logError(testName, function, args, startTime, "", "ListObjects failed unexpectedly", err)
+				return
+			}
+			if objInfo.Key != testObjects[objCursor].name {
+				logError(testName, function, args, startTime, "", "ListObjects does not return expected object name", err)
+			}
+			if objInfo.StorageClass != testObjects[objCursor].storageClass {
+				// Ignored as Gateways (Azure/GCS etc) wont return storage class
+				ignoredLog(testName, function, args, startTime, "ListObjects doesn't return expected storage class").Info()
+			}
+			objCursor++
 		}
-	}
 
-	// check for storage-class from ListObjectsV2 result
-	for objInfo := range c.ListObjectsV2(bucketName, "", true, doneCh) {
-		if objInfo.Err != nil {
-			logError(testName, function, args, startTime, "", "ListObjectsV2 failed unexpectedly", err)
-			return
-		}
-		if objInfo.Key == objectName1 && objInfo.StorageClass != "STANDARD" {
-			// Ignored as Gateways (Azure/GCS etc) wont return storage class
-			ignoredLog(testName, function, args, startTime, "ListObjectsV2 doesn't return expected storage class").Info()
-		}
-		if objInfo.Key == objectName2 && objInfo.StorageClass != "REDUCED_REDUNDANCY" {
-			// Ignored as Gateways (Azure/GCS etc) wont return storage class
-			ignoredLog(testName, function, args, startTime, "ListObjectsV2 doesn't return expected storage class").Info()
+		if objCursor != len(testObjects) {
+			logError(testName, function, args, startTime, "", "ListObjects returned unexpected number of items", errors.New(""))
 		}
 	}
 
+	testList(c.ListObjects, bucketName)
+	testList(c.ListObjectsV2, bucketName)
+
 	// Delete all objects and buckets
 	if err = cleanupBucket(bucketName, c); err != nil {
 		logError(testName, function, args, startTime, "", "Cleanup failed", err)
 		return
 	}
 
 	successLogger(testName, function, args, startTime).Info()
-
 }
 
 // Convert string to bool and always return false if any error

go.sum

@@ -1,3 +1,4 @@
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
@@ -10,6 +11,7 @@ github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKU
 github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
@@ -18,6 +20,7 @@ github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1
 github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a h1:pa8hGb/2YqsZKovtsgrwcDH1RZhVbTKCjLp47XpqCDs=
 github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f h1:R423Cnkcp5JABoeemiGEPlt9tHXFfw5kvc0yqlxRPWo=