fix: don't expect response to be json in endpointcreds provider

aws · Nov 21, 2023 · 33408a6 · 33408a6
1 parent 3bd97c0
commit 33408a6
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 22 deletions.
diff --git a/.changelog/018d3cef4def4b019c5ac7c60555b7e3.json 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"
+    ]
+}
diff --git a/credentials/endpointcreds/internal/client/client_test.go b/credentials/endpointcreds/internal/client/client_test.go
@@ -18,21 +18,23 @@ 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,
 			ResponseBody: []byte(` {
    "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(`<html><body>unexpected message format</body></html>`),
-			ExpectErr:    true,
+			ResponseCode:        500,
+			ResponseBody:        []byte(`<html><body>unexpected message format</body></html>`),
+			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 := "<html><body>unexpected message format</body></html>", 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
@@ -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
@@ -201,6 +201,9 @@ func TestFailedRetrieveCredentials(t *testing.T) {
   "code": "Error",
   "message": "Message"
 }`))),
+				Header: http.Header{
+					"Content-Type": {"application/json"},
+				},
 			}, nil
 		})
 	})