diff --git a/lib/remote-storage.js b/lib/remote-storage.js index 0481539..d69bbba 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -17,6 +17,9 @@ const fs = require('fs-extra') const joi = require('joi') const klaw = require('klaw') const http = require('http') +// Proxy support for AWS SDK v3 (inspired by PR #224 by pat-lego, with compatibility fixes) +const { NodeHttpHandler } = require('@smithy/node-http-handler') +const { ProxyAgent } = require('proxy-agent') const { codes, logAndThrow } = require('./StorageError') const fileExtensionPattern = /\*\.[0-9a-zA-Z]+$/ @@ -71,8 +74,20 @@ module.exports = class RemoteStorage { } this.bucket = creds.params.Bucket + // Configure proxy support for AWS SDK v3 + // ProxyAgent automatically handles proxy environment variables via proxy-from-env + const agent = new ProxyAgent() + const s3Config = { + credentials, + region, + requestHandler: new NodeHttpHandler({ + httpAgent: agent, + httpsAgent: agent + }) + } + // https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-s3/classes/s3.html#constructor - this.s3 = new S3({ credentials, region }) + this.s3 = new S3(s3Config) } /** diff --git a/package.json b/package.json index baa5944..95e93cc 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "@adobe/aio-lib-core-logging": "^3", "@adobe/aio-lib-core-tvm": "^4", "@aws-sdk/client-s3": "^3.624.0", + "@smithy/node-http-handler": "^3.0.0", "core-js": "^3.25.1", "fs-extra": "^11", "joi": "^17.2.1", @@ -40,6 +41,7 @@ "lodash.clonedeep": "^4.5.0", "mime-types": "^2.1.24", "parcel": "^2.7.0", + "proxy-agent": "^6.3.0", "regenerator-runtime": "^0.13.7" }, "devDependencies": { diff --git a/test/lib/remote-storage.test.js b/test/lib/remote-storage.test.js index a7ece8c..abae930 100644 --- a/test/lib/remote-storage.test.js +++ b/test/lib/remote-storage.test.js @@ -49,7 +49,8 @@ describe('RemoteStorage', () => { sessionToken: global.fakeTVMResponse.sessionToken, expiration: new Date(global.fakeTVMResponse.expiration) }, - region: 'us-east-1' + region: 'us-east-1', + requestHandler: expect.any(Object) }) rs.bucket = global.fakeTVMResponse.Bucket }) @@ -59,13 +60,122 @@ describe('RemoteStorage', () => { expect(S3).toHaveBeenCalledWith({ credentials: { accessKeyId: global.fakeTVMResponse.accessKeyId, - secretAccessKey: global.fakeTVMResponse.secretAccessKey + secretAccessKey: global.fakeTVMResponse.secretAccessKey, + sessionToken: undefined, + expiration: undefined }, - region: 'us-east-1' + region: 'us-east-1', + requestHandler: expect.any(Object) }) rs.bucket = global.fakeTVMResponse.Bucket }) + describe('Proxy configuration', () => { + const originalEnv = process.env + + beforeEach(() => { + // Clear environment variables before each test + delete process.env.https_proxy + delete process.env.HTTPS_PROXY + delete process.env.http_proxy + delete process.env.HTTP_PROXY + }) + + afterAll(() => { + // Restore original environment + process.env = originalEnv + }) + + test('Constructor uses HTTPS_PROXY when set (uppercase)', async () => { + process.env.HTTPS_PROXY = 'http://proxy.example.com:8080' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor uses https_proxy when set (lowercase)', async () => { + process.env.https_proxy = 'http://proxy.example.com:3128' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor uses HTTP_PROXY when HTTPS_PROXY not set', async () => { + process.env.HTTP_PROXY = 'http://proxy.example.com:8080' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor uses http_proxy when other proxy vars not set', async () => { + process.env.http_proxy = 'http://proxy.example.com:3128' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor prioritizes HTTPS_PROXY over HTTP_PROXY', async () => { + process.env.HTTPS_PROXY = 'http://https-proxy.example.com:8080' + process.env.HTTP_PROXY = 'http://http-proxy.example.com:8080' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor prioritizes https_proxy over HTTP_PROXY', async () => { + process.env.https_proxy = 'http://https-proxy.example.com:3128' + process.env.HTTP_PROXY = 'http://http-proxy.example.com:8080' + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith(expect.objectContaining({ + requestHandler: expect.any(Object), + credentials: expect.any(Object), + region: 'us-east-1' + })) + }) + + test('Constructor always includes requestHandler with ProxyAgent', async () => { + // eslint-disable-next-line no-new + new RemoteStorage(global.fakeTVMResponse) + + expect(S3).toHaveBeenCalledWith({ + credentials: expect.any(Object), + region: 'us-east-1', + requestHandler: expect.any(Object) + }) + + // ProxyAgent handles proxy detection automatically via proxy-from-env + const s3CallArgs = S3.mock.calls[S3.mock.calls.length - 1][0] + expect(s3CallArgs).toHaveProperty('requestHandler') + }) + }) + test('folderExists missing prefix', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) await expect(rs.folderExists()).rejects.toEqual(expect.objectContaining({ message: 'prefix must be a valid string' }))