From a677be0be1d93095a576ec273c8b55956fe836f3 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Wed, 14 Oct 2026 15:53:19 +0000 Subject: [PATCH 1/5] refactor: replace customEndpoint with getEndpoint method for dynamic API endpoint resolution --- handwritten/storage/src/file.ts | 30 +++--- .../storage/src/nodejs-common/service.ts | 17 +-- handwritten/storage/src/nodejs-common/util.ts | 27 +---- handwritten/storage/src/resumable-upload.ts | 12 +-- handwritten/storage/src/signer.ts | 4 +- handwritten/storage/src/storage.ts | 31 ++++-- handwritten/storage/src/transfer-manager.ts | 2 +- handwritten/storage/test/file.ts | 8 +- handwritten/storage/test/index.ts | 72 ++++--------- .../storage/test/nodejs-common/util.ts | 101 +----------------- handwritten/storage/test/resumable-upload.ts | 98 ----------------- 11 files changed, 71 insertions(+), 331 deletions(-) diff --git a/handwritten/storage/src/file.ts b/handwritten/storage/src/file.ts index 977e3df4a3c..2f50800d128 100644 --- a/handwritten/storage/src/file.ts +++ b/handwritten/storage/src/file.ts @@ -1959,7 +1959,7 @@ class File extends ServiceObject { resumableUpload.createURI( { authClient: this.storage.authClient, - apiEndpoint: this.storage.apiEndpoint, + apiEndpoint: this.storage['apiEndpoint'], bucket: this.bucket.name, customRequestOptions: this.getRequestInterceptors().reduce( (reqOpts, interceptorFn) => interceptorFn(reqOpts), @@ -1979,7 +1979,6 @@ class File extends ServiceObject { retryOptions: retryOptions, params: options?.preconditionOpts || this.instancePreconditionOpts, universeDomain: this.bucket.storage.universeDomain, - useAuthWithCustomEndpoint: this.storage.useAuthWithCustomEndpoint, [GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY], }, callback! @@ -3147,11 +3146,18 @@ class File extends ServiceObject { let url: string; const EMULATOR_HOST = process.env.STORAGE_EMULATOR_HOST; - - if (this.storage.customEndpoint && typeof EMULATOR_HOST === 'string') { - url = `${this.storage.apiEndpoint}/${this.bucket.name}`; - } else if (this.storage.customEndpoint) { - url = this.storage.apiEndpoint; + const endpoint = + typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : this.storage['apiEndpoint']; + const defaultEndpoint = `https://storage.${ + universe || 'googleapis.com' + }`; + + if (endpoint !== defaultEndpoint && typeof EMULATOR_HOST === 'string') { + url = `${endpoint}/${this.bucket.name}`; + } else if (endpoint !== defaultEndpoint) { + url = endpoint; } else if (options.virtualHostedStyle) { url = `https://${this.bucket.name}.storage.${universe}/`; } else if (options.bucketBoundHostname) { @@ -3484,7 +3490,7 @@ class File extends ServiceObject { const allInterceptors = storageInterceptors.concat(fileInterceptors); const headers = allInterceptors.reduce((acc, curInterceptor) => { const currentHeaders = curInterceptor.request({ - uri: `${this.storage.apiEndpoint}/${ + uri: `${this.storage['apiEndpoint']}/${ this.bucket.name }/${encodeURIComponent(this.name)}`, }); @@ -3496,7 +3502,7 @@ class File extends ServiceObject { util.makeRequest( { method: 'GET', - uri: `${this.storage.apiEndpoint}/${ + uri: `${this.storage['apiEndpoint']}/${ this.bucket.name }/${encodeURIComponent(this.name)}`, headers, @@ -3687,7 +3693,7 @@ class File extends ServiceObject { * ``` */ publicUrl(): string { - return `${this.storage.apiEndpoint}/${ + return `${this.storage['apiEndpoint']}/${ this.bucket.name }/${encodeURIComponent(this.name)}`; } @@ -4636,7 +4642,7 @@ class File extends ServiceObject { } const cfg = { authClient: this.storage.authClient, - apiEndpoint: this.storage.apiEndpoint, + apiEndpoint: this.storage['apiEndpoint'], bucket: this.bucket.name, customRequestOptions: this.getRequestInterceptors().reduce( (reqOpts, interceptorFn) => interceptorFn(reqOpts), @@ -4708,7 +4714,7 @@ class File extends ServiceObject { ): void { options.metadata ??= {}; - const apiEndpoint = this.storage.apiEndpoint; + const apiEndpoint = this.storage['apiEndpoint']; const bucketName = this.bucket.name; const uri = `${apiEndpoint}/upload/storage/v1/b/${bucketName}/o`; diff --git a/handwritten/storage/src/nodejs-common/service.ts b/handwritten/storage/src/nodejs-common/service.ts index 33bc7a8ec89..c81c2ac3d12 100644 --- a/handwritten/storage/src/nodejs-common/service.ts +++ b/handwritten/storage/src/nodejs-common/service.ts @@ -62,16 +62,6 @@ export interface ServiceConfig { * Reuse an existing `AuthClient` or `GoogleAuth` client instead of creating a new one. */ authClient?: AuthClient | GoogleAuth; - - /** - * Set to true if the endpoint is a custom URL - */ - customEndpoint?: boolean; - - /** - * Controls whether or not to use authentication when using a custom endpoint. - */ - useAuthWithCustomEndpoint?: boolean; } export interface ServiceOptions extends Omit { @@ -81,7 +71,6 @@ export interface ServiceOptions extends Omit { token?: string; timeout?: number; // http.request.options.timeout userAgent?: string; - useAuthWithCustomEndpoint?: boolean; } export class Service { @@ -94,11 +83,9 @@ export class Service { providedUserAgent?: string; makeAuthenticatedRequest: MakeAuthenticatedRequest; authClient: GoogleAuth; - apiEndpoint: string; + protected apiEndpoint: string; timeout?: number; universeDomain: string; - customEndpoint: boolean; - useAuthWithCustomEndpoint?: boolean; /** * Service is a base class, meant to be inherited from by a "service," like @@ -128,8 +115,6 @@ export class Service { this.projectIdRequired = config.projectIdRequired !== false; this.providedUserAgent = options.userAgent; this.universeDomain = options.universeDomain || DEFAULT_UNIVERSE; - this.customEndpoint = config.customEndpoint || false; - this.useAuthWithCustomEndpoint = config.useAuthWithCustomEndpoint; this.makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory({ ...config, diff --git a/handwritten/storage/src/nodejs-common/util.ts b/handwritten/storage/src/nodejs-common/util.ts index d79c5ca5ce1..981259fa801 100644 --- a/handwritten/storage/src/nodejs-common/util.ts +++ b/handwritten/storage/src/nodejs-common/util.ts @@ -168,16 +168,6 @@ export interface MakeAuthenticatedRequestFactoryConfig extends Omit< */ autoRetry?: boolean; - /** - * If true, just return the provided request options. Default: false. - */ - customEndpoint?: boolean; - - /** - * If true, will authenticate when using a custom endpoint. Default: false. - */ - useAuthWithCustomEndpoint?: boolean; - /** * Account email address, required for PEM/P12 usage. */ @@ -628,8 +618,6 @@ export class Util { * errors. We will exponentially backoff subsequent requests by default. * (default: true) * @param {object=} config.credentials - Credentials object. - * @param {boolean=} config.customEndpoint - If true, just return the provided request options. Default: false. - * @param {boolean=} config.useAuthWithCustomEndpoint - If true, will authenticate when using a custom endpoint. Default: false. * @param {string=} config.email - Account email address, required for PEM/P12 usage. * @param {number=} config.maxRetries - Maximum number of automatic retries attempted before returning the error. (default: 3) * @param {string=} config.keyFile - Path to a .json, .pem, or .p12 keyfile. @@ -805,22 +793,9 @@ export class Util { return setProjectId(); }; - const authorizeRequest = async () => { - if ( - reqConfig.customEndpoint && - !reqConfig.useAuthWithCustomEndpoint - ) { - // Using a custom API override. Do not use `google-auth-library` for - // authentication. (ex: connecting to a local Datastore server) - return reqOpts; - } else { - return authClient.authorizeRequest(reqOpts); - } - }; - const [_projectId, authorizedReqOpts] = await Promise.all([ getProjectId(), - authorizeRequest(), + authClient.authorizeRequest(reqOpts), ]); if (_projectId) { diff --git a/handwritten/storage/src/resumable-upload.ts b/handwritten/storage/src/resumable-upload.ts index d9e97a90a9a..c64070becf8 100644 --- a/handwritten/storage/src/resumable-upload.ts +++ b/handwritten/storage/src/resumable-upload.ts @@ -245,11 +245,6 @@ export interface UploadConfig extends Pick { */ retryOptions: RetryOptions; - /** - * Controls whether or not to use authentication when using a custom endpoint. - */ - useAuthWithCustomEndpoint?: boolean; - [GCCL_GCS_CMD_KEY]?: string; } @@ -406,12 +401,7 @@ export class Upload extends Writable { !isSubDomainOfUniverse && !isSubDomainOfDefaultUniverse ) { - // Check if we should use auth with custom endpoint - if (cfg.useAuthWithCustomEndpoint !== true) { - // Only bypass auth if explicitly not requested - this.authClient = gaxios; - } - // Otherwise keep the authenticated client + this.authClient = gaxios; } } diff --git a/handwritten/storage/src/signer.ts b/handwritten/storage/src/signer.ts index f39a2bf30ab..9b9d0cb07ce 100644 --- a/handwritten/storage/src/signer.ts +++ b/handwritten/storage/src/signer.ts @@ -208,7 +208,7 @@ export class URLSigner { query = Object.assign(query, cfg.queryParams); const signedUrl = new url.URL( - cfg.host?.toString() || config.cname || this.storage.apiEndpoint + cfg.host?.toString() || config.cname || this.storage['apiEndpoint'] ); signedUrl.pathname = this.getResourcePath( @@ -285,7 +285,7 @@ export class URLSigner { const extensionHeaders = Object.assign({}, config.extensionHeaders); const fqdn = new url.URL( - config.host?.toString() || config.cname || this.storage.apiEndpoint + config.host?.toString() || config.cname || this.storage['apiEndpoint'] ); extensionHeaders.host = fqdn.hostname; if (config.contentMd5) { diff --git a/handwritten/storage/src/storage.ts b/handwritten/storage/src/storage.ts index ab036e15b0e..a8575a29242 100644 --- a/handwritten/storage/src/storage.ts +++ b/handwritten/storage/src/storage.ts @@ -29,7 +29,7 @@ import { CRC32CValidatorGenerator, CRC32C_DEFAULT_VALIDATOR_GENERATOR, } from './crc32c.js'; -import {DEFAULT_UNIVERSE} from 'google-auth-library'; +import {DEFAULT_UNIVERSE, PassThroughClient} from 'google-auth-library'; export interface GetServiceAccountOptions { userProject?: string; @@ -541,6 +541,8 @@ export class Storage extends Service { } retryOptions: RetryOptions; + protected apiEndpoint: string; + #hasCustomEndpoint = false; /** * @callback Crc32cGeneratorToStringCallback @@ -653,7 +655,6 @@ export class Storage extends Service { * @property {number} [timeout] The amount of time in milliseconds to wait per http request before timing out. * @property {object[]} [interceptors_] Array of custom request interceptors to be returned in the order they were assigned. * @property {string} [apiEndpoint = storage.google.com] The API endpoint of the service used to make requests. - * @property {boolean} [useAuthWithCustomEndpoint = false] Controls whether or not to use authentication when using a custom endpoint. * @property {Crc32cGeneratorCallback} [callback] A function that generates a CRC32C Validator. Defaults to {@link CRC32C} */ /** @@ -726,18 +727,23 @@ export class Storage extends Service { const universe = options.universeDomain || DEFAULT_UNIVERSE; let apiEndpoint = `https://storage.${universe}`; - let customEndpoint = false; + let hasCustomEndpoint = false; // Note: EMULATOR_HOST is an experimental configuration variable. Use apiEndpoint instead. const EMULATOR_HOST = process.env.STORAGE_EMULATOR_HOST; if (typeof EMULATOR_HOST === 'string') { apiEndpoint = Storage.sanitizeEndpoint(EMULATOR_HOST); - customEndpoint = true; + hasCustomEndpoint = true; + if (!options.authClient) { + options = Object.assign({}, options, { + authClient: new PassThroughClient(), + }); + } } if (options.apiEndpoint && options.apiEndpoint !== apiEndpoint) { apiEndpoint = Storage.sanitizeEndpoint(options.apiEndpoint); - customEndpoint = true; + hasCustomEndpoint = true; } options = Object.assign({}, options, {apiEndpoint}); @@ -773,8 +779,6 @@ export class Storage extends Service { : IDEMPOTENCY_STRATEGY_DEFAULT, }, baseUrl, - customEndpoint, - useAuthWithCustomEndpoint: options?.useAuthWithCustomEndpoint, projectIdRequired: false, scopes: [ 'https://www.googleapis.com/auth/iam', @@ -786,6 +790,9 @@ export class Storage extends Service { super(config, options); + this.apiEndpoint = apiEndpoint; + this.#hasCustomEndpoint = hasCustomEndpoint; + /** * Reference to {@link Storage.acl}. * @@ -802,6 +809,14 @@ export class Storage extends Service { this.getHmacKeysStream = paginator.streamify('getHmacKeys'); } + async getEndpoint(): Promise { + if (!this.#hasCustomEndpoint) { + const universe = await this.authClient.getUniverseDomain(); + this.apiEndpoint = Storage.sanitizeEndpoint(`storage.${universe}`); + } + return this.apiEndpoint; + } + private static sanitizeEndpoint(url: string) { if (!PROTOCOL_REGEX.test(url)) { url = `https://${url}`; @@ -1630,5 +1645,5 @@ paginator.extend(Storage, ['getBuckets', 'getHmacKeys']); * that a callback is omitted. */ promisifyAll(Storage, { - exclude: ['bucket', 'channel', 'hmacKey'], + exclude: ['bucket', 'channel', 'hmacKey', 'getEndpoint'], }); diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts index 9ff9368ce91..ad4d935af3b 100644 --- a/handwritten/storage/src/transfer-manager.ts +++ b/handwritten/storage/src/transfer-manager.ts @@ -214,7 +214,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { this.bucket = bucket; this.fileName = fileName; this.baseUrl = `https://${bucket.name}.${ - new URL(this.bucket.storage.apiEndpoint).hostname + new URL(this.bucket.storage['apiEndpoint']).hostname }/${fileName}`; this.xmlBuilder = new XMLBuilder({arrayNodeName: 'Part'}); this.xmlParser = new XMLParser(); diff --git a/handwritten/storage/test/file.ts b/handwritten/storage/test/file.ts index 938e8e317bb..21f14c238a5 100644 --- a/handwritten/storage/test/file.ts +++ b/handwritten/storage/test/file.ts @@ -237,6 +237,9 @@ describe('File', () => { createBucket: util.noop, request: util.noop, apiEndpoint: 'https://storage.googleapis.com', + async getEndpoint() { + return this.apiEndpoint; + }, // eslint-disable-next-line @typescript-eslint/no-explicit-any makeAuthenticatedRequest(req: {}, callback: any) { if (callback) { @@ -257,7 +260,6 @@ describe('File', () => { }, idempotencyStrategy: IdempotencyStrategy.RetryConditional, }, - customEndpoint: false, }; BUCKET = new Bucket(STORAGE, 'bucket-name'); @@ -3916,7 +3918,6 @@ describe('File', () => { const customEndpoint = 'https://my-custom-endpoint.com'; STORAGE.apiEndpoint = customEndpoint; - STORAGE.customEndpoint = true; CONFIG.virtualHostedStyle = true; CONFIG.bucketBoundHostname = 'http://domain.tld'; @@ -3934,18 +3935,15 @@ describe('File', () => { it('should append bucket name to the URL when using the emulator', done => { const emulatorHost = 'http://127.0.0.1:9199'; const originalApiEndpoint = STORAGE.apiEndpoint; - const originalCustomEndpoint = STORAGE.customEndpoint; const originalEnvHost = process.env.STORAGE_EMULATOR_HOST; process.env.STORAGE_EMULATOR_HOST = emulatorHost; STORAGE.apiEndpoint = emulatorHost; - STORAGE.customEndpoint = true; file.generateSignedPostPolicyV4( CONFIG, (err: Error, res: SignedPostPolicyV4Output) => { STORAGE.apiEndpoint = originalApiEndpoint; - STORAGE.customEndpoint = originalCustomEndpoint; if (originalEnvHost) { process.env.STORAGE_EMULATOR_HOST = originalEnvHost; } else { diff --git a/handwritten/storage/test/index.ts b/handwritten/storage/test/index.ts index f615cbeb1ff..fe35d4bd10e 100644 --- a/handwritten/storage/test/index.ts +++ b/handwritten/storage/test/index.ts @@ -36,6 +36,7 @@ import { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore import {getPackageJSON} from '../src/package-json-helper.cjs'; +import {GoogleAuth, PassThroughClient} from 'google-auth-library'; // eslint-disable-next-line @typescript-eslint/no-var-requires const hmacKeyModule = require('../src/hmacKey'); @@ -83,7 +84,12 @@ const fakePromisify = { } promisified = true; - assert.deepStrictEqual(options.exclude, ['bucket', 'channel', 'hmacKey']); + assert.deepStrictEqual(options.exclude, [ + 'bucket', + 'channel', + 'hmacKey', + 'getEndpoint', + ]); }, }; @@ -174,44 +180,6 @@ describe('Storage', () => { assert.strictEqual(calledWith.apiEndpoint, `${apiEndpoint}`); }); - it('should not set `customEndpoint` if `apiEndpoint` matches default', () => { - const apiEndpoint = 'https://storage.googleapis.com'; - const storage = new Storage({ - apiEndpoint, - }); - - const calledWith = storage.calledWith_[0]; - assert.strictEqual(calledWith.apiEndpoint, apiEndpoint); - assert.strictEqual(calledWith.customEndpoint, false); - }); - - it('should not set `customEndpoint` if `apiEndpoint` matches default (w/ universe domain)', () => { - const universeDomain = 'my.universe'; - const apiEndpoint = `https://storage.${universeDomain}`; - const storage = new Storage({ - apiEndpoint, - universeDomain, - }); - - const calledWith = storage.calledWith_[0]; - assert.strictEqual(calledWith.apiEndpoint, apiEndpoint); - assert.strictEqual(calledWith.customEndpoint, false); - }); - - it('should propagate the useAuthWithCustomEndpoint option', () => { - const useAuthWithCustomEndpoint = true; - const apiEndpoint = 'https://some.fake.endpoint'; - const storage = new Storage({ - projectId: PROJECT_ID, - useAuthWithCustomEndpoint, - apiEndpoint, - }); - const calledWith = storage.calledWith_[0]; - assert.strictEqual(calledWith.apiEndpoint, 'https://some.fake.endpoint'); - assert.strictEqual(calledWith.customEndpoint, true); - assert.strictEqual(calledWith.useAuthWithCustomEndpoint, true); - }); - it('should propagate autoRetry in retryOptions', () => { const autoRetry = false; const storage = new Storage({ @@ -416,16 +384,6 @@ describe('Storage', () => { assert.strictEqual(calledWith.retryOptions.retryableErrorFn(error), true); }); - it('should set customEndpoint to true when using apiEndpoint', () => { - const storage = new Storage({ - projectId: PROJECT_ID, - apiEndpoint: 'https://apiendpoint', - }); - - const calledWith = storage.calledWith_[0]; - assert.strictEqual(calledWith.customEndpoint, true); - }); - it('should prepend apiEndpoint with default protocol', () => { const protocollessApiEndpoint = 'some.fake.endpoint'; const storage = new Storage({ @@ -468,12 +426,19 @@ describe('Storage', () => { ); }); - it('should accept and use a `universeDomain`', () => { + it('should accept and use a `universeDomain`', async () => { const universeDomain = 'my-universe.com'; + const stub = sinon + .stub(GoogleAuth.prototype, 'getUniverseDomain') + .resolves(universeDomain); const storage = new Storage({universeDomain}); - assert.equal(storage.apiEndpoint, `https://storage.${universeDomain}`); + assert.equal( + await storage.getEndpoint(), + `https://storage.${universeDomain}` + ); + stub.restore(); }); describe('STORAGE_EMULATOR_HOST', () => { @@ -527,12 +492,13 @@ describe('Storage', () => { ); }); - it('should set customEndpoint to true', () => { + it('should use PassThroughClient when authClient is not provided', () => { const storage = new Storage({ projectId: PROJECT_ID, }); - assert.strictEqual(storage.customEndpoint, true); + const calledWith = storage.calledWith_[1]; + assert(calledWith.authClient instanceof PassThroughClient); }); }); }); diff --git a/handwritten/storage/test/nodejs-common/util.ts b/handwritten/storage/test/nodejs-common/util.ts index d7ebc515c3f..869b82285db 100644 --- a/handwritten/storage/test/nodejs-common/util.ts +++ b/handwritten/storage/test/nodejs-common/util.ts @@ -735,6 +735,7 @@ describe('common/util', () => { const authClient = { getCredentials() {}, getProjectId: () => Promise.resolve(AUTH_CLIENT_PROJECT_ID), + authorizeRequest: async (rOpts: {}) => rOpts, // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; @@ -826,101 +827,6 @@ describe('common/util', () => { assert.strictEqual(mar.authClient, authClient); }); - describe('customEndpoint (no authentication attempted)', () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let makeAuthenticatedRequest: any; - const config = {customEndpoint: true}; - - beforeEach(() => { - sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient); - makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(config); - }); - - it('should decorate the request', done => { - const decoratedRequest = {}; - stub('decorateRequest', reqOpts_ => { - assert.strictEqual(reqOpts_, fakeReqOpts); - return decoratedRequest; - }); - - makeAuthenticatedRequest(fakeReqOpts, { - onAuthenticated( - err: Error, - authenticatedReqOpts: DecorateRequestOptions - ) { - assert.ifError(err); - assert.strictEqual(authenticatedReqOpts, decoratedRequest); - done(); - }, - }); - }); - - it('should return an error while decorating', done => { - const error = new Error('Error.'); - stub('decorateRequest', () => { - throw error; - }); - makeAuthenticatedRequest(fakeReqOpts, { - onAuthenticated(err: Error) { - assert.strictEqual(err, error); - done(); - }, - }); - }); - - it('should pass options back to callback', done => { - const reqOpts = {a: 'b', c: 'd'}; - makeAuthenticatedRequest(reqOpts, { - onAuthenticated( - err: Error, - authenticatedReqOpts: DecorateRequestOptions - ) { - assert.ifError(err); - assert.deepStrictEqual(reqOpts, authenticatedReqOpts); - done(); - }, - }); - }); - - it('should not authenticate requests with a custom API', done => { - const reqOpts = {a: 'b', c: 'd'}; - - stub('makeRequest', rOpts => { - assert.deepStrictEqual(rOpts, reqOpts); - done(); - }); - - makeAuthenticatedRequest(reqOpts, assert.ifError); - }); - }); - - describe('customEndpoint (authentication attempted)', () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let makeAuthenticatedRequest: any; - const config = {customEndpoint: true, useAuthWithCustomEndpoint: true}; - - beforeEach(() => { - sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient); - makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(config); - }); - - it('should authenticate requests with a custom API', done => { - const reqOpts = {a: 'b', c: 'd'}; - - stub('makeRequest', rOpts => { - assert.deepStrictEqual(rOpts, reqOpts); - done(); - }); - - authClient.authorizeRequest = async (opts: {}) => { - assert.strictEqual(opts, reqOpts); - done(); - }; - - makeAuthenticatedRequest(reqOpts, assert.ifError); - }); - }); - describe('authentication', () => { it('should pass correct args to authorizeRequest', done => { const fake = { @@ -967,7 +873,7 @@ describe('common/util', () => { }); const makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory( - {customEndpoint: true} + {} ); makeAuthenticatedRequest(reqOpts, { @@ -979,7 +885,6 @@ describe('common/util', () => { sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient); const config = { - customEndpoint: true, projectId: 'user-provided-project-id', }; @@ -1002,7 +907,6 @@ describe('common/util', () => { sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient); const config = { - customEndpoint: true, projectIdRequired: false, }; @@ -1028,7 +932,6 @@ describe('common/util', () => { sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient); const config = { - customEndpoint: true, projectIdRequired: false, }; diff --git a/handwritten/storage/test/resumable-upload.ts b/handwritten/storage/test/resumable-upload.ts index 15404d4c79d..84dc1b9a2d4 100644 --- a/handwritten/storage/test/resumable-upload.ts +++ b/handwritten/storage/test/resumable-upload.ts @@ -2703,104 +2703,6 @@ describe('resumable-upload', () => { assert.ok(res.headers); }); - it('should use authentication with custom endpoint when useAuthWithCustomEndpoint is true', async () => { - up = upload({ - bucket: BUCKET, - file: FILE, - customRequestOptions: CUSTOM_REQUEST_OPTIONS, - generation: GENERATION, - metadata: METADATA, - origin: ORIGIN, - params: PARAMS, - predefinedAcl: PREDEFINED_ACL, - userProject: USER_PROJECT, - authConfig: {keyFile}, - apiEndpoint: 'https://custom-proxy.example.com', - useAuthWithCustomEndpoint: true, - retryOptions: RETRY_OPTIONS, - }); - - // Mock the authorization request - mockAuthorizeRequest(); - - // Mock the actual request with auth header expectation - const scopes = [ - nock(REQ_OPTS.url!) - .matchHeader('authorization', /Bearer .+/) - .get(queryPath) - .reply(200, undefined, {}), - ]; - - const res = await up.makeRequest(REQ_OPTS); - scopes.forEach(x => x.done()); - const expectedUrl = String(res.config.url).includes('fake.local/?') - ? REQ_OPTS.url + queryPath - : REQ_OPTS.url + queryPath.slice(1); - assert.strictEqual(String(res.config.url), expectedUrl); - // Headers should include authorization - assert.ok(getHeader(res.config.headers, 'Authorization')); - }); - - it('should bypass authentication with custom endpoint when useAuthWithCustomEndpoint is false', async () => { - up = upload({ - bucket: BUCKET, - file: FILE, - customRequestOptions: CUSTOM_REQUEST_OPTIONS, - generation: GENERATION, - metadata: METADATA, - origin: ORIGIN, - params: PARAMS, - predefinedAcl: PREDEFINED_ACL, - userProject: USER_PROJECT, - authConfig: {keyFile}, - apiEndpoint: 'https://storage-emulator.local', - useAuthWithCustomEndpoint: false, - retryOptions: RETRY_OPTIONS, - }); - - const scopes = [ - nock(REQ_OPTS.url!).get(queryPath).reply(200, undefined, {}), - ]; - const res = await up.makeRequest(REQ_OPTS); - scopes.forEach(x => x.done()); - assert.strictEqual( - res.config.url!.toString(), - `${REQ_OPTS.url}${queryPath}` - ); - // When auth is bypassed, no auth headers should be present - assert.ok(res.headers); - }); - - it('should bypass authentication with custom endpoint when useAuthWithCustomEndpoint is undefined (backward compatibility)', async () => { - up = upload({ - bucket: BUCKET, - file: FILE, - customRequestOptions: CUSTOM_REQUEST_OPTIONS, - generation: GENERATION, - metadata: METADATA, - origin: ORIGIN, - params: PARAMS, - predefinedAcl: PREDEFINED_ACL, - userProject: USER_PROJECT, - authConfig: {keyFile}, - apiEndpoint: 'https://storage-emulator.local', - // useAuthWithCustomEndpoint is intentionally not set - retryOptions: RETRY_OPTIONS, - }); - - const scopes = [ - nock(REQ_OPTS.url!).get(queryPath).reply(200, undefined, {}), - ]; - const res = await up.makeRequest(REQ_OPTS); - scopes.forEach(x => x.done()); - assert.strictEqual( - res.config.url!.toString(), - `${REQ_OPTS.url}${queryPath}` - ); - // When auth is bypassed (backward compatibility), no auth headers should be present - assert.ok(res.headers); - }); - it('should combine customRequestOptions', done => { const up = upload({ bucket: BUCKET, From 50cc2e6e9f0bc144154b1d0b057c0077400be730 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 15 Oct 2026 02:56:29 +0000 Subject: [PATCH 2/5] fix: avoid using PassThroughClient when explicit credentials or key files are provided --- handwritten/storage/src/storage.ts | 7 ++++++- handwritten/storage/test/index.ts | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/handwritten/storage/src/storage.ts b/handwritten/storage/src/storage.ts index a8575a29242..493b99ff9bc 100644 --- a/handwritten/storage/src/storage.ts +++ b/handwritten/storage/src/storage.ts @@ -734,7 +734,12 @@ export class Storage extends Service { if (typeof EMULATOR_HOST === 'string') { apiEndpoint = Storage.sanitizeEndpoint(EMULATOR_HOST); hasCustomEndpoint = true; - if (!options.authClient) { + if ( + !options.authClient && + !options.keyFilename && + !options.keyFile && + !options.credentials + ) { options = Object.assign({}, options, { authClient: new PassThroughClient(), }); diff --git a/handwritten/storage/test/index.ts b/handwritten/storage/test/index.ts index fe35d4bd10e..d7449dd0e8a 100644 --- a/handwritten/storage/test/index.ts +++ b/handwritten/storage/test/index.ts @@ -500,6 +500,26 @@ describe('Storage', () => { const calledWith = storage.calledWith_[1]; assert(calledWith.authClient instanceof PassThroughClient); }); + + it('should not use PassThroughClient when keyFilename or credentials are provided', () => { + const storageWithKeyFile = new Storage({ + projectId: PROJECT_ID, + keyFilename: '/path/to/keyfile.json', + }); + assert.strictEqual( + storageWithKeyFile.calledWith_[1].authClient, + undefined + ); + + const storageWithCreds = new Storage({ + projectId: PROJECT_ID, + credentials: {client_email: 'a@b.com', private_key: 'key'}, + }); + assert.strictEqual( + storageWithCreds.calledWith_[1].authClient, + undefined + ); + }); }); }); From 073360b4826eef1a82a742ff6aef9292de23b35f Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 15 Oct 2026 03:04:53 +0000 Subject: [PATCH 3/5] fix: support asynchronous endpoint resolution for universe domain in signer --- handwritten/storage/src/signer.ts | 48 +++++++++++++++++------------- handwritten/storage/src/storage.ts | 5 +++- handwritten/storage/test/index.ts | 15 ++++++---- handwritten/storage/test/signer.ts | 3 ++ 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/handwritten/storage/src/signer.ts b/handwritten/storage/src/signer.ts index 9b9d0cb07ce..b8b48a33ff3 100644 --- a/handwritten/storage/src/signer.ts +++ b/handwritten/storage/src/signer.ts @@ -204,11 +204,15 @@ export class URLSigner { ); } - return promise.then(query => { + return promise.then(async query => { query = Object.assign(query, cfg.queryParams); + const endpoint = + typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : this.storage['apiEndpoint']; const signedUrl = new url.URL( - cfg.host?.toString() || config.cname || this.storage['apiEndpoint'] + cfg.host?.toString() || config.cname || endpoint ); signedUrl.pathname = this.getResourcePath( @@ -284,17 +288,6 @@ export class URLSigner { } const extensionHeaders = Object.assign({}, config.extensionHeaders); - const fqdn = new url.URL( - config.host?.toString() || config.cname || this.storage['apiEndpoint'] - ); - extensionHeaders.host = fqdn.hostname; - if (config.contentMd5) { - extensionHeaders['content-md5'] = config.contentMd5; - } - if (config.contentType) { - extensionHeaders['content-type'] = config.contentType; - } - let contentSha256: string; const sha256Header = extensionHeaders['x-goog-content-sha256']; if (sha256Header) { @@ -307,17 +300,32 @@ export class URLSigner { contentSha256 = sha256Header; } - const signedHeaders = Object.keys(extensionHeaders) - .map(header => header.toLowerCase()) - .sort() - .join(';'); - - const extensionHeadersString = this.getCanonicalHeaders(extensionHeaders); - const datestamp = formatAsUTCISO(config.accessibleAt); const credentialScope = `${datestamp}/auto/storage/goog4_request`; const sign = async () => { + const endpoint = + typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : this.storage['apiEndpoint']; + const fqdn = new url.URL( + config.host?.toString() || config.cname || endpoint + ); + extensionHeaders.host = fqdn.hostname; + if (config.contentMd5) { + extensionHeaders['content-md5'] = config.contentMd5; + } + if (config.contentType) { + extensionHeaders['content-type'] = config.contentType; + } + + const signedHeaders = Object.keys(extensionHeaders) + .map(header => header.toLowerCase()) + .sort() + .join(';'); + + const extensionHeadersString = this.getCanonicalHeaders(extensionHeaders); + const credentials = await this.auth.getCredentials(); const credential = `${credentials.client_email}/${credentialScope}`; const dateISO = formatAsUTCISO( diff --git a/handwritten/storage/src/storage.ts b/handwritten/storage/src/storage.ts index 493b99ff9bc..e273232715b 100644 --- a/handwritten/storage/src/storage.ts +++ b/handwritten/storage/src/storage.ts @@ -816,7 +816,10 @@ export class Storage extends Service { async getEndpoint(): Promise { if (!this.#hasCustomEndpoint) { - const universe = await this.authClient.getUniverseDomain(); + const universe = + typeof this.authClient.getUniverseDomain === 'function' + ? await this.authClient.getUniverseDomain() + : this.universeDomain; this.apiEndpoint = Storage.sanitizeEndpoint(`storage.${universe}`); } return this.apiEndpoint; diff --git a/handwritten/storage/test/index.ts b/handwritten/storage/test/index.ts index d7449dd0e8a..34e876375cc 100644 --- a/handwritten/storage/test/index.ts +++ b/handwritten/storage/test/index.ts @@ -432,13 +432,16 @@ describe('Storage', () => { .stub(GoogleAuth.prototype, 'getUniverseDomain') .resolves(universeDomain); - const storage = new Storage({universeDomain}); + try { + const storage = new Storage({universeDomain}); - assert.equal( - await storage.getEndpoint(), - `https://storage.${universeDomain}` - ); - stub.restore(); + assert.equal( + await storage.getEndpoint(), + `https://storage.${universeDomain}` + ); + } finally { + stub.restore(); + } }); describe('STORAGE_EMULATOR_HOST', () => { diff --git a/handwritten/storage/test/signer.ts b/handwritten/storage/test/signer.ts index e8f5371084e..6cc7c6833f4 100644 --- a/handwritten/storage/test/signer.ts +++ b/handwritten/storage/test/signer.ts @@ -64,6 +64,9 @@ describe('signer', () => { afterEach(() => fakeTimers.restore()); beforeEach(() => { + sandbox + .stub(GoogleAuth.prototype, 'getUniverseDomain') + .resolves('googleapis.com'); authClient = { sign: async () => 'signature', getCredentials: async () => ({client_email: CLIENT_EMAIL}), From 5b2a8542309f18c34febdf08ca4f525c4cc7d126 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Mon, 19 Oct 2026 06:56:28 +0000 Subject: [PATCH 4/5] feat: add deprecated customEndpoint and useAuthWithCustomEndpoint options to service and upload configurations --- .../storage/src/nodejs-common/service.ts | 24 +++++++++++++++++++ handwritten/storage/src/nodejs-common/util.ts | 12 ++++++++++ handwritten/storage/src/resumable-upload.ts | 6 +++++ 3 files changed, 42 insertions(+) diff --git a/handwritten/storage/src/nodejs-common/service.ts b/handwritten/storage/src/nodejs-common/service.ts index c81c2ac3d12..57fe7680a7e 100644 --- a/handwritten/storage/src/nodejs-common/service.ts +++ b/handwritten/storage/src/nodejs-common/service.ts @@ -62,6 +62,18 @@ export interface ServiceConfig { * Reuse an existing `AuthClient` or `GoogleAuth` client instead of creating a new one. */ authClient?: AuthClient | GoogleAuth; + + /** + * Set to true if the endpoint is a custom URL + * @deprecated + */ + customEndpoint?: boolean; + + /** + * Controls whether or not to use authentication when using a custom endpoint. + * @deprecated + */ + useAuthWithCustomEndpoint?: boolean; } export interface ServiceOptions extends Omit { @@ -71,6 +83,10 @@ export interface ServiceOptions extends Omit { token?: string; timeout?: number; // http.request.options.timeout userAgent?: string; + /** + * @deprecated + */ + useAuthWithCustomEndpoint?: boolean; } export class Service { @@ -86,6 +102,14 @@ export class Service { protected apiEndpoint: string; timeout?: number; universeDomain: string; + /** + * @deprecated + */ + customEndpoint = false; + /** + * @deprecated + */ + useAuthWithCustomEndpoint?: boolean; /** * Service is a base class, meant to be inherited from by a "service," like diff --git a/handwritten/storage/src/nodejs-common/util.ts b/handwritten/storage/src/nodejs-common/util.ts index 981259fa801..37166143e6a 100644 --- a/handwritten/storage/src/nodejs-common/util.ts +++ b/handwritten/storage/src/nodejs-common/util.ts @@ -168,6 +168,18 @@ export interface MakeAuthenticatedRequestFactoryConfig extends Omit< */ autoRetry?: boolean; + /** + * If true, just return the provided request options. Default: false. + * @deprecated + */ + customEndpoint?: boolean; + + /** + * If true, will authenticate when using a custom endpoint. Default: false. + * @deprecated + */ + useAuthWithCustomEndpoint?: boolean; + /** * Account email address, required for PEM/P12 usage. */ diff --git a/handwritten/storage/src/resumable-upload.ts b/handwritten/storage/src/resumable-upload.ts index c64070becf8..0474db09b82 100644 --- a/handwritten/storage/src/resumable-upload.ts +++ b/handwritten/storage/src/resumable-upload.ts @@ -245,6 +245,12 @@ export interface UploadConfig extends Pick { */ retryOptions: RetryOptions; + /** + * Controls whether or not to use authentication when using a custom endpoint. + * @deprecated + */ + useAuthWithCustomEndpoint?: boolean; + [GCCL_GCS_CMD_KEY]?: string; } From 70f65b7df9c96acccf87f05dd49cd7e888c15225 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Mon, 2 Nov 2026 12:18:17 +0000 Subject: [PATCH 5/5] fix: support dynamic storage endpoints and universe domain resolution for uploads and signed URLs --- handwritten/storage/src/file.ts | 216 +++++++++++-------- handwritten/storage/src/resumable-upload.ts | 24 --- handwritten/storage/src/signer.ts | 25 +-- handwritten/storage/src/storage.ts | 1 + handwritten/storage/src/transfer-manager.ts | 19 +- handwritten/storage/test/file.ts | 42 +++- handwritten/storage/test/resumable-upload.ts | 10 +- handwritten/storage/test/signer.ts | 26 +++ 8 files changed, 221 insertions(+), 142 deletions(-) diff --git a/handwritten/storage/src/file.ts b/handwritten/storage/src/file.ts index bd00602d7d2..b033947280a 100644 --- a/handwritten/storage/src/file.ts +++ b/handwritten/storage/src/file.ts @@ -1996,30 +1996,41 @@ class File extends ServiceObject { retryOptions.autoRetry = false; } - resumableUpload.createURI( - { - authClient: this.storage.storageTransport.authClient, - apiEndpoint: this.storage['apiEndpoint'], - bucket: this.bucket.name, - file: this.name, - generation: this.generation, - key: this.encryptionKey === null ? undefined : this.encryptionKey, - kmsKeyName: this.kmsKeyName, - metadata: options.metadata, - offset: options.offset, - origin: options.origin, - predefinedAcl: options.predefinedAcl, - private: options.private, - public: options.public, - userProject: options.userProject || this.userProject, - retryOptions: retryOptions, - params: options?.preconditionOpts || this.instancePreconditionOpts, - universeDomain: this.bucket.storage.universeDomain, - [GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY], - }, - callback!, - ); - this.storage.retryOptions.autoRetry = this.instanceRetryValue; + void (async () => { + try { + const apiEndpoint = + (typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : undefined) || this.storage['apiEndpoint']; + resumableUpload.createURI( + { + authClient: this.storage.storageTransport.authClient, + apiEndpoint, + bucket: this.bucket.name, + file: this.name, + generation: this.generation, + key: this.encryptionKey === null ? undefined : this.encryptionKey, + kmsKeyName: this.kmsKeyName, + metadata: options.metadata, + offset: options.offset, + origin: options.origin, + predefinedAcl: options.predefinedAcl, + private: options.private, + public: options.public, + userProject: options.userProject || this.userProject, + retryOptions: {...retryOptions}, + params: options?.preconditionOpts || this.instancePreconditionOpts, + universeDomain: this.bucket.storage.universeDomain, + [GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY], + }, + callback!, + ); + } catch (err) { + callback!(err as Error); + } finally { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; + } + })(); } /** @@ -2405,7 +2416,7 @@ class File extends ServiceObject { ); if (options.resumable === false) { - this.startSimpleUpload_( + void this.startSimpleUpload_( fileWriteStream, options as CreateWriteStreamOptionsInternal, ); @@ -3240,7 +3251,6 @@ class File extends ServiceObject { options.signingEndpoint, ); const signatureHex = Buffer.from(signature, 'base64').toString('hex'); - const universe = this.parent.storage.universeDomain; fields['policy'] = policyBase64; fields['x-goog-signature'] = signatureHex; @@ -3248,12 +3258,14 @@ class File extends ServiceObject { const EMULATOR_HOST = process.env.STORAGE_EMULATOR_HOST; const endpoint = - typeof this.storage.getEndpoint === 'function' + (typeof this.storage.getEndpoint === 'function' ? await this.storage.getEndpoint() - : this.storage['apiEndpoint']; - const defaultEndpoint = `https://storage.${ - universe || 'googleapis.com' - }`; + : undefined) || this.storage['apiEndpoint']; + const universe = + this.parent.storage.universeDomain || + this.storage.universeDomain || + 'googleapis.com'; + const defaultEndpoint = `https://storage.${universe}`; if (endpoint !== defaultEndpoint && typeof EMULATOR_HOST === 'string') { url = `${endpoint}/${this.bucket.name}`; @@ -3589,11 +3601,6 @@ class File extends ServiceObject { undefined, callback, ); - const baseUrl = this.storage['apiEndpoint'].startsWith('http') - ? this.storage['apiEndpoint'] - : `https://${this.storage['apiEndpoint']}`; - - const url = `${baseUrl}/storage/v1/b/${this.bucket.name}/o/${encodeURIComponent(this.name)}`; const gaxios = new Gaxios(); const storageInterceptors = this.storage?.interceptors || []; @@ -3606,6 +3613,16 @@ class File extends ServiceObject { // eslint-disable-next-line @typescript-eslint/no-floating-promises (async () => { try { + const endpoint = + (typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : undefined) || this.storage['apiEndpoint']; + const baseUrl = endpoint.startsWith('http') + ? endpoint + : `https://${endpoint}`; + + const url = `${baseUrl}/storage/v1/b/${this.bucket.name}/o/${encodeURIComponent(this.name)}`; + await gaxios.request({ method: 'GET', url, @@ -4734,58 +4751,64 @@ class File extends ServiceObject { ) { retryOptions.autoRetry = false; } - const cfg = { - authClient: this.storage.storageTransport.authClient, - apiEndpoint: this.storage['apiEndpoint'], - bucket: this.bucket.name, - file: this.name, - generation: this.generation, - isPartialUpload: options.isPartialUpload, - key: this.encryptionKey === null ? undefined : this.encryptionKey, - kmsKeyName: this.kmsKeyName, - metadata: options.metadata, - offset: options.offset, - predefinedAcl: options.predefinedAcl, - private: options.private, - public: options.public, - uri: options.uri, - userProject: options.userProject || this.userProject, - retryOptions: {...retryOptions}, - params: options?.preconditionOpts || this.instancePreconditionOpts, - chunkSize: options?.chunkSize, - highWaterMark: options?.highWaterMark, - universeDomain: this.bucket.storage.universeDomain, - [GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY], - }; + const uploadRetryOptions = {...retryOptions}; + this.storage.retryOptions.autoRetry = this.instanceRetryValue; - let uploadStream: resumableUpload.Upload; + void (async () => { + let uploadStream: resumableUpload.Upload; - try { - uploadStream = resumableUpload.upload(cfg); - } catch (error) { - dup.destroy(error as Error); - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - return; - } + try { + const apiEndpoint = + (typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : undefined) || this.storage['apiEndpoint']; + const cfg = { + authClient: this.storage.storageTransport.authClient, + apiEndpoint, + bucket: this.bucket.name, + file: this.name, + generation: this.generation, + isPartialUpload: options.isPartialUpload, + key: this.encryptionKey === null ? undefined : this.encryptionKey, + kmsKeyName: this.kmsKeyName, + metadata: options.metadata, + offset: options.offset, + predefinedAcl: options.predefinedAcl, + private: options.private, + public: options.public, + uri: options.uri, + userProject: options.userProject || this.userProject, + retryOptions: uploadRetryOptions, + params: options?.preconditionOpts || this.instancePreconditionOpts, + chunkSize: options?.chunkSize, + highWaterMark: options?.highWaterMark, + universeDomain: this.bucket.storage.universeDomain, + [GCCL_GCS_CMD_KEY]: options[GCCL_GCS_CMD_KEY], + }; + uploadStream = resumableUpload.upload(cfg); + } catch (error) { + dup.destroy(error as Error); + return; + } - uploadStream - .on('response', resp => { - dup.emit('response', resp); - }) - .on('uri', uri => { - dup.emit('uri', uri); - }) - .on('metadata', metadata => { - this.metadata = metadata; - dup.emit('metadata'); - }) - .on('finish', () => { - dup.emit('complete'); - }) - .on('progress', evt => dup.emit('progress', evt)); + uploadStream + .on('response', resp => { + dup.emit('response', resp); + }) + .on('uri', uri => { + dup.emit('uri', uri); + }) + .on('metadata', metadata => { + this.metadata = metadata; + dup.emit('metadata'); + }) + .on('finish', () => { + dup.emit('complete'); + }) + .on('progress', evt => dup.emit('progress', evt)); - dup.setWritable(uploadStream); - this.storage.retryOptions.autoRetry = this.instanceRetryValue; + dup.setWritable(uploadStream); + })(); } /** @@ -4798,13 +4821,26 @@ class File extends ServiceObject { * * @private */ - startSimpleUpload_( + async startSimpleUpload_( dup: Duplexify, options: CreateWriteStreamOptionsInternal = {}, - ): void { + ): Promise { options.metadata ??= {}; - const apiEndpoint = this.storage['apiEndpoint']; + const writeStream = new ProgressStream(); + writeStream.on('progress', evt => dup.emit('progress', evt)); + dup.setWritable(writeStream); + + let apiEndpoint: string; + try { + apiEndpoint = + (typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : undefined) || this.storage['apiEndpoint']; + } catch (err) { + dup.destroy(err as Error); + return; + } const bucketName = this.bucket.name; const url = `${apiEndpoint}/upload/storage/v1/b/${bucketName}/o`; @@ -4851,10 +4887,6 @@ class File extends ServiceObject { options.preconditionOpts, ); - const writeStream = new ProgressStream(); - writeStream.on('progress', evt => dup.emit('progress', evt)); - dup.setWritable(writeStream); - reqOpts.multipart = [ { headers: new Headers({'Content-Type': 'application/json'}), @@ -4892,7 +4924,7 @@ class File extends ServiceObject { dup.emit('response', resp); dup.emit('complete'); }) - .catch(err => dup.destroy(err)); + ?.catch(err => dup.destroy(err)); } disableAutoRetryConditionallyIdempotent_( diff --git a/handwritten/storage/src/resumable-upload.ts b/handwritten/storage/src/resumable-upload.ts index 44ce89e451a..f55d6b73c48 100644 --- a/handwritten/storage/src/resumable-upload.ts +++ b/handwritten/storage/src/resumable-upload.ts @@ -18,7 +18,6 @@ import { GaxiosResponse, GaxiosError, } from 'gaxios'; -import * as gaxios from 'gaxios'; import { DEFAULT_UNIVERSE, GoogleAuth, @@ -386,29 +385,6 @@ export class Upload extends Writable { this.apiEndpoint = `https://storage.${universe}`; if (cfg.apiEndpoint && cfg.apiEndpoint !== this.apiEndpoint) { this.apiEndpoint = this.sanitizeEndpoint(cfg.apiEndpoint); - - const hostname = new URL(this.apiEndpoint).hostname; - - // check if it is a domain of a known universe - const isDomain = hostname === universe; - const isDefaultUniverseDomain = hostname === DEFAULT_UNIVERSE; - - // check if it is a subdomain of a known universe - // by checking a last (universe's length + 1) of a hostname - const isSubDomainOfUniverse = - hostname.slice(-(universe.length + 1)) === `.${universe}`; - const isSubDomainOfDefaultUniverse = - hostname.slice(-(DEFAULT_UNIVERSE.length + 1)) === - `.${DEFAULT_UNIVERSE}`; - - if ( - !isDomain && - !isDefaultUniverseDomain && - !isSubDomainOfUniverse && - !isSubDomainOfDefaultUniverse - ) { - this.authClient = gaxios; - } } this.baseURI = `${this.apiEndpoint}/upload/storage/v1/b`; diff --git a/handwritten/storage/src/signer.ts b/handwritten/storage/src/signer.ts index eecfac88d05..ae59b723ebe 100644 --- a/handwritten/storage/src/signer.ts +++ b/handwritten/storage/src/signer.ts @@ -208,12 +208,13 @@ export class URLSigner { query = Object.assign(query, cfg.queryParams); const endpoint = - typeof this.storage.getEndpoint === 'function' + config.host?.toString() || + config.cname || + (typeof this.storage.getEndpoint === 'function' ? await this.storage.getEndpoint() - : this.storage['apiEndpoint']; - const signedUrl = new url.URL( - cfg.host?.toString() || config.cname || endpoint, - ); + : undefined) || + this.storage['apiEndpoint']; + const signedUrl = new url.URL(endpoint); signedUrl.pathname = this.getResourcePath( !!config.cname, @@ -304,13 +305,13 @@ export class URLSigner { const credentialScope = `${datestamp}/auto/storage/goog4_request`; const sign = async () => { - const endpoint = - typeof this.storage.getEndpoint === 'function' - ? await this.storage.getEndpoint() - : this.storage['apiEndpoint']; - const fqdn = new url.URL( - config.host?.toString() || config.cname || endpoint, - ); + if (!config.host && !config.cname) { + config.host = + (typeof this.storage.getEndpoint === 'function' + ? await this.storage.getEndpoint() + : undefined) || this.storage['apiEndpoint']; + } + const fqdn = new url.URL(config.host?.toString() || config.cname!); extensionHeaders.host = fqdn.hostname; if (config.contentMd5) { extensionHeaders['content-md5'] = config.contentMd5; diff --git a/handwritten/storage/src/storage.ts b/handwritten/storage/src/storage.ts index 9ad1cb7db54..35bc9a8f14a 100644 --- a/handwritten/storage/src/storage.ts +++ b/handwritten/storage/src/storage.ts @@ -965,6 +965,7 @@ export class Storage { 'function' ? await this.storageTransport.authClient.getUniverseDomain() : this.universeDomain; + this.universeDomain = universe; this.apiEndpoint = Storage.sanitizeEndpoint(`storage.${universe}`); } return this.apiEndpoint; diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts index 06f7de04338..bb048a731d9 100644 --- a/handwritten/storage/src/transfer-manager.ts +++ b/handwritten/storage/src/transfer-manager.ts @@ -254,6 +254,17 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { return headers; } + async #getBaseUrl(): Promise { + const endpoint = + (typeof this.bucket.storage.getEndpoint === 'function' + ? await this.bucket.storage.getEndpoint() + : undefined) || this.bucket.storage['apiEndpoint']; + this.baseUrl = `https://${this.bucket.name}.${ + new URL(endpoint).hostname + }/${this.fileName}`; + return this.baseUrl; + } + /** * Initiates a multipart upload (MPU) to the XML API and stores the resultant upload id. * @@ -261,7 +272,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { */ async initiateUpload(headers?: {[key: string]: string}): Promise { const headersObject = new Headers(headers); - const url = `${this.baseUrl}?uploads`; + const url = `${await this.#getBaseUrl()}?uploads`; return AsyncRetry(async bail => { try { const res = await this.authClient.request< @@ -302,7 +313,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { chunk: Buffer, validation?: 'md5' | 'crc32c' | false, ): Promise { - const url = `${this.baseUrl}?partNumber=${partNumber}&uploadId=${this.uploadId}`; + const url = `${await this.#getBaseUrl()}?partNumber=${partNumber}&uploadId=${this.uploadId}`; const headers: Headers = this.#setGoogApiClientHeaders(); if (validation === 'md5') { @@ -341,7 +352,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { * @returns {Promise} */ async completeUpload(): Promise { - const url = `${this.baseUrl}?uploadId=${this.uploadId}`; + const url = `${await this.#getBaseUrl()}?uploadId=${this.uploadId}`; const sortedMap = new Map( [...this.partsMap.entries()].sort((a, b) => a[0] - b[0]), ); @@ -380,7 +391,7 @@ class XMLMultiPartUploadHelper implements MultiPartUploadHelper { * @returns {Promise} */ async abortUpload(): Promise { - const url = `${this.baseUrl}?uploadId=${this.uploadId}`; + const url = `${await this.#getBaseUrl()}?uploadId=${this.uploadId}`; return AsyncRetry(async bail => { try { const res = await this.authClient.request( diff --git a/handwritten/storage/test/file.ts b/handwritten/storage/test/file.ts index 52474d5c8b3..46b1b847ea7 100644 --- a/handwritten/storage/test/file.ts +++ b/handwritten/storage/test/file.ts @@ -3512,6 +3512,31 @@ describe('File', () => { }); }); + it('should append bucket name when universeDomain is resolved dynamically via getEndpoint', done => { + const dynamicUniverse = 'my-universe.com'; + const dynamicEndpoint = `https://storage.${dynamicUniverse}`; + const originalAuthClient = STORAGE.storageTransport.authClient; + const originalUniverseDomain = STORAGE.universeDomain; + + STORAGE.storageTransport.authClient = { + ...BUCKET.storage.storageTransport.authClient, + getUniverseDomain: sandbox.stub().resolves(dynamicUniverse), + }; + (STORAGE.getEndpoint as sinon.SinonStub).callsFake(async () => { + STORAGE.universeDomain = dynamicUniverse; + return dynamicEndpoint; + }); + + file.generateSignedPostPolicyV4(CONFIG, (err, res) => { + STORAGE.storageTransport.authClient = originalAuthClient; + STORAGE.universeDomain = originalUniverseDomain; + + assert.ifError(err); + assert.strictEqual(res?.url, `${dynamicEndpoint}/${file.bucket.name}/`); + done(); + }); + }); + describe('expires', () => { it('should accept Date objects', () => { const expires = new Date(Date.now() + 1000 * 60); @@ -5709,12 +5734,15 @@ describe('File', () => { }); describe('startSimpleUpload_', () => { - it('should get a writable stream', async done => { + it('should get a writable stream', async () => { + let makeRequestCalled = false; file.storageTransport.makeRequest = sandbox.stub().callsFake(() => { - done(); + makeRequestCalled = true; + return Promise.resolve({}); }); await file.startSimpleUpload_(duplexify()); + assert.strictEqual(makeRequestCalled, true); }); it('should pass the required arguments', async () => { @@ -5850,7 +5878,7 @@ describe('File', () => { it('should destroy the stream', done => { const stream = duplexify(); - file.startSimpleUpload_(stream); + void file.startSimpleUpload_(stream); stream.on('error', (err: Error) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -5871,22 +5899,22 @@ describe('File', () => { .resolves({data: body, resp}); }); - it('should set the metadata', () => { + it('should set the metadata', async () => { const stream = duplexify(); - file.startSimpleUpload_(stream); + await file.startSimpleUpload_(stream); assert.deepEqual(file.metadata, body); }); - it('should emit the response', () => { + it('should emit the response', async () => { const stream = duplexify(); stream.on('response', resp_ => { assert.strictEqual(resp_, resp); }); - file.startSimpleUpload_(stream); + await file.startSimpleUpload_(stream); }); it('should emit complete', async () => { diff --git a/handwritten/storage/test/resumable-upload.ts b/handwritten/storage/test/resumable-upload.ts index a21f75d3af6..9d80a0794d5 100644 --- a/handwritten/storage/test/resumable-upload.ts +++ b/handwritten/storage/test/resumable-upload.ts @@ -2708,7 +2708,7 @@ describe('resumable-upload', () => { assert.deepStrictEqual(headersObj, {}); }); - it('should bypass authentication if emulator context detected', async () => { + it('should use authentication with custom endpoint', async () => { up = upload({ bucket: BUCKET, file: FILE, @@ -2724,7 +2724,11 @@ describe('resumable-upload', () => { retryOptions: RETRY_OPTIONS, }); const scopes = [ - nock(REQ_OPTS.url!).get(queryPath).reply(200, undefined, {}), + mockAuthorizeRequest(), + nock(REQ_OPTS.url!) + .matchHeader('authorization', /Bearer .+/) + .get(queryPath) + .reply(200, undefined, {}), ]; const res = await up.makeRequest(REQ_OPTS); scopes.forEach(x => x.done()); @@ -2732,7 +2736,7 @@ describe('resumable-upload', () => { res.config.url!.toString(), REQ_OPTS.url + queryPath.slice(1), ); - assert.ok(res.headers); + assert.ok(getHeader(res.config.headers, 'Authorization')); }); it('should combine customRequestOptions', done => { diff --git a/handwritten/storage/test/signer.ts b/handwritten/storage/test/signer.ts index 735a6c94e82..98fa681e322 100644 --- a/handwritten/storage/test/signer.ts +++ b/handwritten/storage/test/signer.ts @@ -450,6 +450,32 @@ describe('signer', () => { const signedUrl = await signer.getSignedUrl(CONFIG); assert(signedUrl.includes(qsStringify(query))); }); + + it('should call storage.getEndpoint only once for v4 signed URLs when host and cname are not set', async () => { + const getEndpointSpy = sandbox.spy(storage, 'getEndpoint'); + CONFIG.version = 'v4'; + + await signer.getSignedUrl(CONFIG); + assert.strictEqual(getEndpointSpy.callCount, 1); + }); + + it('should not call storage.getEndpoint when host or cname is provided', async () => { + const getEndpointSpy = sandbox.spy(storage, 'getEndpoint'); + + await signer.getSignedUrl({ + ...CONFIG, + version: 'v4', + host: 'https://custom.host.com', + }); + assert.strictEqual(getEndpointSpy.callCount, 0); + + await signer.getSignedUrl({ + ...CONFIG, + version: 'v4', + cname: 'https://cdn.example.com', + }); + assert.strictEqual(getEndpointSpy.callCount, 0); + }); }); describe('getSignedUrlV2', () => {