Add error handling in api-resource.go (#6651)

minio · Oct 18, 2018 · b43e833 · b43e833
1 parent 30040fb
commit b43e833
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 {