aws/defaults: Fix request metadata headers causing signature errors (#537)

Fixes the SDK's adding the request metadata headers in the wrong
location within the request handler stack. This created a situation
where a request that was retried would sign the new attempt using the
old value of the header. The header value would then be changed before
sending the request.

This moves the request invocation id to only occur during build,
maintaining its value across all requests attempts. Also moves request
retry metadata header to be before sign, and after build. This ensures
that a new value for each attempt can be set, and included in each
request attempt.

Fix #533
Fix #521

Author: jasdel

CHANGELOG_PENDING.md

@@ -39,3 +39,7 @@ SDK Bugs
 * `internal/sdk`: Fix SDK's UUID utility to handle partial read ([#536](https://github.com/aws/aws-sdk-go-v2/pull/536))
   * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand.
   * Fix [#534](https://github.com/aws/aws-sdk-go-v2/issues/534)
+* `aws/defaults`: Fix request metadata headers causing signature errors ([#536](https://github.com/aws/aws-sdk-go-v2/pull/536))
+    * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request.
+    * Fix [#533](https://github.com/aws/aws-sdk-go-v2/issues/533)
+    * Fix [#521](https://github.com/aws/aws-sdk-go-v2/issues/521)

aws/defaults/defaults.go

@@ -70,10 +70,10 @@ func Handlers() aws.Handlers {
 	handlers.Validate.AfterEachFn = aws.HandlerListStopOnError
 	handlers.Build.PushBackNamed(SDKVersionUserAgentHandler)
 	handlers.Build.PushBackNamed(AddHostExecEnvUserAgentHander)
+	handlers.Build.PushFrontNamed(RequestInvocationIDHeaderHandler)
 	handlers.Build.AfterEachFn = aws.HandlerListStopOnError
 	handlers.Sign.PushBackNamed(BuildContentLengthHandler)
-	handlers.Send.PushFrontNamed(RequestInvocationIDHeaderHandler)
-	handlers.Send.PushFrontNamed(RetryMetricHeaderHandler)
+	handlers.Sign.PushFrontNamed(RetryMetricHeaderHandler)
 	handlers.Send.PushBackNamed(ValidateReqSigHandler)
 	handlers.Send.PushBackNamed(SendHandler)
 	handlers.Send.PushBackNamed(AttemptClockSkewHandler)

aws/defaults/handlers.go

@@ -190,6 +190,12 @@ var ValidateResponseHandler = aws.NamedHandler{
 var RequestInvocationIDHeaderHandler = aws.NamedHandler{
 	Name: "core.RequestInvocationIDHeaderHandler",
 	Fn: func(r *aws.Request) {
+		if r.ExpireTime != 0 {
+			// ExpireTime set implies a presigned URL which will not have the
+			// header applied.
+			return
+		}
+
 		const invocationIDHeader = "amz-sdk-invocation-id"
 		r.HTTPRequest.Header.Set(invocationIDHeader, r.InvocationID)
 	}}
@@ -199,6 +205,12 @@ var RequestInvocationIDHeaderHandler = aws.NamedHandler{
 var RetryMetricHeaderHandler = aws.NamedHandler{
 	Name: "core.RetryMetricHeaderHandler",
 	Fn: func(r *aws.Request) {
+		if r.ExpireTime != 0 {
+			// ExpireTime set implies a presigned URL which will not have the
+			// header applied.
+			return
+		}
+
 		const retryMetricHeader = "amz-sdk-request"
 		var parts []string
 

aws/defaults/handlers_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -333,16 +334,45 @@ func TestSendHandler_HEADNoBody(t *testing.T) {
 
 func TestRequestInvocationIDHeaderHandler(t *testing.T) {
 	cfg := unit.Config()
-	r := aws.New(cfg, aws.Metadata{}, cfg.Handlers, aws.NoOpRetryer{}, &aws.Operation{},
+	cfg.Handlers.Send.Clear()
+
+	var invokeID string
+	cfg.Handlers.Build.PushBack(func(r *aws.Request) {
+		invokeID = r.InvocationID
+		if len(invokeID) == 0 {
+			t.Fatalf("expect non-empty invocation id")
+		}
+	})
+	cfg.Handlers.Send.PushBack(func(r *aws.Request) {
+		if e, a := invokeID, r.InvocationID; e != a {
+			t.Errorf("expect %v invoke ID, got %v", e, a)
+		}
+		r.Error = &aws.RequestSendError{Err: io.ErrUnexpectedEOF}
+	})
+	retryer := retry.NewStandard(func(o *retry.StandardOptions) {
+		o.MaxAttempts = 3
+	})
+	r := aws.New(cfg, aws.Metadata{}, cfg.Handlers, retryer, &aws.Operation{},
 		&struct{}{}, struct{}{})
 
 	if len(r.InvocationID) == 0 {
 		t.Fatalf("expect invocation id, got none")
 	}
 
-	defaults.RequestInvocationIDHeaderHandler.Fn(r)
-	if r.Error != nil {
-		t.Fatalf("expect no error, got %v", r.Error)
+	err := r.Send()
+	if err == nil {
+		t.Fatalf("expect error got on")
+	}
+	var maxErr *aws.MaxAttemptsError
+	if !errors.As(err, &maxErr) {
+		t.Fatalf("expect max errors, got %v", err)
+	} else {
+		if e, a := 3, maxErr.Attempt; e != a {
+			t.Errorf("expect %v attempts, got %v", e, a)
+		}
+	}
+	if len(invokeID) == 0 {
+		t.Fatalf("expect non-empty invocation id")
 	}
 
 	if e, a := r.InvocationID, r.HTTPRequest.Header.Get("amz-sdk-invocation-id"); e != a {