From c6ee55df44e0cd9a022a692e726a35955cc31a11 Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 22 May 2026 13:20:09 +0000 Subject: [PATCH 1/3] feat: enable CRC32C validation by default in transfer manager and disable it for individual shard downloads --- handwritten/storage/src/transfer-manager.ts | 19 ++++++++- handwritten/storage/test/transfer-manager.ts | 43 ++++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts index e4d9762e1a5f..5830fcc68ac6 100644 --- a/handwritten/storage/src/transfer-manager.ts +++ b/handwritten/storage/src/transfer-manager.ts @@ -736,7 +736,7 @@ export class TransferManager { * @property {number} [concurrencyLimit] The number of concurrently executing promises * to use when downloading the file. * @property {number} [chunkSizeBytes] The size in bytes of each chunk to be downloaded. - * @property {string | boolean} [validation] Whether or not to perform a CRC32C validation check when download is complete. + * @property {string | boolean} [validation] Whether or not to perform a CRC32C validation check when download is complete. Defaults to 'crc32c'. * @property {boolean} [noReturnData] Whether or not to return the downloaded data. A `true` value here would be useful for files with a size that will not fit into memory. * */ @@ -757,10 +757,19 @@ export class TransferManager { * * //- * // Download a large file in chunks utilizing parallel operations. + * // CRC32C validation is performed by default. * //- * const response = await transferManager.downloadFileInChunks(bucket.file('large-file.txt'); * // Your local directory now contains: * // - "large-file.txt" (with the contents from my-bucket.large-file.txt) + * + * //- + * // To disable validation: + * //- + * const responseWithoutValidation = await transferManager.downloadFileInChunks( + * bucket.file('large-file.txt'), + * { validation: false } + * ); * ``` * */ @@ -780,6 +789,9 @@ export class TransferManager { ? this.bucket.file(fileOrName) : fileOrName; + // Default validation to 'crc32c' unless explicitly set to false + const validation = options.validation === false ? false : 'crc32c'; + const fileInfo = await file.get(); const size = parseInt(fileInfo[0].metadata.size!.toString()); // If the file size does not meet the threshold download it as a single chunk. @@ -801,6 +813,7 @@ export class TransferManager { start: chunkStart, end: chunkEnd, [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_SHARDED, + validation: false, // Disable validation on individual chunks }); const result = await fileToWrite.write( resp[0], @@ -823,7 +836,8 @@ export class TransferManager { await fileToWrite.close(); } - if (options.validation === 'crc32c' && fileInfo[0].metadata.crc32c) { + // Check against the defaulted validation option + if (validation === 'crc32c' && fileInfo[0].metadata.crc32c) { const downloadedCrc32C = await CRC32C.fromFile(filePath); if (!downloadedCrc32C.validate(fileInfo[0].metadata.crc32c)) { const mismatchError = new RequestError( @@ -833,6 +847,7 @@ export class TransferManager { throw mismatchError; } } + if (noReturnData) return; return [Buffer.concat(chunks as Buffer[], size)]; } diff --git a/handwritten/storage/test/transfer-manager.ts b/handwritten/storage/test/transfer-manager.ts index 1985f4e751c8..9262995e271d 100644 --- a/handwritten/storage/test/transfer-manager.ts +++ b/handwritten/storage/test/transfer-manager.ts @@ -562,6 +562,7 @@ describe('Transfer Manager', () => { describe('downloadFileInChunks', () => { let file: File; + let fromFileStub: sinon.SinonStub; beforeEach(() => { sandbox.stub(fsp, 'open').resolves({ @@ -569,6 +570,8 @@ describe('Transfer Manager', () => { write: (buffer: unknown) => Promise.resolve({buffer}), } as fsp.FileHandle); + fromFileStub = sandbox.stub(CRC32C, 'fromFile').resolves(new CRC32C(0)); + file = new File(bucket, 'some-large-file'); sandbox.stub(file, 'get').resolves([ { @@ -612,26 +615,48 @@ describe('Transfer Manager', () => { }); it('should call fromFile when validation is set to crc32c', async () => { - let callCount = 0; file.download = () => { return Promise.resolve([Buffer.alloc(0)]) as Promise; }; - CRC32C.fromFile = () => { - callCount++; - return Promise.resolve(new CRC32C(0)); - }; await transferManager.downloadFileInChunks(file, {validation: 'crc32c'}); - assert.strictEqual(callCount, 1); + assert.strictEqual(fromFileStub.callCount, 1); }); - it('should throw an error if crc32c validation fails', async () => { + it('should perform CRC32C validation by default', async () => { + file.download = () => { + return Promise.resolve([Buffer.alloc(0)]) as Promise; + }; + + await transferManager.downloadFileInChunks(file); + assert.strictEqual(fromFileStub.callCount, 1); + }); + + it('should not perform validation when validation is false', async () => { file.download = () => { return Promise.resolve([Buffer.alloc(0)]) as Promise; }; - CRC32C.fromFile = () => { - return Promise.resolve(new CRC32C(1)); // Set non-expected initial value + + await transferManager.downloadFileInChunks(file, {validation: false}); + assert.strictEqual(fromFileStub.callCount, 0); + }); + + it('should disable individual sharded chunk validation in download calls', async () => { + let shardedValidationOption: any = undefined; + sandbox.stub(file, 'download').callsFake(async options => { + shardedValidationOption = (options as DownloadOptions).validation; + return [Buffer.alloc(100)]; + }); + + await transferManager.downloadFileInChunks(file); + assert.strictEqual(shardedValidationOption, false); + }); + + it('should throw an error if crc32c validation fails', async () => { + file.download = () => { + return Promise.resolve([Buffer.alloc(0)]) as Promise; }; + fromFileStub.resolves(new CRC32C(1)); // Set non-expected initial value await assert.rejects( transferManager.downloadFileInChunks(file, {validation: 'crc32c'}), From a9be5d3f80621e1264f4ed9185fe4cb76897e7cb Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 22 May 2026 13:30:35 +0000 Subject: [PATCH 2/3] feat: update validation option to support boolean values and improve default handling logic --- handwritten/storage/src/transfer-manager.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts index 5830fcc68ac6..ebcef4a7b84b 100644 --- a/handwritten/storage/src/transfer-manager.ts +++ b/handwritten/storage/src/transfer-manager.ts @@ -117,7 +117,7 @@ export interface DownloadFileInChunksOptions { concurrencyLimit?: number; chunkSizeBytes?: number; destination?: string; - validation?: 'crc32c' | false; + validation?: 'crc32c' | boolean; noReturnData?: boolean; } @@ -789,8 +789,11 @@ export class TransferManager { ? this.bucket.file(fileOrName) : fileOrName; - // Default validation to 'crc32c' unless explicitly set to false - const validation = options.validation === false ? false : 'crc32c'; + // Default validation to 'crc32c' if undefined or true, otherwise respect user's value + const validation = + options.validation === undefined || options.validation === true + ? 'crc32c' + : options.validation; const fileInfo = await file.get(); const size = parseInt(fileInfo[0].metadata.size!.toString()); From 00b867b8ea3e8d364d676ba6332d037e428a71be Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Fri, 22 May 2026 13:51:18 +0000 Subject: [PATCH 3/3] refactor: update validation property type definition to restrict string input to crc32c --- handwritten/storage/src/transfer-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handwritten/storage/src/transfer-manager.ts b/handwritten/storage/src/transfer-manager.ts index ebcef4a7b84b..3a17e08a3fe4 100644 --- a/handwritten/storage/src/transfer-manager.ts +++ b/handwritten/storage/src/transfer-manager.ts @@ -736,7 +736,7 @@ export class TransferManager { * @property {number} [concurrencyLimit] The number of concurrently executing promises * to use when downloading the file. * @property {number} [chunkSizeBytes] The size in bytes of each chunk to be downloaded. - * @property {string | boolean} [validation] Whether or not to perform a CRC32C validation check when download is complete. Defaults to 'crc32c'. + * @property {'crc32c' | boolean} [validation] Whether or not to perform a CRC32C validation check when download is complete. Defaults to 'crc32c'. * @property {boolean} [noReturnData] Whether or not to return the downloaded data. A `true` value here would be useful for files with a size that will not fit into memory. * */