Return proper errors for invalid bodies (#7179)

This commit is contained in:
Harshavardhana 2019-01-31 07:19:09 -08:00 committed by kannappanr
parent 36dae04671
commit 432aec73d9
2 changed files with 23 additions and 21 deletions

View File

@ -40,7 +40,7 @@ func parseLocationConstraint(r *http.Request) (location string, s3Error APIError
// be created at default region. // be created at default region.
locationConstraint := createBucketLocationConfiguration{} locationConstraint := createBucketLocationConfiguration{}
err := xmlDecoder(r.Body, &locationConstraint, r.ContentLength) err := xmlDecoder(r.Body, &locationConstraint, r.ContentLength)
if err != nil && err != io.EOF { if err != nil && r.ContentLength != 0 {
logger.LogIf(context.Background(), err) logger.LogIf(context.Background(), err)
// Treat all other failures as XML parsing errors. // Treat all other failures as XML parsing errors.
return "", ErrMalformedXML return "", ErrMalformedXML

View File

@ -39,48 +39,50 @@ func TestIsValidLocationContraint(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
// Test with corrupted XML // Corrupted XML
malformedReq := &http.Request{ malformedReq := &http.Request{
Body: ioutil.NopCloser(bytes.NewBuffer([]byte("<>"))), Body: ioutil.NopCloser(bytes.NewBuffer([]byte("<>"))),
ContentLength: int64(len("<>")), ContentLength: int64(len("<>")),
} }
if _, err := parseLocationConstraint(malformedReq); err != ErrMalformedXML {
t.Fatal("Unexpected error: ", err) // Not an XML
badRequest := &http.Request{
Body: ioutil.NopCloser(bytes.NewReader([]byte("garbage"))),
ContentLength: int64(len("garbage")),
} }
// generates the input request with XML bucket configuration set to the request body. // generates the input request with XML bucket configuration set to the request body.
createExpectedRequest := func(req *http.Request, location string) (*http.Request, error) { createExpectedRequest := func(req *http.Request, location string) *http.Request {
createBucketConfig := createBucketLocationConfiguration{} createBucketConfig := createBucketLocationConfiguration{}
createBucketConfig.Location = location createBucketConfig.Location = location
var createBucketConfigBytes []byte createBucketConfigBytes, _ := xml.Marshal(createBucketConfig)
createBucketConfigBytes, e := xml.Marshal(createBucketConfig)
if e != nil {
return nil, e
}
createBucketConfigBuffer := bytes.NewBuffer(createBucketConfigBytes) createBucketConfigBuffer := bytes.NewBuffer(createBucketConfigBytes)
req.Body = ioutil.NopCloser(createBucketConfigBuffer) req.Body = ioutil.NopCloser(createBucketConfigBuffer)
req.ContentLength = int64(createBucketConfigBuffer.Len()) req.ContentLength = int64(createBucketConfigBuffer.Len())
return req, nil return req
} }
testCases := []struct { testCases := []struct {
locationForInputRequest string request *http.Request
serverConfigRegion string serverConfigRegion string
expectedCode APIErrorCode expectedCode APIErrorCode
}{ }{
// Test case - 1. // Test case - 1.
{globalMinioDefaultRegion, globalMinioDefaultRegion, ErrNone}, {createExpectedRequest(&http.Request{}, "eu-central-1"), globalMinioDefaultRegion, ErrNone},
// Test case - 2. // Test case - 2.
// In case of empty request body ErrNone is returned. // In case of empty request body ErrNone is returned.
{"", globalMinioDefaultRegion, ErrNone}, {createExpectedRequest(&http.Request{}, ""), globalMinioDefaultRegion, ErrNone},
// Test case - 3
// In case of garbage request body ErrMalformedXML is returned.
{badRequest, globalMinioDefaultRegion, ErrMalformedXML},
// Test case - 4
// In case of invalid XML request body ErrMalformedXML is returned.
{malformedReq, globalMinioDefaultRegion, ErrMalformedXML},
} }
for i, testCase := range testCases { 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)
}
globalServerConfig.SetRegion(testCase.serverConfigRegion) globalServerConfig.SetRegion(testCase.serverConfigRegion)
_, actualCode := parseLocationConstraint(inputRequest) _, actualCode := parseLocationConstraint(testCase.request)
if testCase.expectedCode != actualCode { if testCase.expectedCode != actualCode {
t.Errorf("Test %d: Expected the APIErrCode to be %d, but instead found %d", i+1, testCase.expectedCode, actualCode) t.Errorf("Test %d: Expected the APIErrCode to be %d, but instead found %d", i+1, testCase.expectedCode, actualCode)
} }