From 95c16f51cbbc7993d490b2daf3047b6cabbf052e Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Wed, 17 Aug 2016 16:34:03 -0700
Subject: [PATCH] notification: Handle ARN with empty account and name types.
 (#2482)

Fixes #2481
---
 bucket-notification-utils.go      | 32 +++++++++------
 bucket-notification-utils_test.go | 66 ++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/bucket-notification-utils.go b/bucket-notification-utils.go
index e1301933d..8b3cdb9bb 100644
--- a/bucket-notification-utils.go
+++ b/bucket-notification-utils.go
@@ -100,26 +100,36 @@ func checkFilterRules(filterRules []filterRule) APIErrorCode {
 	return ErrNone
 }
 
-// checkQueueARN - check if the queue arn is valid.
-func checkQueueARN(queueARN string) APIErrorCode {
-	if !strings.HasPrefix(queueARN, minioSqs) {
+// Checks validity of input ARN for a given arnType.
+func checkARN(arn, arnType string) APIErrorCode {
+	if !strings.HasPrefix(arn, arnType) {
 		return ErrARNNotification
 	}
-	if !strings.HasPrefix(queueARN, minioSqs+serverConfig.GetRegion()+":") {
+	if !strings.HasPrefix(arn, arnType+serverConfig.GetRegion()+":") {
 		return ErrRegionNotification
 	}
+	account := strings.SplitN(strings.TrimPrefix(arn, arnType+serverConfig.GetRegion()+":"), ":", 2)
+	switch len(account) {
+	case 1:
+		// This means ARN is malformed, account should have min of 2elements.
+		return ErrARNNotification
+	case 2:
+		// Account topic id or topic name cannot be empty.
+		if account[0] == "" || account[1] == "" {
+			return ErrARNNotification
+		}
+	}
 	return ErrNone
 }
 
+// checkQueueARN - check if the queue arn is valid.
+func checkQueueARN(queueARN string) APIErrorCode {
+	return checkARN(queueARN, minioSqs)
+}
+
 // checkTopicARN - check if the topic arn is valid.
 func checkTopicARN(topicARN string) APIErrorCode {
-	if !strings.HasPrefix(topicARN, minioTopic) {
-		return ErrARNNotification
-	}
-	if !strings.HasPrefix(topicARN, minioTopic+serverConfig.GetRegion()+":") {
-		return ErrRegionNotification
-	}
-	return ErrNone
+	return checkARN(topicARN, minioTopic)
 }
 
 // Returns true if the topicARN is for an Minio sns listen type.
diff --git a/bucket-notification-utils_test.go b/bucket-notification-utils_test.go
index 43e1a49ad..2c8d79092 100644
--- a/bucket-notification-utils_test.go
+++ b/bucket-notification-utils_test.go
@@ -119,18 +119,49 @@ func TestTopicARN(t *testing.T) {
 			topicARN: "arn:minio:sns:us-east-1:10:minio",
 			errCode:  ErrNone,
 		},
-		// Invalid empty queue arn.
+		// Invalid empty topic  arn.
 		{
 			topicARN: "",
 			errCode:  ErrARNNotification,
 		},
+		// Invalid notification service type.
+		{
+			topicARN: "arn:minio:sqs:us-east-1:1:listen",
+			errCode:  ErrARNNotification,
+		},
 		// Invalid region 'us-west-1' in queue arn.
 		{
-			topicARN: "arn:minio:sns:us-west-1:1:redis",
+			topicARN: "arn:minio:sns:us-west-1:1:listen",
 			errCode:  ErrRegionNotification,
 		},
+		// Empty topic account id is invalid.
+		{
+			topicARN: "arn:minio:sns:us-east-1::listen",
+			errCode:  ErrARNNotification,
+		},
+		// Empty topic account name is invalid.
+		{
+			topicARN: "arn:minio:sns:us-east-1:10:",
+			errCode:  ErrARNNotification,
+		},
+		// Empty topic account id and account name is invalid.
+		{
+			topicARN: "arn:minio:sns:us-east-1::",
+			errCode:  ErrARNNotification,
+		},
+		// Missing topic id and separator missing at the end in topic arn.
+		{
+			topicARN: "arn:minio:sns:us-east-1:listen",
+			errCode:  ErrARNNotification,
+		},
+		// Missing topic id and empty string at the end in topic arn.
+		{
+			topicARN: "arn:minio:sns:us-east-1:",
+			errCode:  ErrARNNotification,
+		},
 	}
 
+	// Validate all topics.
 	for i, testCase := range testCases {
 		errCode := checkTopicARN(testCase.topicARN)
 		if testCase.errCode != errCode {
@@ -171,13 +202,44 @@ func TestQueueARN(t *testing.T) {
 			queueARN: "",
 			errCode:  ErrARNNotification,
 		},
+		// Invalid notification service type.
+		{
+			queueARN: "arn:minio:sns:us-east-1:1:listen",
+			errCode:  ErrARNNotification,
+		},
 		// Invalid region 'us-west-1' in queue arn.
 		{
 			queueARN: "arn:minio:sqs:us-west-1:1:redis",
 			errCode:  ErrRegionNotification,
 		},
+		// Invalid queue name empty in queue arn.
+		{
+			queueARN: "arn:minio:sqs:us-east-1:1:",
+			errCode:  ErrARNNotification,
+		},
+		// Invalid queue id empty in queue arn.
+		{
+			queueARN: "arn:minio:sqs:us-east-1::redis",
+			errCode:  ErrARNNotification,
+		},
+		// Invalid queue id and queue name empty in queue arn.
+		{
+			queueARN: "arn:minio:sqs:us-east-1::",
+			errCode:  ErrARNNotification,
+		},
+		// Missing queue id and separator missing at the end in queue arn.
+		{
+			queueARN: "arn:minio:sqs:us-east-1:amqp",
+			errCode:  ErrARNNotification,
+		},
+		// Missing queue id and empty string at the end in queue arn.
+		{
+			queueARN: "arn:minio:sqs:us-east-1:",
+			errCode:  ErrARNNotification,
+		},
 	}
 
+	// Validate all tests for queue arn.
 	for i, testCase := range testCases {
 		errCode := checkQueueARN(testCase.queueARN)
 		if testCase.errCode != errCode {