From cb1116725bca783512977098721beb206b0fc0f0 Mon Sep 17 00:00:00 2001 From: karthic rao Date: Thu, 21 Apr 2016 06:05:38 +0530 Subject: [PATCH] api: verify Location constraint for make bucket. (#1342) --- api-datatypes.go | 11 ++++++ bucket-handlers.go | 8 +++++ object-utils.go | 34 ++++++++++++++++++ object-utils_test.go | 86 +++++++++++++++++++++++++++++++------------- utils.go | 11 ++++-- 5 files changed, 123 insertions(+), 27 deletions(-) diff --git a/api-datatypes.go b/api-datatypes.go index 958b3af33..22cc4180f 100644 --- a/api-datatypes.go +++ b/api-datatypes.go @@ -16,11 +16,22 @@ package main +import ( + "encoding/xml" +) + // ObjectIdentifier carries key name for the object to delete. type ObjectIdentifier struct { ObjectName string `xml:"Key"` } +// createBucketConfiguration container for bucket configuration request from client. +// Used for parsing the location from the request body for MakeBucketbucket. +type createBucketLocationConfiguration struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ CreateBucketConfiguration" json:"-"` + Location string `xml:"LocationConstraint"` +} + // DeleteObjectsRequest - xml carrying the object key names which needs to be deleted. type DeleteObjectsRequest struct { // Element to enable quiet mode for the request diff --git a/bucket-handlers.go b/bucket-handlers.go index af6aeec2f..db4ccd78f 100644 --- a/bucket-handlers.go +++ b/bucket-handlers.go @@ -464,6 +464,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, // ---------- // This implementation of the PUT operation creates a new bucket for authenticated request func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) bucket := vars["bucket"] @@ -480,6 +481,13 @@ func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Req } } + // the location value in the request body should match the Region in serverConfig. + // other values of location are not accepted. + // make bucket fails in such cases. + errCode := isValidLocationContraint(r.Body, serverConfig.GetRegion()) + if errCode != ErrNone { + writeErrorResponse(w, r, errCode, r.URL.Path) + } // Make bucket. err := api.ObjectAPI.MakeBucket(bucket) if err != nil { diff --git a/object-utils.go b/object-utils.go index 1c31becb2..f1afb7f71 100644 --- a/object-utils.go +++ b/object-utils.go @@ -17,6 +17,7 @@ package main import ( + "io" "regexp" "strings" "unicode/utf8" @@ -93,3 +94,36 @@ func IsValidObjectPrefix(object string) bool { func pathJoin(path1 string, path2 string) string { return strings.TrimSuffix(path1, slashPathSeparator) + slashPathSeparator + path2 } + +// validates location constraint from the request body. +// the location value in the request body should match the Region in serverConfig. +// other values of location are not accepted. +// make bucket fails in such cases. +func isValidLocationContraint(reqBody io.Reader, serverRegion string) APIErrorCode { + var locationContraint createBucketLocationConfiguration + var errCode APIErrorCode + errCode = ErrNone + e := xmlDecoder(reqBody, &locationContraint) + if e != nil { + if e == io.EOF { + // Do nothing. + // failed due to empty body. The location will be set to default value from the serverConfig. + // this is valid. + errCode = ErrNone + } else { + // Failed due to malformed configuration. + errCode = ErrMalformedXML + //writeErrorResponse(w, r, ErrMalformedXML, r.URL.Path) + } + } else { + // Region obtained from the body. + // It should be equal to Region in serverConfig. + // Else ErrInvalidRegion returned. + // For empty value location will be to set to default value from the serverConfig. + if locationContraint.Location != "" && serverRegion != locationContraint.Location { + //writeErrorResponse(w, r, ErrInvalidRegion, r.URL.Path) + errCode = ErrInvalidRegion + } + } + return errCode +} diff --git a/object-utils_test.go b/object-utils_test.go index 24f2c2ad0..e8a371177 100644 --- a/object-utils_test.go +++ b/object-utils_test.go @@ -17,20 +17,14 @@ package main import ( + "bytes" + "encoding/xml" + "io/ioutil" + "net/http" "testing" ) -//Validating bucket name. -func ensureBucketName(t *testing.T, name string, testNum int, pass bool) { - isValidBucketName := IsValidBucketName(name) - if pass && !isValidBucketName { - t.Errorf("Test case %d: Expected \"%s\" to be a valid bucket name", testNum, name) - } - if !pass && isValidBucketName { - t.Errorf("Test case %d: Expected bucket name \"%s\" to be invalid", testNum, name) - } -} - +// Tests validate bucket name. func TestIsValidBucketName(t *testing.T) { testCases := []struct { bucketName string @@ -73,22 +67,17 @@ func TestIsValidBucketName(t *testing.T) { } for i, testCase := range testCases { - ensureBucketName(t, testCase.bucketName, i+1, testCase.shouldPass) + isValidBucketName := IsValidBucketName(testCase.bucketName) + if testCase.shouldPass && !isValidBucketName { + t.Errorf("Test case %d: Expected \"%s\" to be a valid bucket name", i+1, testCase.bucketName) + } + if !testCase.shouldPass && isValidBucketName { + t.Errorf("Test case %d: Expected bucket name \"%s\" to be invalid", i+1, testCase.bucketName) + } } } -//Test for validating object name. -func ensureObjectName(t *testing.T, name string, testNum int, pass bool) { - isValidObjectName := IsValidObjectName(name) - if pass && !isValidObjectName { - t.Errorf("Test case %d: Expected \"%s\" to be a valid object name", testNum, name) - } - if !pass && isValidObjectName { - t.Errorf("Test case %d: Expected object name \"%s\" to be invalid", testNum, name) - } - -} - +// Tests for validate object name. func TestIsValidObjectName(t *testing.T) { testCases := []struct { objectName string @@ -109,6 +98,53 @@ func TestIsValidObjectName(t *testing.T) { } for i, testCase := range testCases { - ensureObjectName(t, testCase.objectName, i+1, testCase.shouldPass) + isValidObjectName := IsValidObjectName(testCase.objectName) + if testCase.shouldPass && !isValidObjectName { + t.Errorf("Test case %d: Expected \"%s\" to be a valid object name", i+1, testCase.objectName) + } + if !testCase.shouldPass && isValidObjectName { + t.Errorf("Test case %d: Expected object name \"%s\" to be invalid", i+1, testCase.objectName) + } + } +} + +// Tests validate bucket LocationConstraint. +func TestIsValidLocationContraint(t *testing.T) { + // generates the input request with XML bucket configuration set to the request body. + createExpectedRequest := func(req *http.Request, location string) (*http.Request, error) { + createBucketConfig := createBucketLocationConfiguration{} + createBucketConfig.Location = location + var createBucketConfigBytes []byte + createBucketConfigBytes, e := xml.Marshal(createBucketConfig) + if e != nil { + return nil, e + } + createBucketConfigBuffer := bytes.NewBuffer(createBucketConfigBytes) + req.Body = ioutil.NopCloser(createBucketConfigBuffer) + return req, nil + } + + testCases := []struct { + locationForInputRequest string + serverConfigRegion string + expectedCode APIErrorCode + }{ + // Test case - 1. + {"us-east-1", "us-east-1", ErrNone}, + // Test case - 2. + // In case of empty request body ErrNone is returned. + {"", "us-east-1", ErrNone}, + // Test case - 3. + {"eu-central-1", "us-east-1", ErrInvalidRegion}, + } + for i, testCase := range testCases { + inputRequest, e := createExpectedRequest(&http.Request{}, testCase.locationForInputRequest) + if e != nil { + t.Fatalf("Test %d: Failed to Marshal bucket configuration", i+1) + } + actualCode := isValidLocationContraint(inputRequest.Body, testCase.serverConfigRegion) + if testCase.expectedCode != actualCode { + t.Errorf("Test %d: Expected the APIErrCode to be %d, but instead found %d", i+1, testCase.expectedCode, actualCode) + } } } diff --git a/utils.go b/utils.go index 59336edfb..d4de6fac5 100644 --- a/utils.go +++ b/utils.go @@ -18,11 +18,18 @@ package main import ( "encoding/base64" - "strings" - + "encoding/xml" "github.com/minio/minio/pkg/probe" + "io" + "strings" ) +// xmlDecoder provide decoded value in xml. +func xmlDecoder(body io.Reader, v interface{}) error { + d := xml.NewDecoder(body) + return d.Decode(v) +} + // checkValidMD5 - verify if valid md5, returns md5 in bytes. func checkValidMD5(md5 string) ([]byte, *probe.Error) { md5Bytes, e := base64.StdEncoding.DecodeString(strings.TrimSpace(md5))