diff --git a/service/route53/service.go b/service/route53/service.go index f4a82bde562..0caa3de7bdd 100644 --- a/service/route53/service.go +++ b/service/route53/service.go @@ -62,7 +62,9 @@ func newClient(cfg aws.Config, handlers request.Handlers, endpoint, signingRegio svc.Handlers.Build.PushBackNamed(restxml.BuildHandler) svc.Handlers.Unmarshal.PushBackNamed(restxml.UnmarshalHandler) svc.Handlers.UnmarshalMeta.PushBackNamed(restxml.UnmarshalMetaHandler) - svc.Handlers.UnmarshalError.PushBackNamed(restxml.UnmarshalErrorHandler) + + // Route53 uses a custom error parser + svc.Handlers.UnmarshalError.PushBack(unmarshalError) // Run custom client initialization if present if initClient != nil { diff --git a/service/route53/unmarshal_error.go b/service/route53/unmarshal_error.go new file mode 100644 index 00000000000..5a1caf775d6 --- /dev/null +++ b/service/route53/unmarshal_error.go @@ -0,0 +1,86 @@ +package route53 + +import ( + "bytes" + "encoding/xml" + "io/ioutil" + + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/private/protocol/restxml" +) + +type baseXMLErrorResponse struct { + XMLName xml.Name +} + +type standardXMLErrorResponse struct { + XMLName xml.Name `xml:"ErrorResponse"` + Code string `xml:"Error>Code"` + Message string `xml:"Error>Message"` + RequestID string `xml:"RequestId"` +} + +type invalidChangeBatchXMLErrorResponse struct { + XMLName xml.Name `xml:"InvalidChangeBatch"` + Messages []string `xml:"Messages>Message"` +} + +func unmarshalError(r *request.Request) { + switch r.Operation.Name { + case opChangeResourceRecordSets: + unmarshalChangeResourceRecordSetsError(r) + default: + restxml.UnmarshalError(r) + } +} + +func unmarshalChangeResourceRecordSetsError(r *request.Request) { + defer r.HTTPResponse.Body.Close() + + responseBody, err := ioutil.ReadAll(r.HTTPResponse.Body) + + if err != nil { + r.Error = awserr.New("SerializationError", "failed to read Route53 XML error response", err) + return + } + + baseError := &baseXMLErrorResponse{} + + if err := xml.Unmarshal(responseBody, baseError); err != nil { + r.Error = awserr.New("SerializationError", "failed to decode Route53 XML error response", err) + return + } + + switch baseError.XMLName.Local { + case "InvalidChangeBatch": + unmarshalInvalidChangeBatchError(r, responseBody) + default: + r.HTTPResponse.Body = ioutil.NopCloser(bytes.NewReader(responseBody)) + restxml.UnmarshalError(r) + } +} + +func unmarshalInvalidChangeBatchError(r *request.Request, requestBody []byte) { + resp := &invalidChangeBatchXMLErrorResponse{} + err := xml.Unmarshal(requestBody, resp) + + if err != nil { + r.Error = awserr.New("SerializationError", "failed to decode query XML error response", err) + return + } + + const errorCode = "InvalidChangeBatch" + errors := []error{} + + for _, msg := range resp.Messages { + errors = append(errors, awserr.New(errorCode, msg, nil)) + } + + r.Error = awserr.NewRequestFailure( + awserr.NewBatchError(errorCode, "ChangeBatch errors occured", errors), + r.HTTPResponse.StatusCode, + r.RequestID, + ) + +} diff --git a/service/route53/unmarshal_error_leak_test.go b/service/route53/unmarshal_error_leak_test.go new file mode 100644 index 00000000000..2bafdd3700c --- /dev/null +++ b/service/route53/unmarshal_error_leak_test.go @@ -0,0 +1,37 @@ +package route53 + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/awstesting" +) + +func TestUnmarhsalErrorLeak(t *testing.T) { + req := &request.Request{ + Operation: &request.Operation{ + Name: opChangeResourceRecordSets, + }, + HTTPRequest: &http.Request{ + Header: make(http.Header), + Body: &awstesting.ReadCloser{Size: 2048}, + }, + } + req.HTTPResponse = &http.Response{ + Body: &awstesting.ReadCloser{Size: 2048}, + Header: http.Header{ + "X-Amzn-Requestid": []string{"1"}, + }, + StatusCode: http.StatusOK, + } + + reader := req.HTTPResponse.Body.(*awstesting.ReadCloser) + unmarshalError(req) + + assert.NotNil(t, req.Error) + assert.Equal(t, reader.Closed, true) + assert.Equal(t, reader.Size, 0) +} diff --git a/service/route53/unmarshal_error_test.go b/service/route53/unmarshal_error_test.go new file mode 100644 index 00000000000..97a06dc1faf --- /dev/null +++ b/service/route53/unmarshal_error_test.go @@ -0,0 +1,111 @@ +package route53_test + +import ( + "bytes" + "io/ioutil" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/awstesting/unit" + "github.com/aws/aws-sdk-go/service/route53" +) + +func makeClientWithResponse(response string) *route53.Route53 { + r := route53.New(unit.Session) + r.Handlers.Send.Clear() + r.Handlers.Send.PushBack(func(r *request.Request) { + body := ioutil.NopCloser(bytes.NewReader([]byte(response))) + r.HTTPResponse = &http.Response{ + ContentLength: int64(len(response)), + StatusCode: 400, + Status: "Bad Request", + Body: body, + } + }) + + return r +} + +func TestUnmarshalStandardError(t *testing.T) { + const errorResponse = ` + + + InvalidDomainName + The domain name is invalid + + 12345 + +` + + r := makeClientWithResponse(errorResponse) + + _, err := r.CreateHostedZone(&route53.CreateHostedZoneInput{ + CallerReference: aws.String("test"), + Name: aws.String("test_zone"), + }) + + assert.Error(t, err) + assert.Equal(t, "InvalidDomainName", err.(awserr.Error).Code()) + assert.Equal(t, "The domain name is invalid", err.(awserr.Error).Message()) +} + +func TestUnmarshalInvalidChangeBatch(t *testing.T) { + const errorMessage = ` +Tried to create resource record set duplicate.example.com. type A, +but it already exists +` + const errorResponse = ` + + + ` + errorMessage + ` + + +` + + r := makeClientWithResponse(errorResponse) + + req := &route53.ChangeResourceRecordSetsInput{ + HostedZoneId: aws.String("zoneId"), + ChangeBatch: &route53.ChangeBatch{ + Changes: []*route53.Change{ + &route53.Change{ + Action: aws.String("CREATE"), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("domain"), + Type: aws.String("CNAME"), + TTL: aws.Int64(120), + ResourceRecords: []*route53.ResourceRecord{ + { + Value: aws.String("cname"), + }, + }, + }, + }, + }, + }, + } + + _, err := r.ChangeResourceRecordSets(req) + assert.Error(t, err) + + if reqErr, ok := err.(awserr.RequestFailure); ok { + assert.Error(t, reqErr) + assert.Equal(t, 400, reqErr.StatusCode()) + } else { + assert.Fail(t, "returned error is not a RequestFailure") + } + + if batchErr, ok := err.(awserr.BatchedErrors); ok { + errs := batchErr.OrigErrs() + assert.Len(t, errs, 1) + assert.Equal(t, "InvalidChangeBatch", errs[0].(awserr.Error).Code()) + assert.Equal(t, errorMessage, errs[0].(awserr.Error).Message()) + } else { + assert.Fail(t, "returned error is not a BatchedErrors") + } +}