From 0563a9235aad4c7b07fc25d77dcfbc9fc3edf26c Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Tue, 10 Jan 2017 11:01:23 -0800
Subject: [PATCH] handlers: Handle crash if r.URL.Path is empty. (#3554)

URL paths can be empty and not have preceding separator,
we do not yet know the conditions this can happen inside
Go http server.

This patch is to ensure that we do not crash ourselves
under conditions where r.URL.Path may be empty.

Fixes #3553
---
 cmd/generic-handlers.go | 13 +-----
 cmd/utils.go            | 32 +++++++++++++++
 cmd/utils_test.go       | 88 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go
index 2ce0a878f..e38b65339 100644
--- a/cmd/generic-handlers.go
+++ b/cmd/generic-handlers.go
@@ -311,18 +311,7 @@ var notimplementedObjectResourceNames = map[string]bool{
 
 // Resource handler ServeHTTP() wrapper
 func (h resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// Skip the first element which is usually '/' and split the rest.
-	splits := strings.SplitN(r.URL.Path[1:], "/", 2)
-
-	// Save bucketName and objectName extracted from url Path.
-	var bucketName, objectName string
-	if len(splits) == 1 {
-		bucketName = splits[0]
-	}
-	if len(splits) == 2 {
-		bucketName = splits[0]
-		objectName = splits[1]
-	}
+	bucketName, objectName := urlPath2BucketObjectName(r.URL)
 
 	// If bucketName is present and not objectName check for bucket level resource queries.
 	if bucketName != "" && objectName == "" {
diff --git a/cmd/utils.go b/cmd/utils.go
index 359715fd6..8723e62ad 100644
--- a/cmd/utils.go
+++ b/cmd/utils.go
@@ -70,6 +70,38 @@ func checkDuplicateStrings(list []string) error {
 	return nil
 }
 
+// splitStr splits a string into n parts, empty strings are added
+// if we are not able to reach n elements
+func splitStr(path, sep string, n int) []string {
+	splits := strings.SplitN(path, sep, n)
+	// Add empty strings if we found elements less than nr
+	for i := n - len(splits); i > 0; i-- {
+		splits = append(splits, "")
+	}
+	return splits
+}
+
+// Convert url path into bucket and object name.
+func urlPath2BucketObjectName(u *url.URL) (bucketName, objectName string) {
+	if u == nil {
+		// Empty url, return bucket and object names.
+		return
+	}
+
+	// Trim any preceding slash separator.
+	urlPath := strings.TrimPrefix(u.Path, slashSeparator)
+
+	// Split urlpath using slash separator into a given number of
+	// expected tokens.
+	tokens := splitStr(urlPath, slashSeparator, 2)
+
+	// Extract bucket and objects.
+	bucketName, objectName = tokens[0], tokens[1]
+
+	// Success.
+	return bucketName, objectName
+}
+
 // checkDuplicates - function to validate if there are duplicates in a slice of endPoints.
 func checkDuplicateEndpoints(endpoints []*url.URL) error {
 	var strs []string
diff --git a/cmd/utils_test.go b/cmd/utils_test.go
index 89d0aae67..a112b1d55 100644
--- a/cmd/utils_test.go
+++ b/cmd/utils_test.go
@@ -220,6 +220,94 @@ func TestMaxPartID(t *testing.T) {
 	}
 }
 
+// Tests extracting bucket and objectname from various types of URL paths.
+func TestURL2BucketObjectName(t *testing.T) {
+	testCases := []struct {
+		u              *url.URL
+		bucket, object string
+	}{
+		// Test case 1 normal case.
+		{
+			u: &url.URL{
+				Path: "/bucket/object",
+			},
+			bucket: "bucket",
+			object: "object",
+		},
+		// Test case 2 where url only has separator.
+		{
+			u: &url.URL{
+				Path: "/",
+			},
+			bucket: "",
+			object: "",
+		},
+		// Test case 3 only bucket is present.
+		{
+			u: &url.URL{
+				Path: "/bucket",
+			},
+			bucket: "bucket",
+			object: "",
+		},
+		// Test case 4 many separators and object is a directory.
+		{
+			u: &url.URL{
+				Path: "/bucket/object/1/",
+			},
+			bucket: "bucket",
+			object: "object/1/",
+		},
+		// Test case 5 object has many trailing separators.
+		{
+			u: &url.URL{
+				Path: "/bucket/object/1///",
+			},
+			bucket: "bucket",
+			object: "object/1///",
+		},
+		// Test case 6 object has only trailing separators.
+		{
+			u: &url.URL{
+				Path: "/bucket/object///////",
+			},
+			bucket: "bucket",
+			object: "object///////",
+		},
+		// Test case 7 object has preceding separators.
+		{
+			u: &url.URL{
+				Path: "/bucket////object////",
+			},
+			bucket: "bucket",
+			object: "///object////",
+		},
+		// Test case 8 url is not allocated.
+		{
+			u:      nil,
+			bucket: "",
+			object: "",
+		},
+		// Test case 9 url path is empty.
+		{
+			u:      &url.URL{},
+			bucket: "",
+			object: "",
+		},
+	}
+
+	// Validate all test cases.
+	for i, testCase := range testCases {
+		bucketName, objectName := urlPath2BucketObjectName(testCase.u)
+		if bucketName != testCase.bucket {
+			t.Errorf("Test %d: failed expected bucket name \"%s\", got \"%s\"", i+1, testCase.bucket, bucketName)
+		}
+		if objectName != testCase.object {
+			t.Errorf("Test %d: failed expected bucket name \"%s\", got \"%s\"", i+1, testCase.object, objectName)
+		}
+	}
+}
+
 // Add tests for starting and stopping different profilers.
 func TestStartProfiler(t *testing.T) {
 	if startProfiler("") != nil {