fix(lib-storage): fix bucket duplication in hostname (#4157)

Author: kuhe

lib/lib-storage/src/Upload.spec.ts

@@ -21,13 +21,14 @@ const putObjectTaggingMock = jest.fn().mockResolvedValue({
   Success: "Tags have been applied!",
 });
 
-const endpointMock = jest.fn().mockResolvedValue({
-  hostname: "s3.region.amazonaws.com",
+let hostname = "s3.region.amazonaws.com";
+const endpointMock = jest.fn().mockImplementation(() => ({
+  hostname,
   port: undefined,
   protocol: "https:",
   path: "/",
   query: undefined,
-});
+}));
 
 import { EventEmitter, Readable } from "stream";
 
@@ -64,11 +65,11 @@ jest.mock("@aws-sdk/client-s3", () => ({
   PutObjectCommand: putObjectMock,
 }));
 
+import { AbortController } from "@aws-sdk/abort-controller";
 import { CompleteMultipartUploadCommandOutput, S3, S3Client } from "@aws-sdk/client-s3";
 import { createHash } from "crypto";
 
 import { Progress, Upload } from "./index";
-import { AbortController } from "@aws-sdk/abort-controller";
 
 const DEFAULT_PART_SIZE = 1024 * 1024 * 5;
 
@@ -245,6 +246,42 @@ describe(Upload.name, () => {
     expect(result.Location).toEqual("https://example-bucket.s3.region.amazonaws.com/example-key");
   });
 
+  describe("bucket hostname deduplication", () => {
+    afterEach(() => {
+      hostname = "s3.region.amazonaws.com";
+    });
+    it("should dedupe bucket in endpoint hostname when forcePathStyle = false", async () => {
+      hostname = "example-bucket.example-host.com";
+      const buffer = Buffer.from("");
+      const actionParams = { ...params, Body: buffer };
+      const upload = new Upload({
+        params: actionParams,
+        client: new S3({
+          forcePathStyle: false,
+        }),
+      });
+      const result = (await upload.done()) as CompleteMultipartUploadCommandOutput;
+      expect(result.Key).toEqual("example-key");
+      expect(result.Bucket).toEqual("example-bucket");
+      expect(result.Location).toEqual("https://example-bucket.example-host.com/example-key");
+    });
+    it("should prepend bucket in endpoint hostname when it does not already contain it and forcePathStyle = false", async () => {
+      hostname = "example-host.com";
+      const buffer = Buffer.from("");
+      const actionParams = { ...params, Body: buffer };
+      const upload = new Upload({
+        params: actionParams,
+        client: new S3({
+          forcePathStyle: false,
+        }),
+      });
+      const result = (await upload.done()) as CompleteMultipartUploadCommandOutput;
+      expect(result.Key).toEqual("example-key");
+      expect(result.Bucket).toEqual("example-bucket");
+      expect(result.Location).toEqual("https://example-bucket.example-host.com/example-key");
+    });
+  });
+
   it("should return a Location field formatted in path style when forcePathStyle is true", async () => {
     const buffer = Buffer.from("");
     const actionParams = { ...params, Body: buffer };

lib/lib-storage/src/Upload.ts

@@ -153,9 +153,17 @@ export class Upload extends EventEmitter {
       .join("/");
     const locationBucket = extendedEncodeURIComponent(this.params.Bucket!);
 
-    const Location: string = this.client.config.forcePathStyle
-      ? `${endpoint.protocol}//${endpoint.hostname}/${locationBucket}/${locationKey}`
-      : `${endpoint.protocol}//${locationBucket}.${endpoint.hostname}/${locationKey}`;
+    const Location: string = (() => {
+      const endpointHostnameIncludesBucket = endpoint.hostname.startsWith(`${locationBucket}.`);
+      const forcePathStyle = this.client.config.forcePathStyle;
+      if (forcePathStyle) {
+        return `${endpoint.protocol}//${endpoint.hostname}/${locationBucket}/${locationKey}`;
+      }
+      if (endpointHostnameIncludesBucket) {
+        return `${endpoint.protocol}//${endpoint.hostname}/${locationKey}`;
+      }
+      return `${endpoint.protocol}//${locationBucket}.${endpoint.hostname}/${locationKey}`;
+    })();
 
     this.singleUploadResult = {
       ...putResult,