From b001807a82616521680baf5429fec6af639aeb58 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Wed, 5 Aug 2026 05:49:25 +0000 Subject: [PATCH 1/7] fix(storage): pass signingEndpoint to URLSigner in file.getSignedUrl (#8982) Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/{{metadata['repo']['name']}}/issues) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #8829 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- handwritten/storage/src/file.ts | 1 + handwritten/storage/src/signer.ts | 2 +- handwritten/storage/test/file.ts | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/handwritten/storage/src/file.ts b/handwritten/storage/src/file.ts index 27765d935a99..d5aa40bf96e1 100644 --- a/handwritten/storage/src/file.ts +++ b/handwritten/storage/src/file.ts @@ -3236,6 +3236,7 @@ class File extends ServiceObject { contentMd5: cfg.contentMd5, contentType: cfg.contentType, host: cfg.host, + signingEndpoint: cfg.signingEndpoint, }; if (cfg.cname) { diff --git a/handwritten/storage/src/signer.ts b/handwritten/storage/src/signer.ts index a657cef6133d..ba5c17c04b75 100644 --- a/handwritten/storage/src/signer.ts +++ b/handwritten/storage/src/signer.ts @@ -25,7 +25,7 @@ type GoogleAuthLike = Pick; * @deprecated Use {@link GoogleAuth} instead */ export interface AuthClient { - sign(blobToSign: string): Promise; + sign(blobToSign: string, signingEndpoint?: string): Promise; getCredentials(): Promise<{ client_email?: string; }>; diff --git a/handwritten/storage/test/file.ts b/handwritten/storage/test/file.ts index 26823995b907..d9f9185a16e8 100644 --- a/handwritten/storage/test/file.ts +++ b/handwritten/storage/test/file.ts @@ -3794,11 +3794,30 @@ describe('File', () => { contentType: config.contentType, cname: CNAME, virtualHostedStyle: true, + signingEndpoint: undefined, }); done(); }); }); + it('should pass signingEndpoint to URLSigner', done => { + const signingEndpoint = 'https://my-endpoint.com'; + const config = { + ...SIGNED_URL_CONFIG, + signingEndpoint, + }; + + file.getSignedUrl(config, (err: Error | null) => { + assert.ifError(err); + const getSignedUrlArgs = signerGetSignedUrlStub.getCall(0).args; + assert.strictEqual( + getSignedUrlArgs[0]['signingEndpoint'], + signingEndpoint + ); + done(); + }); + }); + it('should add "x-goog-resumable: start" header if action is resumable', done => { SIGNED_URL_CONFIG.action = 'resumable'; SIGNED_URL_CONFIG.extensionHeaders = { From 81612c7679a23535b90508e49e73e724cecb1787 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Wed, 5 Aug 2026 06:11:34 +0000 Subject: [PATCH 2/7] docs(storage): add comprehensive JSDoc documentation to GetSignedUrlConfig interface properties (#8782) Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/{{metadata['repo']['name']}}/issues) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #7352 --------- Co-authored-by: Dhriti07 <56169283+Dhriti07@users.noreply.github.com> --- handwritten/storage/src/file.ts | 75 ++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/handwritten/storage/src/file.ts b/handwritten/storage/src/file.ts index d5aa40bf96e1..786998c5f4e4 100644 --- a/handwritten/storage/src/file.ts +++ b/handwritten/storage/src/file.ts @@ -148,21 +148,94 @@ export interface SignedPostPolicyV4Output { url: string; fields: PolicyFields; } - export interface GetSignedUrlConfig extends Pick { + /** + * The action to permit with the signed URL. + * - `'read'`: Allows downloading/viewing the file (HTTP GET). + * - `'write'`: Allows uploading/overwriting the file (HTTP PUT). + * - `'delete'`: Allows removing the file (HTTP DELETE). + * - `'resumable'`: Allows resumable uploads (HTTP POST). + * Note: When using `'resumable'`, the header `X-Goog-Resumable: start` must be sent in the client request. + */ action: 'read' | 'write' | 'delete' | 'resumable'; + + /** + * The signing version to use. + * @default 'v2' + */ version?: 'v2' | 'v4'; + + /** + * Determines the URL structure for accessing bucket resources. + * - `true`: Uses virtual hosted-style URLs (e.g., `https://mybucket.storage.googleapis.com/...`) + * - `false`: Uses path-style URLs (e.g., `https://storage.googleapis.com/mybucket/...`). + * Virtual hosted-style URLs are generally preferred. + * @default false + */ virtualHostedStyle?: boolean; + + /** + * The custom domain name (CNAME) mapped to this bucket (e.g., `"https://cdn.example.com"`). + */ cname?: string; + + /** + * The MD5 digest value in base64. If provided, the client request **must** + * include an identical `Content-MD5` HTTP header. + * If omitted, the client request must not include this header. + */ contentMd5?: string; + + /** + * The expected Content-Type of the file. If provided, the client request **must** + * include an identical `Content-Type` HTTP header. + * If omitted, the client request must not include this header. + */ contentType?: string; + + /** + * The expiration timestamp for the link. Any provided value is passed directly to `new Date()`. + * @throws {Error} If an expiration timestamp from the past is given. + * Note: `'v4'` signing supports a maximum duration of 7 days (604,800 seconds) from the creation time. + */ expires: string | number | Date; + + /** + * The timestamp when this link becomes usable. Any provided value is passed directly to `new Date()`. + * @default Date.now() + * Note: Only supported/applicable when `version` is set to `'v4'`. + */ accessibleAt?: string | number | Date; + + /** + * Canonical extension headers that the server will validate against the client's request. + * Requirements: + * - Header names must be prefixed with `x-goog-` and must be entirely lowercase. + * - Multi-valued headers passed as an array are converted into a comma-separated string (no spaces). + * The client must format them identically to prevent signature mismatches. + */ extensionHeaders?: http.OutgoingHttpHeaders; + + /** + * The filename to prompt the browser/user to save the file as upon access. + * Note: This option is ignored if `responseDisposition` is explicitly set. + */ promptSaveAs?: string; + + /** + * Maps to the `response-content-disposition` query parameter in the signed URL. + */ responseDisposition?: string; + + /** + * Maps to the `response-content-type` query parameter in the signed URL. + */ responseType?: string; + + /** + * Additional query parameters to include natively in the generated signed URL. + */ queryParams?: Query; } From d3833dac7534baf7b436ce7a2417f6530067f965 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 09:20:15 +0000 Subject: [PATCH 3/7] fix(storage): destroy local read stream on upload write failure to prevent resource leaks --- handwritten/storage/src/bucket.ts | 11 ++++++-- handwritten/storage/test/bucket.ts | 44 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 5c796789ebd3..09907e82a92a 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4501,17 +4501,22 @@ class Bucket extends ServiceObject { ) { newFile.storage.retryOptions.autoRetry = false; } + const readStream = fs.createReadStream(pathString); const writable = newFile.createWriteStream(options); if (options.onUploadProgress) { writable.on('progress', options.onUploadProgress); } - fs.createReadStream(pathString) - .on('error', bail) + readStream + .on('error', err => { + readStream.destroy(); + bail(err); + }) .pipe(writable) .on('error', err => { + readStream.destroy(); if ( this.storage.retryOptions.autoRetry && - this.storage.retryOptions.retryableErrorFn!(err) + this.storage.retryOptions.retryableErrorFn!(err as any) ) { return reject(err); } else { diff --git a/handwritten/storage/test/bucket.ts b/handwritten/storage/test/bucket.ts index 555d8e8c1c9c..3333b38a96d3 100644 --- a/handwritten/storage/test/bucket.ts +++ b/handwritten/storage/test/bucket.ts @@ -100,11 +100,18 @@ class FakeNotification { } let fsStatOverride: Function | null; +let fsCreateReadStreamOverride: Function | null; const fakeFs = { ...fs, stat: (filePath: string, callback: Function) => { return (fsStatOverride || fs.stat)(filePath, callback); }, + createReadStream: (filePath: string, options?: any) => { + return (fsCreateReadStreamOverride || fs.createReadStream)( + filePath, + options + ); + }, }; let pLimitOverride: Function | null; @@ -3231,6 +3238,43 @@ describe('Bucket', () => { }); }); + it('should destroy the local read stream if write stream fails', done => { + const fakeFile = new FakeFile(bucket, 'file-name'); + const options = {destination: fakeFile, resumable: false}; + const originalCreateReadStream = fs.createReadStream; + let readStream: fs.ReadStream; + fsCreateReadStreamOverride = (path: string, opts: any) => { + readStream = originalCreateReadStream(path, opts); + return readStream; + }; + + fakeFile.createWriteStream = (options_: CreateWriteStreamOptions) => { + const ws = new stream.Writable({ + write(chunk, encoding, callback) { + callback(new Error('write error')); + }, + }); + return ws; + }; + + const textfilepath = path.join( + getDirName(), + '../../../test/testdata/textfile.txt' + ); + + bucket.upload(textfilepath, options, (err: Error) => { + try { + assert.strictEqual(err.message, 'write error'); + assert.ok(readStream.destroyed); + done(); + } catch (e) { + done(e); + } finally { + fsCreateReadStreamOverride = null; + } + }); + }); + it('should allow overriding content type', done => { const fakeFile = new FakeFile(bucket, 'file-name'); const metadata = {contentType: 'made-up-content-type'}; From fb21fdc5c3fa86dbe7775751a226de262f8e760f Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 09:58:00 +0000 Subject: [PATCH 4/7] fix(storage): destroy writable stream on read error and reset read stream override in tests --- handwritten/storage/src/bucket.ts | 1 + handwritten/storage/test/bucket.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 09907e82a92a..169a7797f8a3 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4509,6 +4509,7 @@ class Bucket extends ServiceObject { readStream .on('error', err => { readStream.destroy(); + writable.destroy(); bail(err); }) .pipe(writable) diff --git a/handwritten/storage/test/bucket.ts b/handwritten/storage/test/bucket.ts index 3333b38a96d3..1f80a317afa0 100644 --- a/handwritten/storage/test/bucket.ts +++ b/handwritten/storage/test/bucket.ts @@ -241,6 +241,7 @@ describe('Bucket', () => { beforeEach(() => { fsStatOverride = null; + fsCreateReadStreamOverride = null; pLimitOverride = null; bucket = new Bucket(STORAGE, BUCKET_NAME); }); @@ -3265,6 +3266,7 @@ describe('Bucket', () => { bucket.upload(textfilepath, options, (err: Error) => { try { assert.strictEqual(err.message, 'write error'); + assert.ok(readStream); assert.ok(readStream.destroyed); done(); } catch (e) { From ac442976845077402b1cd8a4cc8e1ce817747eba Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 25 Jun 2026 11:36:48 +0000 Subject: [PATCH 5/7] fix: reorder read stream initialization to prevent premature stream destruction in upload flow --- handwritten/storage/src/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 169a7797f8a3..76decb6cd95d 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4501,11 +4501,11 @@ class Bucket extends ServiceObject { ) { newFile.storage.retryOptions.autoRetry = false; } - const readStream = fs.createReadStream(pathString); const writable = newFile.createWriteStream(options); if (options.onUploadProgress) { writable.on('progress', options.onUploadProgress); } + const readStream = fs.createReadStream(pathString); readStream .on('error', err => { readStream.destroy(); From 94037d4729102605d54d869abfbf1e7970e7b956 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 26 Jun 2026 06:46:12 +0000 Subject: [PATCH 6/7] fix: update error type to ApiError for retryable error check in bucket operations --- handwritten/storage/src/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/src/bucket.ts b/handwritten/storage/src/bucket.ts index 76decb6cd95d..527e7396f87f 100644 --- a/handwritten/storage/src/bucket.ts +++ b/handwritten/storage/src/bucket.ts @@ -4517,7 +4517,7 @@ class Bucket extends ServiceObject { readStream.destroy(); if ( this.storage.retryOptions.autoRetry && - this.storage.retryOptions.retryableErrorFn!(err as any) + this.storage.retryOptions.retryableErrorFn!(err as ApiError) ) { return reject(err); } else { From 130f7d5c036c723220beeb7b87743d8cdd34cf23 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Wed, 5 Aug 2026 07:24:44 +0000 Subject: [PATCH 7/7] refactor: update createReadStream options type to use fs.createReadStream parameters --- handwritten/storage/test/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/test/bucket.ts b/handwritten/storage/test/bucket.ts index 1f80a317afa0..23874839e1a2 100644 --- a/handwritten/storage/test/bucket.ts +++ b/handwritten/storage/test/bucket.ts @@ -106,7 +106,7 @@ const fakeFs = { stat: (filePath: string, callback: Function) => { return (fsStatOverride || fs.stat)(filePath, callback); }, - createReadStream: (filePath: string, options?: any) => { + createReadStream: (filePath: string, options?: Parameters[1]) => { return (fsCreateReadStreamOverride || fs.createReadStream)( filePath, options