From 6df7bc42b8c082b8e6033eff283a0f4aa66ed41c Mon Sep 17 00:00:00 2001
From: Aditya Manthramurthy <donatello@users.noreply.github.com>
Date: Fri, 3 Mar 2017 23:53:41 +0530
Subject: [PATCH] Fix check for bucket name: (#3832)

* Do not allow bucket names with adjacent hypen and periods.
* Improve performance by eliminating the usage of regular expressions.
---
 cmd/object-api-utils.go      | 55 +++++++++++++++++++++++++++---------
 cmd/object-api-utils_test.go | 11 ++++++--
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go
index 40047657d..ce78e9265 100644
--- a/cmd/object-api-utils.go
+++ b/cmd/object-api-utils.go
@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"io"
 	"path"
-	"regexp"
 	"runtime"
 	"strings"
 	"unicode/utf8"
@@ -38,12 +37,10 @@ const (
 	minioMetaMultipartBucket = minioMetaBucket + "/" + mpartMetaPrefix
 	// Minio Tmp meta prefix.
 	minioMetaTmpBucket = minioMetaBucket + "/tmp"
+	// DNS separator (period), used for bucket name validation.
+	dnsDelimiter = "."
 )
 
-// validBucket regexp.
-var validBucket = regexp.MustCompile(`^[a-z0-9][a-z0-9\.\-]{1,61}[a-z0-9]$`)
-var isIPAddress = regexp.MustCompile(`^(\d+\.){3}\d+$`)
-
 // isMinioBucket returns true if given bucket is a Minio internal
 // bucket and false otherwise.
 func isMinioMetaBucketName(bucket string) bool {
@@ -52,10 +49,13 @@ func isMinioMetaBucketName(bucket string) bool {
 		bucket == minioMetaTmpBucket
 }
 
-// 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
+// IsValidBucketName verifies that a bucket name is in accordance with
+// Amazon's requirements (i.e. DNS naming conventions). It must be 3-63
+// characters long, and it must be a sequence of one or more labels
+// separated by periods. Each label can contain lowercase ascii
+// letters, decimal digits and hyphens, but must not begin or end with
+// a hyphen. See:
+// http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
 func IsValidBucketName(bucket string) bool {
 	// Special case when bucket is equal to one of the meta buckets.
 	if isMinioMetaBucketName(bucket) {
@@ -64,12 +64,39 @@ func IsValidBucketName(bucket string) bool {
 	if len(bucket) < 3 || len(bucket) > 63 {
 		return false
 	}
-	if bucket[0] == '.' || bucket[len(bucket)-1] == '.' {
-		return false
+
+	// Split on dot and check each piece conforms to rules.
+	allNumbers := true
+	pieces := strings.Split(bucket, dnsDelimiter)
+	for _, piece := range pieces {
+		if len(piece) == 0 || piece[0] == '-' ||
+			piece[len(piece)-1] == '-' {
+			// Current piece has 0-length or starts or
+			// ends with a hyphen.
+			return false
+		}
+		// Now only need to check if each piece is a valid
+		// 'label' in AWS terminology and if the bucket looks
+		// like an IP address.
+		isNotNumber := false
+		for i := 0; i < len(piece); i++ {
+			switch {
+			case (piece[i] >= 'a' && piece[i] <= 'z' ||
+				piece[i] == '-'):
+				// Found a non-digit character, so
+				// this piece is not a number.
+				isNotNumber = true
+			case piece[i] >= '0' && piece[i] <= '9':
+				// Nothing to do.
+			default:
+				// Found invalid character.
+				return false
+			}
+		}
+		allNumbers = allNumbers && !isNotNumber
 	}
-	return (validBucket.MatchString(bucket) &&
-		!isIPAddress.MatchString(bucket) &&
-		!strings.Contains(bucket, ".."))
+	// Does the bucket name look like an IP address?
+	return !(len(pieces) == 4 && allNumbers)
 }
 
 // IsValidObjectName verifies an object name in accordance with Amazon's
diff --git a/cmd/object-api-utils_test.go b/cmd/object-api-utils_test.go
index b93ceabb8..abf3560da 100644
--- a/cmd/object-api-utils_test.go
+++ b/cmd/object-api-utils_test.go
@@ -41,6 +41,8 @@ func TestIsValidBucketName(t *testing.T) {
 		{"testbucket", true},
 		{"1bucket", true},
 		{"bucket1", true},
+		{"a.b", true},
+		{"ab.a.bc", true},
 		// cases for which test should fail.
 		// passing invalid bucket names.
 		{"------", false},
@@ -59,11 +61,14 @@ func TestIsValidBucketName(t *testing.T) {
 		{"ends-with-a-dot.", false},
 		{"ends-with-a-dash-", false},
 		{"-starts-with-a-dash", false},
-		{"THIS-BEINGS-WITH-UPPERCASe", false},
+		{"THIS-BEGINS-WITH-UPPERCASe", false},
 		{"tHIS-ENDS-WITH-UPPERCASE", false},
-		{"ThisBeginsAndEndsWithUpperCase", false},
+		{"ThisBeginsAndEndsWithUpperCasE", false},
 		{"una ñina", false},
-		{"lalalallalallalalalallalallalala-theString-size-is-greater-than-64", false},
+		{"dash-.may-not-appear-next-to-dot", false},
+		{"dash.-may-not-appear-next-to-dot", false},
+		{"dash-.-may-not-appear-next-to-dot", false},
+		{"lalalallalallalalalallalallalala-thestring-size-is-greater-than-63", false},
 	}
 
 	for i, testCase := range testCases {