From 73982c8cb64c6070633a3578d8b6d37101248797 Mon Sep 17 00:00:00 2001 From: Mateusz Gajewski Date: Wed, 12 Oct 2016 20:02:15 +0200 Subject: [PATCH] Listen bucket notification for multiple prefixes/suffixes (#2911) * Listen bucket notification for multiple prefixes/suffixes * After review fixes --- cmd/api-resources.go | 16 +++++++++-- cmd/api-resources_test.go | 36 ++++++++++++++++++++++++ cmd/bucket-notification-handlers.go | 18 ++++++++---- cmd/bucket-notification-handlers_test.go | 16 +++++------ cmd/test-utils_test.go | 7 +++-- 5 files changed, 74 insertions(+), 19 deletions(-) diff --git a/cmd/api-resources.go b/cmd/api-resources.go index 9e10ab2c0..aa5dfa183 100644 --- a/cmd/api-resources.go +++ b/cmd/api-resources.go @@ -80,9 +80,19 @@ func getObjectResources(values url.Values) (uploadID string, partNumberMarker, m } // Parse listen bucket notification resources. -func getListenBucketNotificationResources(values url.Values) (prefix string, suffix string, events []string) { - prefix = values.Get("prefix") - suffix = values.Get("suffix") +func getListenBucketNotificationResources(values url.Values) (prefix []string, suffix []string, events []string) { + prefix = values["prefix"] + suffix = values["suffix"] events = values["events"] return prefix, suffix, events } + +// Validates filter values +func validateFilterValues(values []string) (err APIErrorCode) { + for _, value := range values { + if !IsValidObjectPrefix(value) { + return ErrFilterValueInvalid + } + } + return ErrNone +} diff --git a/cmd/api-resources_test.go b/cmd/api-resources_test.go index b76fb18ca..4a43324d6 100644 --- a/cmd/api-resources_test.go +++ b/cmd/api-resources_test.go @@ -18,6 +18,7 @@ package cmd import ( "net/url" + "strings" "testing" ) @@ -188,3 +189,38 @@ func TestGetObjectsResources(t *testing.T) { } } } + +// Validates if filter values are correct +func TestValidateFilterValues(t *testing.T) { + testCases := []struct { + values []string + expectedError APIErrorCode + }{ + { + values: []string{""}, + expectedError: ErrNone, + }, + { + values: []string{"", "prefix"}, + expectedError: ErrNone, + }, + { + values: []string{strings.Repeat("a", 1025)}, + expectedError: ErrFilterValueInvalid, + }, + { + values: []string{"a\\b"}, + expectedError: ErrFilterValueInvalid, + }, + { + values: []string{string([]byte{0xff, 0xfe, 0xfd})}, + expectedError: ErrFilterValueInvalid, + }, + } + + for i, testCase := range testCases { + if actualError := validateFilterValues(testCase.values); actualError != testCase.expectedError { + t.Errorf("Test %d: Expected %d, got %d", i+1, testCase.expectedError, actualError) + } + } +} diff --git a/cmd/bucket-notification-handlers.go b/cmd/bucket-notification-handlers.go index af5a7f77a..402e5cd8c 100644 --- a/cmd/bucket-notification-handlers.go +++ b/cmd/bucket-notification-handlers.go @@ -250,9 +250,15 @@ func (api objectAPIHandlers) ListenBucketNotificationHandler(w http.ResponseWrit bucket := vars["bucket"] // Parse listen bucket notification resources. - prefix, suffix, events := getListenBucketNotificationResources(r.URL.Query()) - if !IsValidObjectPrefix(prefix) || !IsValidObjectPrefix(suffix) { - writeErrorResponse(w, r, ErrFilterValueInvalid, r.URL.Path) + prefixes, suffixes, events := getListenBucketNotificationResources(r.URL.Query()) + + if err := validateFilterValues(prefixes); err != ErrNone { + writeErrorResponse(w, r, err, r.URL.Path) + return + } + + if err := validateFilterValues(suffixes); err != ErrNone { + writeErrorResponse(w, r, err, r.URL.Path) return } @@ -279,13 +285,15 @@ func (api objectAPIHandlers) ListenBucketNotificationHandler(w http.ResponseWrit globalMinioAddr, ) var filterRules []filterRule - if prefix != "" { + + for _, prefix := range prefixes { filterRules = append(filterRules, filterRule{ Name: "prefix", Value: prefix, }) } - if suffix != "" { + + for _, suffix := range suffixes { filterRules = append(filterRules, filterRule{ Name: "suffix", Value: suffix, diff --git a/cmd/bucket-notification-handlers_test.go b/cmd/bucket-notification-handlers_test.go index 39257304e..cb1abb13f 100644 --- a/cmd/bucket-notification-handlers_test.go +++ b/cmd/bucket-notification-handlers_test.go @@ -235,24 +235,24 @@ func testListenBucketNotificationHandler(obj ObjectLayer, instanceType string, t invalidEvents := []string{"invalidEvent"} testCases := []struct { bucketName string - prefix string - suffix string + prefixes []string + suffixes []string events []string kind testKind expectedHTTPCode int expectedAPIError string }{ // FIXME: Need to find a way to run valid listen bucket notification test case without blocking the unit test. - {randBucket, "", "", invalidEvents, CheckStatus, signatureMismatchError.HTTPStatusCode, ""}, - {randBucket, tooBigPrefix, "", validEvents, CheckStatus, http.StatusBadRequest, ""}, - {invalidBucket, "", "", validEvents, CheckStatus, http.StatusBadRequest, ""}, - {randBucket, "", "", validEvents, InvalidAuth, signatureMismatchError.HTTPStatusCode, signatureMismatchError.Code}, + {randBucket, []string{}, []string{}, invalidEvents, CheckStatus, signatureMismatchError.HTTPStatusCode, ""}, + {randBucket, []string{tooBigPrefix}, []string{}, validEvents, CheckStatus, http.StatusBadRequest, ""}, + {invalidBucket, []string{}, []string{}, validEvents, CheckStatus, http.StatusBadRequest, ""}, + {randBucket, []string{}, []string{}, validEvents, InvalidAuth, signatureMismatchError.HTTPStatusCode, signatureMismatchError.Code}, } for i, test := range testCases { testRec = httptest.NewRecorder() testReq, tErr = newTestSignedRequestV4("GET", - getListenBucketNotificationURL("", test.bucketName, test.prefix, test.suffix, test.events), + getListenBucketNotificationURL("", test.bucketName, test.prefixes, test.suffixes, test.events), 0, nil, credentials.AccessKeyID, credentials.SecretAccessKey) if tErr != nil { t.Fatalf("%s: Failed to create HTTP testRequest for ListenBucketNotification: %v", instanceType, tErr) @@ -301,7 +301,7 @@ func testListenBucketNotificationHandler(obj ObjectLayer, instanceType string, t }) testRec = httptest.NewRecorder() testReq, tErr = newTestSignedRequestV4("GET", - getListenBucketNotificationURL("", randBucket, "", "*.jpg", []string{"s3:ObjectCreated:*", "s3:ObjectRemoved:*"}), + getListenBucketNotificationURL("", randBucket, []string{}, []string{"*.jpg"}, []string{"s3:ObjectCreated:*", "s3:ObjectRemoved:*"}), 0, nil, credentials.AccessKeyID, credentials.SecretAccessKey) if tErr != nil { t.Fatalf("%s: Failed to create HTTP testRequest for ListenBucketNotification: %v", instanceType, tErr) diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index f6d729ce9..e7438252f 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1402,10 +1402,11 @@ func getGetBucketNotificationURL(endPoint, bucketName string) string { } // return URL for listen bucket notification. -func getListenBucketNotificationURL(endPoint, bucketName, prefix, suffix string, events []string) string { +func getListenBucketNotificationURL(endPoint, bucketName string, prefixes, suffixes, events []string) string { queryValue := url.Values{} - queryValue.Set("prefix", prefix) - queryValue.Set("suffix", suffix) + + queryValue["prefix"] = prefixes + queryValue["suffix"] = suffixes queryValue["events"] = events return makeTestTargetURL(endPoint, bucketName, "", queryValue) }