From cd3eb63c4ae13d5a5601cb69cb1cb5cae606a43c Mon Sep 17 00:00:00 2001
From: Brendan Ashworth <brendan.ashworth@me.com>
Date: Mon, 7 Mar 2016 19:30:30 -0800
Subject: [PATCH] pkg/fs: test, document, and fix IsValid{Bucket,Object}Name

This commit improves the docs for both functions (more Go-like) and
drops an unnecessary condition in IsValidBucketName. This also drops a
condition in IsValidObjectName where "" (empty string) was a valid
object name. This has been fixed and will no longer return true.

This commit also adds tests for both functions, including a regression
test for the bug fix.
---
 pkg/fs/fs-utils.go      | 18 +++++------
 pkg/fs/fs-utils_test.go | 72 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 10 deletions(-)
 create mode 100644 pkg/fs/fs-utils_test.go

diff --git a/pkg/fs/fs-utils.go b/pkg/fs/fs-utils.go
index 309431592..6490e1551 100644
--- a/pkg/fs/fs-utils.go
+++ b/pkg/fs/fs-utils.go
@@ -24,12 +24,11 @@ import (
 // validBucket regexp.
 var validBucket = regexp.MustCompile(`^[a-z0-9][a-z0-9\.\-]{1,61}[a-z0-9]$`)
 
-// IsValidBucketName - verify bucket name in accordance with
-//  - http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingBucket.html
+// IsValidBucketName verifies a bucket name in accordance with Amazon's
+// requirements. It must be 3-63 characters long, can contain dashes
+// and periods, but must begin and end with a lowercase letter or a number.
+// See: http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
 func IsValidBucketName(bucket string) bool {
-	if bucket == "" {
-		return false
-	}
 	if len(bucket) < 3 || len(bucket) > 63 {
 		return false
 	}
@@ -39,12 +38,11 @@ func IsValidBucketName(bucket string) bool {
 	return validBucket.MatchString(bucket)
 }
 
-// IsValidObjectName - verify object name in accordance with
-//   - http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html
+// IsValidObjectName verifies an object name in accordance with Amazon's
+// requirements. It cannot exceed 1024 characters and must be a valid UTF8
+// string.
+// See: http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html
 func IsValidObjectName(object string) bool {
-	if object == "" {
-		return true
-	}
 	if len(object) > 1024 || len(object) == 0 {
 		return false
 	}
diff --git a/pkg/fs/fs-utils_test.go b/pkg/fs/fs-utils_test.go
new file mode 100644
index 000000000..c1eb1d61c
--- /dev/null
+++ b/pkg/fs/fs-utils_test.go
@@ -0,0 +1,72 @@
+/*
+ * Minio Cloud Storage, (C) 2016 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package fs
+
+import (
+	"testing"
+)
+
+func ensureBucketName(name string, t *testing.T, pass bool) {
+	if pass && IsValidBucketName(name) {
+		return
+	}
+	if !pass && !IsValidBucketName(name) {
+		return
+	}
+
+	t.Errorf("\"%s\" should have passed [%t]\n", name, pass)
+}
+
+func TestIsValidBucketName(t *testing.T) {
+	ensureBucketName("lol", t, true)
+	ensureBucketName("s3-eu-west-1.amazonaws.com", t, true)
+	ensureBucketName("ideas-are-more-powerful-than-guns", t, true)
+	ensureBucketName("testbucket", t, true)
+	ensureBucketName("1bucket", t, true)
+	ensureBucketName("bucket1", t, true)
+
+	ensureBucketName("testing.", t, false)
+	ensureBucketName("", t, false)
+	ensureBucketName("......", t, false)
+	ensureBucketName("THIS-IS-UPPERCASE", t, false)
+	ensureBucketName("una ñina", t, false)
+	ensureBucketName("lalalallalallalalalallalallalallalallalallalallalallalallallalala", t, false)
+}
+
+func ensureObjectName(name string, t *testing.T, pass bool) {
+	if pass && IsValidObjectName(name) {
+		return
+	}
+	if !pass && !IsValidObjectName(name) {
+		return
+	}
+
+	t.Errorf("\"%s\" should have passed [%t]\n", name, pass)
+}
+
+func TestIsValidObjectName(t *testing.T) {
+	ensureObjectName("object", t, true)
+	ensureObjectName("The Shining Script <v1>.pdf", t, true)
+	ensureObjectName("Cost Benefit Analysis (2009-2010).pptx", t, true)
+	ensureObjectName("117Gn8rfHL2ACARPAhaFd0AGzic9pUbIA/5OCn5A", t, true)
+	ensureObjectName("SHØRT", t, true)
+	ensureObjectName("There are far too many object names, and far too few bucket names!", t, true)
+
+	ensureObjectName("", t, false)
+	// Bad UTF8 strings should not pass.
+	ensureObjectName(string([]byte{0xff, 0xfe, 0xfd}), t, false)
+}