From 33408a6010720aa093e284cf670e97f16b01a996 Mon Sep 17 00:00:00 2001 From: Luc Talatinian Date: Mon, 20 Nov 2023 18:09:29 -0500 Subject: [PATCH] fix: don't expect response to be json in endpointcreds provider --- .../018d3cef4def4b019c5ac7c60555b7e3.json | 8 +++ .../internal/client/client_test.go | 53 +++++++++++++------ .../internal/client/middleware.go | 40 +++++++++++--- credentials/endpointcreds/provider_test.go | 3 ++ 4 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 .changelog/018d3cef4def4b019c5ac7c60555b7e3.json diff --git a/.changelog/018d3cef4def4b019c5ac7c60555b7e3.json b/.changelog/018d3cef4def4b019c5ac7c60555b7e3.json new file mode 100644 index 00000000000..a1461d3686a --- /dev/null +++ b/.changelog/018d3cef4def4b019c5ac7c60555b7e3.json @@ -0,0 +1,8 @@ +{ + "id": "018d3cef-4def-4b01-9c5a-c7c60555b7e3", + "type": "bugfix", + "description": "Don't expect error responses to have a JSON payload in the endpointcreds provider.", + "modules": [ + "credentials" + ] +} \ No newline at end of file diff --git a/credentials/endpointcreds/internal/client/client_test.go b/credentials/endpointcreds/internal/client/client_test.go index ac7e37524b1..fb28373a9eb 100644 --- a/credentials/endpointcreds/internal/client/client_test.go +++ b/credentials/endpointcreds/internal/client/client_test.go @@ -18,14 +18,15 @@ import ( func TestClient_GetCredentials(t *testing.T) { cases := map[string]struct { - Token string - RelativeURI string - ResponseCode int - ResponseBody []byte - ExpectResult *GetCredentialsOutput - ExpectErr bool - ValidateRequest func(*testing.T, *http.Request) - ValidateError func(*testing.T, error) bool + Token string + RelativeURI string + ResponseCode int + ResponseBody []byte + ResponseContentType string + ExpectResult *GetCredentialsOutput + ExpectErr bool + ValidateRequest func(*testing.T, *http.Request) + ValidateError func(*testing.T, error) bool }{ "success static": { ResponseCode: 200, @@ -33,6 +34,7 @@ func TestClient_GetCredentials(t *testing.T) { "AccessKeyId" : "FooKey", "SecretAccessKey" : "FooSecret" }`), + ResponseContentType: "application/json", ExpectResult: &GetCredentialsOutput{ AccessKeyID: "FooKey", SecretAccessKey: "FooSecret", @@ -45,6 +47,7 @@ func TestClient_GetCredentials(t *testing.T) { "AccessKeyId" : "FooKey", "SecretAccessKey" : "FooSecret" }`), + ResponseContentType: "application/json", ExpectResult: &GetCredentialsOutput{ AccessKeyID: "FooKey", SecretAccessKey: "FooSecret", @@ -59,6 +62,7 @@ func TestClient_GetCredentials(t *testing.T) { "Token": "FooToken", "Expiration": "2016-02-25T06:03:31Z" }`), + ResponseContentType: "application/json", ExpectResult: &GetCredentialsOutput{ AccessKeyID: "FooKey", SecretAccessKey: "FooSecret", @@ -76,6 +80,7 @@ func TestClient_GetCredentials(t *testing.T) { "AccessKeyId" : "FooKey", "SecretAccessKey" : "FooSecret" }`), + ResponseContentType: "application/json", ValidateRequest: func(t *testing.T, r *http.Request) { t.Helper() if e, a := "/path/to/thing", r.URL.Path; e != a { @@ -96,7 +101,8 @@ func TestClient_GetCredentials(t *testing.T) { "code": "Unauthorized", "message": "not authorized for endpoint" }`), - ExpectErr: true, + ResponseContentType: "application/json", + ExpectErr: true, ValidateError: func(t *testing.T, err error) (ok bool) { t.Helper() var apiError smithy.APIError @@ -126,7 +132,8 @@ func TestClient_GetCredentials(t *testing.T) { "code": "InternalError", "message": "an error occurred" }`), - ExpectErr: true, + ResponseContentType: "application/json", + ExpectErr: true, ValidateError: func(t *testing.T, err error) (ok bool) { t.Helper() var apiError smithy.APIError @@ -151,13 +158,28 @@ func TestClient_GetCredentials(t *testing.T) { }, }, "non-json error response": { - ResponseCode: 500, - ResponseBody: []byte(`unexpected message format`), - ExpectErr: true, + ResponseCode: 500, + ResponseBody: []byte(`unexpected message format`), + ResponseContentType: "text/html", + ExpectErr: true, ValidateError: func(t *testing.T, err error) (ok bool) { t.Helper() - if e, a := "failed to decode error message", err.Error(); !strings.Contains(a, e) { - t.Errorf("expect %v, got %v", e, a) + var apiError smithy.APIError + if errors.As(err, &apiError) { + if e, a := "", apiError.ErrorCode(); e != a { + t.Errorf("expect %v, got %v", e, a) + ok = false + } + if e, a := "unexpected message format", apiError.ErrorMessage(); e != a { + t.Errorf("expect %v, got %v", e, a) + ok = false + } + if e, a := smithy.FaultServer, apiError.ErrorFault(); e != a { + t.Errorf("expect %v, got %v", e, a) + ok = false + } + } else { + t.Errorf("expect %T error type, got %T: %v", apiError, err, err) ok = false } return ok @@ -177,6 +199,7 @@ func TestClient_GetCredentials(t *testing.T) { actualReq.Body = ioutil.NopCloser(bytes.NewReader(buf.Bytes())) + w.Header().Set("Content-Type", tt.ResponseContentType) w.WriteHeader(tt.ResponseCode) w.Write(tt.ResponseBody) })) diff --git a/credentials/endpointcreds/internal/client/middleware.go b/credentials/endpointcreds/internal/client/middleware.go index 40747a53c18..ab8ac444deb 100644 --- a/credentials/endpointcreds/internal/client/middleware.go +++ b/credentials/endpointcreds/internal/client/middleware.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/url" "github.com/aws/smithy-go" @@ -104,17 +105,42 @@ func (d *deserializeOpGetCredential) HandleDeserialize(ctx context.Context, in s } func deserializeError(response *smithyhttp.Response) error { - var errShape *EndpointError - err := json.NewDecoder(response.Body).Decode(&errShape) + // we could be talking to anything, json isn't guaranteed + // see https://github.com/aws/aws-sdk-go-v2/issues/2316 + if response.Header.Get("Content-Type") == "application/json" { + return deserializeJSONError(response) + } + + msg, err := io.ReadAll(response.Body) if err != nil { - return &smithy.DeserializationError{Err: fmt.Errorf("failed to decode error message, %w", err)} + return &smithy.DeserializationError{ + Err: fmt.Errorf("read response, %w", err), + } + } + + return &EndpointError{ + // no sensible value for Code + Message: string(msg), + Fault: stof(response.StatusCode), } +} - if response.StatusCode >= 500 { - errShape.Fault = smithy.FaultServer - } else { - errShape.Fault = smithy.FaultClient +func deserializeJSONError(response *smithyhttp.Response) error { + var errShape *EndpointError + if err := json.NewDecoder(response.Body).Decode(&errShape); err != nil { + return &smithy.DeserializationError{ + Err: fmt.Errorf("failed to decode error message, %w", err), + } } + errShape.Fault = stof(response.StatusCode) return errShape } + +// maps HTTP status code to smithy ErrorFault +func stof(code int) smithy.ErrorFault { + if code >= 500 { + return smithy.FaultServer + } + return smithy.FaultClient +} diff --git a/credentials/endpointcreds/provider_test.go b/credentials/endpointcreds/provider_test.go index 6f39ea4899c..167a340c531 100644 --- a/credentials/endpointcreds/provider_test.go +++ b/credentials/endpointcreds/provider_test.go @@ -201,6 +201,9 @@ func TestFailedRetrieveCredentials(t *testing.T) { "code": "Error", "message": "Message" }`))), + Header: http.Header{ + "Content-Type": {"application/json"}, + }, }, nil }) })