Fix http_request_timeout disabling for S3

Author: jochenehret

s3/config/config.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"math"
+	"strconv"
 	"strings"
 	"time"
 )
@@ -74,7 +75,7 @@ const credentialsSourceEnvOrProfile = "env_or_profile"
 const noCredentialsSourceProvided = ""
 
 var errorStaticCredentialsMissing = errors.New("access_key_id and secret_access_key must be provided")
-var errorNonPositiveHTTPRequestTimeout = errors.New("http_request_timeout must be greater than 0")
+var errorNegativeHTTPRequestTimeout = errors.New("http_request_timeout must not be negative")
 
 type errorStaticCredentialsPresent struct {
 	credentialsSource string
@@ -269,17 +270,17 @@ func (c *S3Cli) HTTPRequestTimeoutValue() (time.Duration, error) {
 		return defaultHTTPRequestTimeout, nil
 	}
 
-	if c.HTTPRequestTimeout == "0" {
-		return 0, nil
+	if _, err := strconv.ParseFloat(c.HTTPRequestTimeout, 64); err == nil {
+		return 0, fmt.Errorf("invalid http_request_timeout: missing duration unit")
 	}
 
 	httpRequestTimeout, err := time.ParseDuration(c.HTTPRequestTimeout)
 	if err != nil {
 		return 0, fmt.Errorf("invalid http_request_timeout: %w", err)
 	}
 
-	if httpRequestTimeout <= 0 {
-		return 0, errorNonPositiveHTTPRequestTimeout
+	if httpRequestTimeout < 0 {
+		return 0, errorNegativeHTTPRequestTimeout
 	}
 
 	return httpRequestTimeout, nil

s3/config/config_test.go

@@ -421,17 +421,27 @@ var _ = Describe("BlobstoreClient configuration", func() {
 			Expect(timeout.Seconds()).To(Equal(45.0))
 		})
 
-		It("disables timeout when set to 0", func() {
-			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":"0"}`)
+		It("disables timeout when set to 0s", func() {
+			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":"0s"}`)
 			dummyJSONReader := bytes.NewReader(dummyJSONBytes)
 
 			c, err := config.NewFromReader(dummyJSONReader)
 			Expect(err).ToNot(HaveOccurred())
+			Expect(c.HTTPRequestTimeout).To(Equal("0s"))
 			timeout, err := c.HTTPRequestTimeoutValue()
 			Expect(err).ToNot(HaveOccurred())
 			Expect(timeout).To(BeZero())
 		})
 
+		It("rejects numeric timeout values", func() {
+			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":45}`)
+			dummyJSONReader := bytes.NewReader(dummyJSONBytes)
+
+			_, err := config.NewFromReader(dummyJSONReader)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("cannot unmarshal number into Go struct field"))
+		})
+
 		It("rejects invalid duration formats", func() {
 			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":"bananas"}`)
 			dummyJSONReader := bytes.NewReader(dummyJSONBytes)
@@ -441,12 +451,12 @@ var _ = Describe("BlobstoreClient configuration", func() {
 			Expect(err.Error()).To(ContainSubstring("invalid http_request_timeout"))
 		})
 
-		It("rejects non-positive durations", func() {
-			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":"0s"}`)
+		It("rejects negative durations", func() {
+			dummyJSONBytes := []byte(`{"access_key_id":"id","secret_access_key":"key","bucket_name":"some-bucket","http_request_timeout":"-1s"}`)
 			dummyJSONReader := bytes.NewReader(dummyJSONBytes)
 
 			_, err := config.NewFromReader(dummyJSONReader)
-			Expect(err).To(MatchError("http_request_timeout must be greater than 0"))
+			Expect(err).To(MatchError("http_request_timeout must not be negative"))
 		})
 	})