From 150834fa046352324be811bcabb3e9cfea1d6a2c Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:52:22 -0400 Subject: [PATCH 1/3] fix: Location returned by createFile omits the bucket when a custom endpoint is configured --- index.js | 23 ++++++++++++++++- spec/test.spec.js | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 51864bb..4c1a330 100644 --- a/index.js +++ b/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') { diff --git a/spec/test.spec.js b/spec/test.spec.js index 2fed658..37adf0a 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -905,6 +905,72 @@ 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 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; From 666b9957b60167dbc085b662c3e8b3fa8261341f Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:31:56 -0400 Subject: [PATCH 2/3] Use the same location base for getFileLocation --- index.js | 6 +++++- spec/test.spec.js | 28 +++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 4c1a330..b87458f 100644 --- a/index.js +++ b/index.js @@ -289,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; diff --git a/spec/test.spec.js b/spec/test.spec.js index 37adf0a..bb476be 100644 --- a/spec/test.spec.js +++ b/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' ); }); @@ -959,6 +959,28 @@ describe('S3Adapter tests', () => { 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. From 1ff43cdea372860be33637ec5743fa0826d9e436 Mon Sep 17 00:00:00 2001 From: Adrian Curtin <48138055+AdrianCurtin@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:14:26 -0400 Subject: [PATCH 3/3] Resolve endpoint objects and providers instead of falling back to the AWS host --- index.js | 42 ++++++++++++++++++++++++++++++++++-------- spec/test.spec.js | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index b87458f..88e2382 100644 --- a/index.js +++ b/index.js @@ -180,22 +180,48 @@ class S3Adapter { return params; } + // The SDK accepts an endpoint as a url string, as an object carrying + // protocol, hostname, port and path, or as an EndpointV2 carrying a url. Each + // has to be reduced to a url here, because falling back to the AWS host would + // point a custom deployment at Amazon. + _endpointToUrl(endpoint) { + if (!endpoint) { + return `https://s3.${this._region}.amazonaws.com`; + } + if (typeof endpoint === 'string') { + return endpoint; + } + if (endpoint.url) { + return String(endpoint.url); + } + if (endpoint.hostname) { + const protocol = endpoint.protocol + ? endpoint.protocol.replace(/:?$/, ':') + : 'https:'; + const port = endpoint.port ? `:${endpoint.port}` : ''; + return `${protocol}//${endpoint.hostname}${port}${endpoint.path || ''}`; + } + return null; + } + // 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`; + async _buildLocationBase() { + let endpoint = this._endpoint; + if (typeof endpoint === 'function') { + // An endpoint provider, which the SDK resolves per request. + endpoint = await endpoint(); + } try { - const { protocol, host, pathname } = new URL(endpoint); + const { protocol, host, pathname } = new URL(this._endpointToUrl(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. + // An endpoint that names no host at all leaves nothing to build from. return `https://${this._bucket}.s3.${this._region}.amazonaws.com`; } } @@ -204,7 +230,7 @@ class S3Adapter { // 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._buildLocationBase(); + const endpoint = await this._buildLocationBase(); // Streaming upload path if (typeof data?.pipe === 'function') { @@ -293,7 +319,7 @@ class S3Adapter { // 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}`; + return `${await this._buildLocationBase()}/${fileKey}`; } const baseUrlFileKey = this._baseUrlDirect ? fileName : fileKey; diff --git a/spec/test.spec.js b/spec/test.spec.js index bb476be..8b6b7c1 100644 --- a/spec/test.spec.js +++ b/spec/test.spec.js @@ -981,10 +981,43 @@ describe('S3Adapter tests', () => { 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: '/' } }; + it('should resolve an endpoint given as an object', async () => { + // The SDK accepts this form as well as a url string. + options.s3overrides = { + endpoint: { hostname: 'example.com', protocol: 'https:', path: '/' }, + }; + + expect(await locationOf(options)).toBe('https://bucket-1.example.com/test/file.txt'); + }); + + it('should keep the port and path of an endpoint object', async () => { + options.s3overrides = { + endpoint: { hostname: 'example.com', protocol: 'http:', port: 9000, path: '/s3' }, + forcePathStyle: true, + }; + + expect(await locationOf(options)).toBe( + 'http://example.com:9000/s3/bucket-1/test/file.txt' + ); + }); + + it('should resolve an EndpointV2 carrying a url', async () => { + options.s3overrides = { endpoint: { url: new URL('https://example.com/s3') } }; + + expect(await locationOf(options)).toBe('https://bucket-1.example.com/s3/test/file.txt'); + }); + + it('should resolve an endpoint provider', async () => { + // Providers are resolved per request by the SDK, and may be async. + options.s3overrides = { + endpoint: async () => ({ hostname: 'example.com', protocol: 'https:' }), + }; + + expect(await locationOf(options)).toBe('https://bucket-1.example.com/test/file.txt'); + }); + + it('should fall back to the bucket host when the endpoint names no host', async () => { + options.s3overrides = { endpoint: {} }; const s3 = new S3Adapter(options); expect(await locationOf(options)).toBe(