From 9b90af4cc4235797576d8d7e3f1e27b782c60656 Mon Sep 17 00:00:00 2001
From: Luc Talatinian <102624213+lucix-aws@users.noreply.github.com>
Date: Thu, 7 Dec 2023 16:29:34 -0500
Subject: [PATCH] fix: add non-vhostable buckets to path when using legacy
 endpoint resolver (#2417)

---
 .../422df6b2a00a43a6a0eeac3c4f27c1a9.json      |  8 ++++++++
 .../customizations/update_endpoint_test.go     | 18 ++++++++++++++++++
 .../s3/serialize_immutable_hostname_bucket.go  | 15 ++++++++++-----
 3 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 .changelog/422df6b2a00a43a6a0eeac3c4f27c1a9.json

diff --git a/.changelog/422df6b2a00a43a6a0eeac3c4f27c1a9.json b/.changelog/422df6b2a00a43a6a0eeac3c4f27c1a9.json
new file mode 100644
index 00000000000..c3b8beed778
--- /dev/null
+++ b/.changelog/422df6b2a00a43a6a0eeac3c4f27c1a9.json
@@ -0,0 +1,8 @@
+{
+    "id": "422df6b2-a00a-43a6-a0ee-ac3c4f27c1a9",
+    "type": "bugfix",
+    "description": "Add non-vhostable buckets to request path when using legacy V1 endpoint resolver.",
+    "modules": [
+        "service/s3"
+    ]
+}
\ No newline at end of file
diff --git a/service/s3/internal/customizations/update_endpoint_test.go b/service/s3/internal/customizations/update_endpoint_test.go
index b8c5ef1c4dc..ece67c21497 100644
--- a/service/s3/internal/customizations/update_endpoint_test.go
+++ b/service/s3/internal/customizations/update_endpoint_test.go
@@ -55,6 +55,24 @@ func Test_UpdateEndpointBuild(t *testing.T) {
 					{"abc", "k:e,y", "https://abc.s3.mock-region.amazonaws.com/k%3Ae%2Cy?x-id=GetObject", ""},
 				},
 			},
+			"VirtualHostStyleBucketV1EndpointHTTPS": {
+				customEndpoint: &aws.Endpoint{
+					URL: "https://foo.bar",
+				},
+				tests: []s3BucketTest{
+					{"abc", "key", "https://abc.foo.bar/key?x-id=GetObject", ""},
+					{"a.b.c", "key", "https://foo.bar/a.b.c/key?x-id=GetObject", ""},
+				},
+			},
+			"VirtualHostStyleBucketV1EndpointHTTP": {
+				customEndpoint: &aws.Endpoint{
+					URL: "http://foo.bar",
+				},
+				tests: []s3BucketTest{
+					{"abc", "key", "http://abc.foo.bar/key?x-id=GetObject", ""},
+					{"a.b.c", "key", "http://a.b.c.foo.bar/key?x-id=GetObject", ""},
+				},
+			},
 			"Accelerate": {
 				useAccelerate: true,
 				tests: []s3BucketTest{
diff --git a/service/s3/serialize_immutable_hostname_bucket.go b/service/s3/serialize_immutable_hostname_bucket.go
index cb22779fb78..4e34d1a22f3 100644
--- a/service/s3/serialize_immutable_hostname_bucket.go
+++ b/service/s3/serialize_immutable_hostname_bucket.go
@@ -3,9 +3,10 @@ package s3
 import (
 	"context"
 	"fmt"
-	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
 	"path"
 
+	awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
+
 	"github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn"
 	smithy "github.com/aws/smithy-go"
 	"github.com/aws/smithy-go/encoding/httpbinding"
@@ -38,16 +39,20 @@ func (m *serializeImmutableHostnameBucketMiddleware) HandleSerialize(
 	if !ok {
 		return out, metadata, &smithy.SerializationError{Err: fmt.Errorf("unknown transport type %T", in.Request)}
 	}
-	if !smithyhttp.GetHostnameImmutable(ctx) &&
-		!(awsmiddleware.GetRequiresLegacyEndpoints(ctx) && m.UsePathStyle) {
-		return next.HandleSerialize(ctx, in)
-	}
 
 	bucket, ok := bucketFromInput(in.Parameters)
 	if !ok {
 		return next.HandleSerialize(ctx, in)
 	}
 
+	// a bucket being un-vhostable will also force us to use path style
+	usePathStyle := m.UsePathStyle || !awsrulesfn.IsVirtualHostableS3Bucket(bucket, request.URL.Scheme != "https")
+
+	if !smithyhttp.GetHostnameImmutable(ctx) &&
+		!(awsmiddleware.GetRequiresLegacyEndpoints(ctx) && usePathStyle) {
+		return next.HandleSerialize(ctx, in)
+	}
+
 	parsedBucket := awsrulesfn.ParseARN(bucket)
 
 	// disallow ARN buckets except for MRAP arns