From aaf7803831550ea048916debfa143477d8d0ab83 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sun, 17 Jul 2016 12:32:05 -0700 Subject: [PATCH] api: Requests should be differentiated if possible based on http router. (#2219) In current master ListObjectsV2 was merged into ListObjectsHandler which also implements V1 API as well. Move the detection of ListObject types to its rightful place in http router. --- api-response.go | 6 +- api-router.go | 6 +- bucket-handlers-listobjects.go | 167 +++++++++++++++++++++++++++++++++ bucket-handlers.go | 83 ---------------- server_test.go | 17 +++- test-utils_test.go | 14 ++- 6 files changed, 200 insertions(+), 93 deletions(-) create mode 100644 bucket-handlers-listobjects.go diff --git a/api-response.go b/api-response.go index da29f25ff..1d12a3f51 100644 --- a/api-response.go +++ b/api-response.go @@ -291,8 +291,8 @@ func generateListBucketsResponse(buckets []BucketInfo) ListBucketsResponse { return data } -// generates an ListObjects response for the said bucket with other enumerated options. -func generateListObjectsResponse(bucket, prefix, marker, delimiter string, maxKeys int, resp ListObjectsInfo) ListObjectsResponse { +// generates an ListObjectsV1 response for the said bucket with other enumerated options. +func generateListObjectsV1Response(bucket, prefix, marker, delimiter string, maxKeys int, resp ListObjectsInfo) ListObjectsResponse { var contents []Object var prefixes []CommonPrefix var owner = Owner{} @@ -336,7 +336,7 @@ func generateListObjectsResponse(bucket, prefix, marker, delimiter string, maxKe return data } -// generates an ListObjects response for the said bucket with other enumerated options. +// generates an ListObjectsV2 response for the said bucket with other enumerated options. func generateListObjectsV2Response(bucket, prefix, token, startAfter, delimiter string, maxKeys int, resp ListObjectsInfo) ListObjectsV2Response { var contents []Object var prefixes []CommonPrefix diff --git a/api-router.go b/api-router.go index 81a59567d..6ad0696d2 100644 --- a/api-router.go +++ b/api-router.go @@ -62,8 +62,10 @@ func registerAPIRouter(mux *router.Router, api objectAPIHandlers) { bucket.Methods("GET").HandlerFunc(api.GetBucketPolicyHandler).Queries("policy", "") // ListMultipartUploads bucket.Methods("GET").HandlerFunc(api.ListMultipartUploadsHandler).Queries("uploads", "") - // ListObjects - bucket.Methods("GET").HandlerFunc(api.ListObjectsHandler) + // ListObjectsV2 + bucket.Methods("GET").HandlerFunc(api.ListObjectsV2Handler).Queries("list-type", "2") + // ListObjectsV1 (Legacy) + bucket.Methods("GET").HandlerFunc(api.ListObjectsV1Handler) // PutBucketPolicy bucket.Methods("PUT").HandlerFunc(api.PutBucketPolicyHandler).Queries("policy", "") // PutBucket diff --git a/bucket-handlers-listobjects.go b/bucket-handlers-listobjects.go new file mode 100644 index 000000000..a6d1a1d43 --- /dev/null +++ b/bucket-handlers-listobjects.go @@ -0,0 +1,167 @@ +/* + * 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 main + +import ( + "net/http" + "strings" + + "github.com/gorilla/mux" +) + +// Validate all the ListObjects query arguments, returns an APIErrorCode +// if one of the args do not meet the required conditions. +// Special conditions required by Minio server are as below +// - delimiter if set should be equal to '/', otherwise the request is rejected. +// - marker if set should have a common prefix with 'prefix' param, otherwise +// the request is rejected. +func listObjectsValidateArgs(prefix, marker, delimiter string, maxKeys int) APIErrorCode { + // Max keys cannot be negative. + if maxKeys < 0 { + return ErrInvalidMaxKeys + } + + /// Minio special conditions for ListObjects. + + // Verify if delimiter is anything other than '/', which we do not support. + if delimiter != "" && delimiter != "/" { + return ErrNotImplemented + } + // Marker is set validate pre-condition. + if marker != "" { + // Marker not common with prefix is not implemented. + if !strings.HasPrefix(marker, prefix) { + return ErrNotImplemented + } + } + // Success. + return ErrNone +} + +// ListObjectsV2Handler - GET Bucket (List Objects) Version 2. +// -------------------------- +// This implementation of the GET operation returns some or all (up to 1000) +// of the objects in a bucket. You can use the request parameters as selection +// criteria to return a subset of the objects in a bucket. +// +// NOTE: It is recommended that this API to be used for application development. +// Minio continues to support ListObjectsV1 for supporting legacy tools. +func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + bucket := vars["bucket"] + + switch getRequestAuthType(r) { + default: + // For all unknown auth types return error. + writeErrorResponse(w, r, ErrAccessDenied, r.URL.Path) + return + case authTypeAnonymous: + // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html + if s3Error := enforceBucketPolicy("s3:ListBucket", bucket, r.URL); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + case authTypeSigned, authTypePresigned: + if s3Error := isReqAuthenticated(r); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + } + // Extract all the listObjectsV2 query params to their native values. + prefix, token, startAfter, delimiter, maxKeys, _ := getListObjectsV2Args(r.URL.Query()) + + // In ListObjectsV2 'continuation-token' is the marker. + marker := token + // Check if 'continuation-token' is empty. + if token == "" { + // Then we need to use 'start-after' as marker instead. + marker = startAfter + } + // Validate all the query params before beginning to serve the request. + if s3Error := listObjectsValidateArgs(prefix, marker, delimiter, maxKeys); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + // Inititate a list objects operation based on the input params. + // On success would return back ListObjectsInfo object to be + // marshalled into S3 compatible XML header. + listObjectsInfo, err := api.ObjectAPI.ListObjects(bucket, prefix, marker, delimiter, maxKeys) + if err != nil { + errorIf(err, "Unable to list objects.") + writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path) + return + } + + response := generateListObjectsV2Response(bucket, prefix, token, startAfter, delimiter, maxKeys, listObjectsInfo) + // Write headers + setCommonHeaders(w) + // Write success response. + writeSuccessResponse(w, encodeResponse(response)) +} + +// ListObjectsV1Handler - GET Bucket (List Objects) Version 1. +// -------------------------- +// This implementation of the GET operation returns some or all (up to 1000) +// of the objects in a bucket. You can use the request parameters as selection +// criteria to return a subset of the objects in a bucket. +// +func (api objectAPIHandlers) ListObjectsV1Handler(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + bucket := vars["bucket"] + + switch getRequestAuthType(r) { + default: + // For all unknown auth types return error. + writeErrorResponse(w, r, ErrAccessDenied, r.URL.Path) + return + case authTypeAnonymous: + // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html + if s3Error := enforceBucketPolicy("s3:ListBucket", bucket, r.URL); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + case authTypeSigned, authTypePresigned: + if s3Error := isReqAuthenticated(r); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + } + + // Extract all the litsObjectsV1 query params to their native values. + prefix, marker, delimiter, maxKeys, _ := getListObjectsV1Args(r.URL.Query()) + + // Validate all the query params before beginning to serve the request. + if s3Error := listObjectsValidateArgs(prefix, marker, delimiter, maxKeys); s3Error != ErrNone { + writeErrorResponse(w, r, s3Error, r.URL.Path) + return + } + + // Inititate a list objects operation based on the input params. + // On success would return back ListObjectsInfo object to be + // marshalled into S3 compatible XML header. + listObjectsInfo, err := api.ObjectAPI.ListObjects(bucket, prefix, marker, delimiter, maxKeys) + if err != nil { + errorIf(err, "Unable to list objects.") + writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path) + return + } + response := generateListObjectsV1Response(bucket, prefix, marker, delimiter, maxKeys, listObjectsInfo) + // Write headers + setCommonHeaders(w) + // Write success response. + writeSuccessResponse(w, encodeResponse(response)) +} diff --git a/bucket-handlers.go b/bucket-handlers.go index afe0b192f..99ab7fc18 100644 --- a/bucket-handlers.go +++ b/bucket-handlers.go @@ -193,89 +193,6 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter, writeSuccessResponse(w, encodedSuccessResponse) } -// ListObjectsHandler - GET Bucket (List Objects) -// -- ----------------------- -// This implementation of the GET operation returns some or all (up to 1000) -// of the objects in a bucket. You can use the request parameters as selection -// criteria to return a subset of the objects in a bucket. -// -func (api objectAPIHandlers) ListObjectsHandler(w http.ResponseWriter, r *http.Request) { - vars := mux.Vars(r) - bucket := vars["bucket"] - - switch getRequestAuthType(r) { - default: - // For all unknown auth types return error. - writeErrorResponse(w, r, ErrAccessDenied, r.URL.Path) - return - case authTypeAnonymous: - // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy("s3:ListBucket", bucket, r.URL); s3Error != ErrNone { - writeErrorResponse(w, r, s3Error, r.URL.Path) - return - } - case authTypeSigned, authTypePresigned: - if s3Error := isReqAuthenticated(r); s3Error != ErrNone { - writeErrorResponse(w, r, s3Error, r.URL.Path) - return - } - } - var prefix, marker, token, delimiter, startAfter string - var maxkeys int - var listV2 bool - // TODO handle encoding type. - if r.URL.Query().Get("list-type") == "2" { - listV2 = true - prefix, token, startAfter, delimiter, maxkeys, _ = getListObjectsV2Args(r.URL.Query()) - // For ListV2 "start-after" is considered only if "continuation-token" is empty. - if token == "" { - marker = startAfter - } else { - marker = token - } - } else { - prefix, marker, delimiter, maxkeys, _ = getListObjectsV1Args(r.URL.Query()) - } - if maxkeys < 0 { - writeErrorResponse(w, r, ErrInvalidMaxKeys, r.URL.Path) - return - } - // Verify if delimiter is anything other than '/', which we do not support. - if delimiter != "" && delimiter != "/" { - writeErrorResponse(w, r, ErrNotImplemented, r.URL.Path) - return - } - // If marker is set unescape. - if marker != "" { - // Marker not common with prefix is not implemented. - if !strings.HasPrefix(marker, prefix) { - writeErrorResponse(w, r, ErrNotImplemented, r.URL.Path) - return - } - } - - listObjectsInfo, err := api.ObjectAPI.ListObjects(bucket, prefix, marker, delimiter, maxkeys) - - if err == nil { - var encodedSuccessResponse []byte - // generate response - if listV2 { - response := generateListObjectsV2Response(bucket, prefix, token, startAfter, delimiter, maxkeys, listObjectsInfo) - encodedSuccessResponse = encodeResponse(response) - } else { - response := generateListObjectsResponse(bucket, prefix, marker, delimiter, maxkeys, listObjectsInfo) - encodedSuccessResponse = encodeResponse(response) - } - // Write headers - setCommonHeaders(w) - // Write success response. - writeSuccessResponse(w, encodedSuccessResponse) - return - } - errorIf(err, "Unable to list objects.") - writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path) -} - // ListBucketsHandler - GET Service // ----------- // This implementation of the GET operation returns a list of all buckets diff --git a/server_test.go b/server_test.go index fb1ab3e48..b0d3fe442 100644 --- a/server_test.go +++ b/server_test.go @@ -1153,9 +1153,8 @@ func (s *TestSuiteCommon) TestListObjectsHandlerErrors(c *C) { c.Assert(err, IsNil) c.Assert(response.StatusCode, Equals, http.StatusOK) - // create HTTP request with invalid value of max-keys parameter. - // max-keys is set to -2. - request, err = newTestSignedRequest("GET", getListObjectsURL(s.endPoint, bucketName, "-2"), + // create listObjectsV1 request with invalid value of max-keys parameter. max-keys is set to -2. + request, err = newTestSignedRequest("GET", getListObjectsV1URL(s.endPoint, bucketName, "-2"), 0, nil, s.accessKey, s.secretKey) c.Assert(err, IsNil) client = http.Client{} @@ -1164,6 +1163,18 @@ func (s *TestSuiteCommon) TestListObjectsHandlerErrors(c *C) { c.Assert(err, IsNil) // validating the error response. verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647", http.StatusBadRequest) + + // create listObjectsV2 request with invalid value of max-keys parameter. max-keys is set to -2. + request, err = newTestSignedRequest("GET", getListObjectsV2URL(s.endPoint, bucketName, "-2"), + 0, nil, s.accessKey, s.secretKey) + c.Assert(err, IsNil) + client = http.Client{} + // execute the HTTP request. + response, err = client.Do(request) + c.Assert(err, IsNil) + // validating the error response. + verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647", http.StatusBadRequest) + } // TestPutBucketErrors - request for non valid bucket operation diff --git a/test-utils_test.go b/test-utils_test.go index 52bd8ca0c..c6435ec46 100644 --- a/test-utils_test.go +++ b/test-utils_test.go @@ -561,8 +561,8 @@ func getDeleteBucketURL(endPoint, bucketName string) string { } -// return URL for listing the bucket. -func getListObjectsURL(endPoint, bucketName string, maxKeys string) string { +// return URL for listing objects in the bucket with V1 legacy API. +func getListObjectsV1URL(endPoint, bucketName string, maxKeys string) string { queryValue := url.Values{} if maxKeys != "" { queryValue.Set("max-keys", maxKeys) @@ -570,6 +570,16 @@ func getListObjectsURL(endPoint, bucketName string, maxKeys string) string { return makeTestTargetURL(endPoint, bucketName, "", queryValue) } +// return URL for listing objects in the bucket with V2 API. +func getListObjectsV2URL(endPoint, bucketName string, maxKeys string) string { + queryValue := url.Values{} + queryValue.Set("list-type", "2") // Enables list objects V2 URL. + if maxKeys != "" { + queryValue.Set("max-keys", maxKeys) + } + return makeTestTargetURL(endPoint, bucketName, "", queryValue) +} + // return URL for a new multipart upload. func getNewMultipartURL(endPoint, bucketName, objectName string) string { queryValue := url.Values{}