From b43e8337b110347ecdad6b3cc557894a7af68079 Mon Sep 17 00:00:00 2001 From: Pontus Leitzler Date: Thu, 18 Oct 2018 16:31:46 +0200 Subject: [PATCH] Add error handling in api-resource.go (#6651) --- cmd/api-resources.go | 72 +++++++++++++++++++++--------- cmd/api-resources_test.go | 10 ++++- cmd/bucket-handlers-listobjects.go | 6 ++- cmd/bucket-handlers.go | 6 ++- cmd/object-handlers.go | 18 ++++++-- 5 files changed, 85 insertions(+), 27 deletions(-) diff --git a/cmd/api-resources.go b/cmd/api-resources.go index b5f2757c1..b5d01cfe2 100644 --- a/cmd/api-resources.go +++ b/cmd/api-resources.go @@ -22,15 +22,22 @@ import ( ) // Parse bucket url queries -func getListObjectsV1Args(values url.Values) (prefix, marker, delimiter string, maxkeys int, encodingType string) { - prefix = values.Get("prefix") - marker = values.Get("marker") - delimiter = values.Get("delimiter") +func getListObjectsV1Args(values url.Values) (prefix, marker, delimiter string, maxkeys int, encodingType string, errCode APIErrorCode) { + errCode = ErrNone + if values.Get("max-keys") != "" { - maxkeys, _ = strconv.Atoi(values.Get("max-keys")) + var err error + if maxkeys, err = strconv.Atoi(values.Get("max-keys")); err != nil { + errCode = ErrInvalidMaxKeys + return + } } else { maxkeys = maxObjectList } + + prefix = values.Get("prefix") + marker = values.Get("marker") + delimiter = values.Get("delimiter") encodingType = values.Get("encoding-type") return } @@ -47,44 +54,69 @@ func getListObjectsV2Args(values url.Values) (prefix, token, startAfter, delimit } } - prefix = values.Get("prefix") - token = values.Get("continuation-token") - startAfter = values.Get("start-after") - delimiter = values.Get("delimiter") if values.Get("max-keys") != "" { - maxkeys, _ = strconv.Atoi(values.Get("max-keys")) + var err error + if maxkeys, err = strconv.Atoi(values.Get("max-keys")); err != nil { + errCode = ErrInvalidMaxKeys + return + } } else { maxkeys = maxObjectList } + + prefix = values.Get("prefix") + token = values.Get("continuation-token") + startAfter = values.Get("start-after") + delimiter = values.Get("delimiter") fetchOwner = values.Get("fetch-owner") == "true" encodingType = values.Get("encoding-type") return } // Parse bucket url queries for ?uploads -func getBucketMultipartResources(values url.Values) (prefix, keyMarker, uploadIDMarker, delimiter string, maxUploads int, encodingType string) { - prefix = values.Get("prefix") - keyMarker = values.Get("key-marker") - uploadIDMarker = values.Get("upload-id-marker") - delimiter = values.Get("delimiter") +func getBucketMultipartResources(values url.Values) (prefix, keyMarker, uploadIDMarker, delimiter string, maxUploads int, encodingType string, errCode APIErrorCode) { + errCode = ErrNone + if values.Get("max-uploads") != "" { - maxUploads, _ = strconv.Atoi(values.Get("max-uploads")) + var err error + if maxUploads, err = strconv.Atoi(values.Get("max-uploads")); err != nil { + errCode = ErrInvalidMaxUploads + return + } } else { maxUploads = maxUploadsList } + + prefix = values.Get("prefix") + keyMarker = values.Get("key-marker") + uploadIDMarker = values.Get("upload-id-marker") + delimiter = values.Get("delimiter") encodingType = values.Get("encoding-type") return } // Parse object url queries -func getObjectResources(values url.Values) (uploadID string, partNumberMarker, maxParts int, encodingType string) { - uploadID = values.Get("uploadId") - partNumberMarker, _ = strconv.Atoi(values.Get("part-number-marker")) +func getObjectResources(values url.Values) (uploadID string, partNumberMarker, maxParts int, encodingType string, errCode APIErrorCode) { + var err error + errCode = ErrNone + if values.Get("max-parts") != "" { - maxParts, _ = strconv.Atoi(values.Get("max-parts")) + if maxParts, err = strconv.Atoi(values.Get("max-parts")); err != nil { + errCode = ErrInvalidMaxParts + return + } } else { maxParts = maxPartsList } + + if values.Get("part-number-marker") != "" { + if partNumberMarker, err = strconv.Atoi(values.Get("part-number-marker")); err != nil { + errCode = ErrInvalidPartNumberMarker + return + } + } + + uploadID = values.Get("uploadId") encodingType = values.Get("encoding-type") return } diff --git a/cmd/api-resources_test.go b/cmd/api-resources_test.go index 7dc91bcd5..3a34d1d82 100644 --- a/cmd/api-resources_test.go +++ b/cmd/api-resources_test.go @@ -156,7 +156,10 @@ func TestListObjectsV1Resources(t *testing.T) { } for i, testCase := range testCases { - prefix, marker, delimiter, maxKeys, encodingType := getListObjectsV1Args(testCase.values) + prefix, marker, delimiter, maxKeys, encodingType, argsErr := getListObjectsV1Args(testCase.values) + if argsErr != ErrNone { + t.Errorf("Test %d: argument parsing failed, got %v", i+1, argsErr) + } if prefix != testCase.prefix { t.Errorf("Test %d: Expected %s, got %s", i+1, testCase.prefix, prefix) } @@ -198,7 +201,10 @@ func TestGetObjectsResources(t *testing.T) { } for i, testCase := range testCases { - uploadID, partNumberMarker, maxParts, encodingType := getObjectResources(testCase.values) + uploadID, partNumberMarker, maxParts, encodingType, argsErr := getObjectResources(testCase.values) + if argsErr != ErrNone { + t.Errorf("Test %d: argument parsing failed, got %v", i+1, argsErr) + } if uploadID != testCase.uploadID { t.Errorf("Test %d: Expected %s, got %s", i+1, testCase.uploadID, uploadID) } diff --git a/cmd/bucket-handlers-listobjects.go b/cmd/bucket-handlers-listobjects.go index ccda15713..f8e0ebb08 100644 --- a/cmd/bucket-handlers-listobjects.go +++ b/cmd/bucket-handlers-listobjects.go @@ -157,7 +157,11 @@ func (api objectAPIHandlers) ListObjectsV1Handler(w http.ResponseWriter, r *http } // Extract all the litsObjectsV1 query params to their native values. - prefix, marker, delimiter, maxKeys, _ := getListObjectsV1Args(r.URL.Query()) + prefix, marker, delimiter, maxKeys, _, s3Error := getListObjectsV1Args(r.URL.Query()) + if s3Error != ErrNone { + writeErrorResponse(w, s3Error, r.URL) + return + } // Validate the maxKeys lowerbound. When maxKeys > 1000, S3 returns 1000 but // does not throw an error. diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 3c4bfd930..fb99076bc 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -155,7 +155,11 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter, return } - prefix, keyMarker, uploadIDMarker, delimiter, maxUploads, _ := getBucketMultipartResources(r.URL.Query()) + prefix, keyMarker, uploadIDMarker, delimiter, maxUploads, _, s3Error := getBucketMultipartResources(r.URL.Query()) + if s3Error != ErrNone { + writeErrorResponse(w, s3Error, r.URL) + return + } if maxUploads < 0 { writeErrorResponse(w, ErrInvalidMaxUploads, r.URL) return diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 890bac5e8..90f820f71 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1861,7 +1861,11 @@ func (api objectAPIHandlers) AbortMultipartUploadHandler(w http.ResponseWriter, } } - uploadID, _, _, _ := getObjectResources(r.URL.Query()) + uploadID, _, _, _, s3Error := getObjectResources(r.URL.Query()) + if s3Error != ErrNone { + writeErrorResponse(w, s3Error, r.URL) + return + } if err := abortMultipartUpload(ctx, bucket, object, uploadID); err != nil { writeErrorResponse(w, toAPIErrorCode(err), r.URL) return @@ -1890,7 +1894,11 @@ func (api objectAPIHandlers) ListObjectPartsHandler(w http.ResponseWriter, r *ht return } - uploadID, partNumberMarker, maxParts, _ := getObjectResources(r.URL.Query()) + uploadID, partNumberMarker, maxParts, _, s3Error := getObjectResources(r.URL.Query()) + if s3Error != ErrNone { + writeErrorResponse(w, s3Error, r.URL) + return + } if partNumberMarker < 0 { writeErrorResponse(w, ErrInvalidPartNumberMarker, r.URL) return @@ -1941,7 +1949,11 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite } // Get upload id. - uploadID, _, _, _ := getObjectResources(r.URL.Query()) + uploadID, _, _, _, s3Error := getObjectResources(r.URL.Query()) + if s3Error != ErrNone { + writeErrorResponse(w, s3Error, r.URL) + return + } completeMultipartBytes, err := goioutil.ReadAll(r.Body) if err != nil {