Skip to content

Commit ba3d3b2

Browse files
committed
fix(storage): omit unserializable properties when hash uploadDataOptions
1 parent 9e956f5 commit ba3d3b2

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

packages/storage/__tests__/providers/s3/apis/internal/uploadData/multipartHandlers.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ const region = 'region';
4545
const defaultKey = 'key';
4646
const defaultContentType = 'application/octet-stream';
4747
const defaultCacheKey =
48-
'Jz3O2w==_8388608_application/octet-stream_bucket_public_key';
48+
'o6a/Qw==_8388608_application/octet-stream_bucket_public_key';
4949
const testPath = 'testPath/object';
50-
const testPathCacheKey = `Jz3O2w==_8388608_${defaultContentType}_${bucket}_custom_${testPath}`;
50+
const testPathCacheKey = `o6a/Qw==_8388608_${defaultContentType}_${bucket}_custom_${testPath}`;
5151

5252
const mockCreateMultipartUpload = jest.mocked(createMultipartUpload);
5353
const mockUploadPart = jest.mocked(uploadPart);
@@ -545,14 +545,14 @@ describe('getMultipartUploadHandlers with key', () => {
545545
describe('cache validation', () => {
546546
it.each([
547547
{
548-
name: 'wrong part count',
548+
name: 'mismatch part count between cached upload and actual upload',
549549
parts: [{ PartNumber: 1 }, { PartNumber: 2 }, { PartNumber: 3 }],
550550
},
551551
{
552-
name: 'wrong part numbers',
552+
name: 'mismatched part numbers between cached upload and actual upload',
553553
parts: [{ PartNumber: 1 }, { PartNumber: 1 }],
554554
},
555-
])('should throw with $name', async ({ parts }) => {
555+
])('should throw integrity error with $name', async ({ parts }) => {
556556
mockMultipartUploadSuccess();
557557

558558
const mockDefaultStorage = defaultStorage as jest.Mocked<
@@ -693,7 +693,7 @@ describe('getMultipartUploadHandlers with key', () => {
693693
expect(Object.keys(cacheValue)).toEqual([
694694
expect.stringMatching(
695695
// \d{13} is the file lastModified property of a file
696-
/someName_\d{13}_Jz3O2w==_8388608_application\/octet-stream_bucket_public_key/,
696+
/someName_\d{13}_o6a\/Qw==_8388608_application\/octet-stream_bucket_public_key/,
697697
),
698698
]);
699699
});

packages/storage/src/providers/s3/apis/internal/uploadData/multipart/uploadCache.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { UPLOADS_STORAGE_KEY } from '../../../../utils/constants';
1010
import { ResolvedS3Config } from '../../../../types/options';
1111
import { Part, listParts } from '../../../../utils/client/s3data';
1212
import { logger } from '../../../../../../utils';
13+
// TODO: Remove this interface when we move to public advanced APIs.
14+
import { UploadDataInput as UploadDataWithPathInputWithAdvancedOptions } from '../../../../../../internals/types/inputs';
1315

1416
const ONE_HOUR = 1000 * 60 * 60;
1517

@@ -96,6 +98,28 @@ const listCachedUploadTasks = async (
9698
}
9799
};
98100

101+
/**
102+
* Serialize the uploadData API options to string so it can be hashed.
103+
*/
104+
export const serializeUploadOptions = (
105+
options: UploadDataWithPathInputWithAdvancedOptions['options'] & {
106+
resumableUploadsCache?: KeyValueStorageInterface;
107+
} = {},
108+
): string => {
109+
const unserializableOptionProperties: string[] = [
110+
'onProgress',
111+
'resumableUploadsCache', // Internally injected implementation not set by customers
112+
'locationCredentialsProvider', // Internally injected implementation not set by customers
113+
] satisfies (keyof typeof options)[];
114+
const serializableOptions = Object.fromEntries(
115+
Object.entries(options).filter(
116+
([key]) => !unserializableOptionProperties.includes(key),
117+
),
118+
);
119+
120+
return JSON.stringify(serializableOptions);
121+
};
122+
99123
interface UploadsCacheKeyOptions {
100124
size: number;
101125
contentType?: string;

packages/storage/src/providers/s3/apis/internal/uploadData/multipart/uploadHandlers.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ import { StorageOperationOptionsInput } from '../../../../../../types/inputs';
4040
import { IntegrityError } from '../../../../../../errors/IntegrityError';
4141

4242
import { uploadPartExecutor } from './uploadPartExecutor';
43-
import { getUploadsCacheKey, removeCachedUpload } from './uploadCache';
43+
import {
44+
getUploadsCacheKey,
45+
removeCachedUpload,
46+
serializeUploadOptions,
47+
} from './uploadCache';
4448
import { getConcurrentUploadsProgressTracker } from './progressTracker';
4549
import { loadOrCreateMultipartUpload } from './initialUpload';
4650
import { getDataChunker } from './getDataChunker';
@@ -151,9 +155,9 @@ export const getMultipartUploadHandlers = (
151155
resolvedAccessLevel = resolveAccessLevel(accessLevel);
152156
}
153157

154-
const optionsHash = (
155-
await calculateContentCRC32(JSON.stringify(uploadDataOptions))
156-
).checksum;
158+
const { checksum: optionsHash } = await calculateContentCRC32(
159+
serializeUploadOptions(uploadDataOptions),
160+
);
157161

158162
if (!inProgressUpload) {
159163
const { uploadId, cachedParts, finalCrc32 } =

0 commit comments

Comments
 (0)