From b6919f3da74fe3714f5c9283c99920a85a003a8a Mon Sep 17 00:00:00 2001
From: Jason Del Ponte <delpontej@gmail.com>
Date: Thu, 7 Nov 2019 10:12:21 -0800
Subject: [PATCH] aws/request: Ensure New request handles nil retryer

Adds additional default behavior to the SDK's New request constructor,
to handle the case where a nil Retryer was passed in. This error could
occur when the SDK's Request type was being used to create requests
directly, not through one of the SDK's client.

Fixes #2889
---
 aws/request/request.go      | 12 ++++++++++--
 aws/request/retryer.go      | 27 ++++++++++++++++++++++++++-
 aws/request/retryer_test.go | 15 +++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/aws/request/request.go b/aws/request/request.go
index 8e332cce6a..52178141da 100644
--- a/aws/request/request.go
+++ b/aws/request/request.go
@@ -99,8 +99,12 @@ type Operation struct {
 	BeforePresignFn func(r *Request) error
 }
 
-// New returns a new Request pointer for the service API
-// operation and parameters.
+// New returns a new Request pointer for the service API operation and
+// parameters.
+//
+// A Retryer should be provided to direct how the request is retried. If
+// Retryer is nil, a default no retry value will be used. You can use
+// NoOpRetryer in the Client package to disable retry behavior directly.
 //
 // Params is any value of input parameters to be the request payload.
 // Data is pointer value to an object which the request's response
@@ -108,6 +112,10 @@ type Operation struct {
 func New(cfg aws.Config, clientInfo metadata.ClientInfo, handlers Handlers,
 	retryer Retryer, operation *Operation, params interface{}, data interface{}) *Request {
 
+	if retryer == nil {
+		retryer = noOpRetryer{}
+	}
+
 	method := operation.HTTPMethod
 	if method == "" {
 		method = "POST"
diff --git a/aws/request/retryer.go b/aws/request/retryer.go
index e84084da5e..7d9179f55b 100644
--- a/aws/request/retryer.go
+++ b/aws/request/retryer.go
@@ -35,10 +35,35 @@ type Retryer interface {
 }
 
 // WithRetryer sets a Retryer value to the given Config returning the Config
-// value for chaining.
+// value for chaining. The value must not be nil.
 func WithRetryer(cfg *aws.Config, retryer Retryer) *aws.Config {
 	cfg.Retryer = retryer
 	return cfg
+
+}
+
+// noOpRetryer is a internal no op retryer used when a request is created
+// without a retryer.
+//
+// Provides a retryer that performs no retries.
+// It should be used when we do not want retries to be performed.
+type noOpRetryer struct{}
+
+// MaxRetries returns the number of maximum returns the service will use to make
+// an individual API; For NoOpRetryer the MaxRetries will always be zero.
+func (d noOpRetryer) MaxRetries() int {
+	return 0
+}
+
+// ShouldRetry will always return false for NoOpRetryer, as it should never retry.
+func (d noOpRetryer) ShouldRetry(_ *Request) bool {
+	return false
+}
+
+// RetryRules returns the delay duration before retrying this request again;
+// since NoOpRetryer does not retry, RetryRules always returns 0.
+func (d noOpRetryer) RetryRules(_ *Request) time.Duration {
+	return 0
 }
 
 // retryableCodes is a collection of service response codes which are retry-able
diff --git a/aws/request/retryer_test.go b/aws/request/retryer_test.go
index 6dcc7762d7..96db17a8cb 100644
--- a/aws/request/retryer_test.go
+++ b/aws/request/retryer_test.go
@@ -5,7 +5,9 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/client/metadata"
 )
 
 func TestRequestThrottling(t *testing.T) {
@@ -64,3 +66,16 @@ func TestIsErrorRetryable(t *testing.T) {
 		}
 	}
 }
+
+func TestRequest_NilRetyer(t *testing.T) {
+	clientInfo := metadata.ClientInfo{Endpoint: "https://mock.region.amazonaws.com"}
+	req := New(aws.Config{}, clientInfo, Handlers{}, nil, &Operation{}, nil, nil)
+
+	if req.Retryer == nil {
+		t.Fatalf("expect retryer to be set")
+	}
+
+	if e, a := 0, req.MaxRetries(); e != a {
+		t.Errorf("expect no retries, got %v", a)
+	}
+}