fix: File location returned by createFile omits the bucket when a custom endpoint is configured

index.js

@@ -77,6 +77,7 @@ class S3Adapter {
     this._encryption = options.ServerSideEncryption;
     this._generateKey = options.generateKey;
     this._endpoint = options.s3overrides?.endpoint;
+    this._forcePathStyle = options.s3overrides?.forcePathStyle;
     // Optional FilesAdaptor method
     this.validateFilename = options.validateFilename;
 
@@ -179,11 +180,31 @@ class S3Adapter {
     return params;
   }
 
+  // The url prefix the S3 client addresses this bucket at, mirroring how the
+  // SDK resolves the bucket: as a leading path segment when forcePathStyle is
+  // set, otherwise as a host prefix. Without this the bucket is missing from
+  // the url whenever a custom endpoint does not already contain it.
+  _buildLocationBase() {
+    const endpoint = this._endpoint || `https://s3.${this._region}.amazonaws.com`;
+    try {
+      const { protocol, host, pathname } = new URL(endpoint);
+      const basePath = pathname.replace(/\/+$/, '');
+      return this._forcePathStyle
+        ? `${protocol}//${host}${basePath}/${this._bucket}`
+        : `${protocol}//${this._bucket}.${host}${basePath}`;
+    } catch {
+      // An endpoint that is not a url string, for example an object or a
+      // provider function, cannot be resolved here. Fall back to the bucket's
+      // default host.
+      return `https://${this._bucket}.s3.${this._region}.amazonaws.com`;
+    }
+  }
+
   // For a given config object, filename, and data, store a file in S3
   // Returns a promise containing the S3 object creation response
   async createFile(filename, data, contentType, options = {}) {
     const params = this._buildCreateFileParams(filename, data, contentType, options);
-    const endpoint = this._endpoint || `https://${this._bucket}.s3.${this._region}.amazonaws.com`;
+    const endpoint = this._buildLocationBase();
 
     // Streaming upload path
     if (typeof data?.pipe === 'function') {
@@ -268,7 +289,11 @@ class S3Adapter {
     }
 
     if (!this._baseUrl) {
-      return `https://${this._bucket}.s3.amazonaws.com/${fileKey}`;
+      // Same base as the location reported by createFile, so both name the
+      // object at the url the S3 client actually addresses it at. Previously
+      // this hardcoded the AWS host and ignored both the custom endpoint and
+      // the region.
+      return `${this._buildLocationBase()}/${fileKey}`;
     }
 
     const baseUrlFileKey = this._baseUrlDirect ? fileName : fileKey;

spec/test.spec.js

@@ -489,7 +489,7 @@ describe('S3Adapter tests', () => {
       delete options.baseUrl;
       const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket', options);
       await expectAsync(s3.getFileLocation(testConfig, 'test.png')).toBeResolvedTo(
-        'https://my-bucket.s3.amazonaws.com/foo/bar/test.png'
+        'https://my-bucket.s3.us-east-1.amazonaws.com/foo/bar/test.png'
       );
     });
   });
@@ -540,7 +540,7 @@ describe('S3Adapter tests', () => {
       delete options.baseUrl;
       const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket', options);
       await expectAsync(s3.getFileLocation(testConfig, 'test.png')).toBeResolvedTo(
-        'https://my-bucket.s3.amazonaws.com/foo/bar/test.png'
+        'https://my-bucket.s3.us-east-1.amazonaws.com/foo/bar/test.png'
       );
     });
   });
@@ -616,7 +616,7 @@ describe('S3Adapter tests', () => {
       delete options.baseUrl;
       const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket', options);
       await expectAsync(s3.getFileLocation(testConfig, 'test.png')).toBeResolvedTo(
-        'https://my-bucket.s3.amazonaws.com/foo/bar/test.png'
+        'https://my-bucket.s3.us-east-1.amazonaws.com/foo/bar/test.png'
       );
     });
 
@@ -905,6 +905,94 @@ describe('S3Adapter tests', () => {
       expect(s3ClientMock.send).toHaveBeenCalledWith(jasmine.any(PutObjectCommand));
     });
 
+    describe('location for custom endpoints', () => {
+      // Each expectation is the url the S3 client itself addresses the object
+      // at for the same options, so the reported location is where the file
+      // actually is.
+      const locationOf = async adapterOptions => {
+        const s3 = new S3Adapter(adapterOptions);
+        s3._s3Client = s3ClientMock;
+        const { Location } = await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
+        return Location;
+      };
+
+      it('should keep the bucket in the host without a custom endpoint', async () => {
+        const s3 = new S3Adapter(options);
+
+        expect(await locationOf(options)).toBe(
+          `https://bucket-1.s3.${s3._region}.amazonaws.com/test/file.txt`
+        );
+      });
+
+      it('should put the bucket in the path without a custom endpoint when path style', async () => {
+        options.s3overrides = { forcePathStyle: true };
+        const s3 = new S3Adapter(options);
+
+        expect(await locationOf(options)).toBe(
+          `https://s3.${s3._region}.amazonaws.com/bucket-1/test/file.txt`
+        );
+      });
+
+      it('should prefix a custom endpoint host with the bucket', async () => {
+        options.s3overrides = { endpoint: 'https://nyc3.digitaloceanspaces.com' };
+
+        expect(await locationOf(options)).toBe(
+          'https://bucket-1.nyc3.digitaloceanspaces.com/test/file.txt'
+        );
+      });
+
+      it('should put the bucket in the path of a custom endpoint when path style', async () => {
+        options.s3overrides = { endpoint: 'http://localhost:9000', forcePathStyle: true };
+
+        expect(await locationOf(options)).toBe('http://localhost:9000/bucket-1/test/file.txt');
+      });
+
+      it('should preserve a base path on the custom endpoint', async () => {
+        options.s3overrides = { endpoint: 'https://example.com/s3' };
+
+        expect(await locationOf(options)).toBe('https://bucket-1.example.com/s3/test/file.txt');
+      });
+
+      it('should not double the separator when the endpoint has a trailing slash', async () => {
+        options.s3overrides = { endpoint: 'https://example.com/s3/' };
+
+        expect(await locationOf(options)).toBe('https://bucket-1.example.com/s3/test/file.txt');
+      });
+
+      it('should use the same base for the url getFileLocation returns', async () => {
+        // Otherwise createFile reports one url for the object and
+        // getFileLocation reports another, and the second is the one a client
+        // is handed.
+        const s3 = new S3Adapter({
+          bucket: 'bucket-1',
+          bucketPrefix: 'test/',
+          directAccess: true,
+          s3overrides: { endpoint: 'http://localhost:9000', forcePathStyle: true },
+        });
+        s3._s3Client = s3ClientMock;
+
+        const { Location } = await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
+        const url = await s3.getFileLocation(
+          { mount: 'http://my.server.com/parse', applicationId: 'xxxx' },
+          'file.txt'
+        );
+
+        expect(Location).toBe('http://localhost:9000/bucket-1/test/file.txt');
+        expect(url).toBe(Location);
+      });
+
+      it('should fall back to the bucket host when the endpoint is not a url', async () => {
+        // The SDK also accepts an endpoint object or provider, which cannot be
+        // resolved to a url here.
+        options.s3overrides = { endpoint: { hostname: 'example.com', protocol: 'https:', path: '/' } };
+        const s3 = new S3Adapter(options);
+
+        expect(await locationOf(options)).toBe(
+          `https://bucket-1.s3.${s3._region}.amazonaws.com/test/file.txt`
+        );
+      });
+    });
+
     it('should save a stream with metadata added', async () => {
       const rewiredModule = rewire('../index');
       let uploadParams;