diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/download.spec.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/download.spec.ts index f427408870..7ed59d7f37 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/download.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/download.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-unsafe-argument */ import { getUrl, GetUrlInput } from '../../../storage-internal'; import { downloadHandler, DownloadHandlerInput } from '../download'; @@ -70,4 +71,15 @@ describe('downloadHandler', () => { message: errorMessage, }); }); + + it('should fail if getUrl does not return a url', async () => { + mockGetUrl.mockResolvedValue({} as any); + + const result = await downloadHandler(baseInput).result; + expect(result).toEqual({ + message: + 'Required keys missing for StorageGetUrlOutput: url.\nObject: {}', + status: 'FAILED', + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/integrity/checkRequiredKeys.spec.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/integrity/checkRequiredKeys.spec.ts new file mode 100644 index 0000000000..eb59091807 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/integrity/checkRequiredKeys.spec.ts @@ -0,0 +1,71 @@ +import { checkRequiredKeys } from '../../integrity'; + +interface TestObject { + key1?: string | null; + key2?: string | null; + key3?: string | null; + extraKey?: string | null; +} + +describe('checkRequiredKeys', () => { + it('should not throw when all required keys are present', () => { + const object: TestObject = { + key1: 'value1', + key2: 'value2', + }; + + expect( + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toBeUndefined(); + }); + + it('should not throw when required keys array is empty', () => { + const object: TestObject = { key1: 'value1', key2: 'value2' }; + + expect(checkRequiredKeys(object, 'test-object', [])).toBeUndefined(); + }); + + it('should not throw when the object has extra keys', () => { + const object: TestObject = { + key1: 'value1', + key2: 'value2', + extraKey: 'extra', + }; + + expect( + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toBeUndefined(); + }); + + it('should throw when a required key is missing', () => { + const object: TestObject = { key1: 'value1', key3: 'value3' }; + + expect(() => + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toThrow(); + }); + + it('should throw when object is empty and required keys are specified', () => { + const object: TestObject = {}; + + expect(() => + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toThrow(); + }); + + it('should throw on null values correctly', () => { + const object: TestObject = { key1: null, key2: 'value2' }; + + expect(() => + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toThrow(); + }); + + it('should throw on undefined values correctly', () => { + const object: TestObject = { key1: undefined, key2: 'value2' }; + + expect(() => + checkRequiredKeys(object, 'test-object', ['key1', 'key2']) + ).toThrow(); + }); +}); diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocationItems.spec.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocationItems.spec.ts index 7689e27319..21707751a9 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocationItems.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocationItems.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-unsafe-argument */ import { list } from '../../../storage-internal'; import { @@ -150,6 +151,33 @@ describe('listLocationItemsHandler', () => { }) ); }); + + it('should throw if list does not provide items', async () => { + mockList.mockResolvedValueOnce({} as any); + await expect(listLocationItemsHandler(baseInput)).rejects.toThrow( + 'Required keys missing for ListOutput: items.' + ); + }); + + ['path', 'lastModified', 'size'].forEach((requiredKey) => { + it(`should throw if any item is missing ${requiredKey}`, async () => { + mockList.mockResolvedValueOnce({ + items: [ + { path: `${prefix}-1`, lastModified: new Date(), size: 0 }, + { + path: `${prefix}-2`, + lastModified: new Date(), + size: 0, + [requiredKey]: undefined, + }, + ], + }); + + await expect(listLocationItemsHandler(baseInput)).rejects.toThrow( + `Required keys missing for ListOutputItem #1: ${requiredKey}.` + ); + }); + }); }); describe('parseResult', () => { diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocations.spec.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocations.spec.ts index 60fd72da48..f31e3c3a2a 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocations.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/listLocations.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-unsafe-argument */ import { listCallerAccessGrants, ListLocationsOutput, @@ -127,4 +128,37 @@ describe('listLocationsHandler', () => { 'Storage Browser: Must provide accountId to `listCallerAccessGrants`.' ); }); + + it('should throw if list does not provide locations', async () => { + input.config.accountId = accountId; + mockListCallerAccessGrants.mockResolvedValueOnce({} as any); + await expect(listLocationsHandler(input)).rejects.toThrow( + 'Required keys missing for ListLocationsOutput: locations.' + ); + }); + + ['scope', 'type', 'permission'].forEach((requiredKey) => { + input.config.accountId = accountId; + it(`should throw if any location is missing ${requiredKey}`, async () => { + mockListCallerAccessGrants.mockResolvedValueOnce({ + locations: [ + { + scope: 's3://bucket/prefix1/*', + permission: 'READWRITE', + type: 'PREFIX', + }, + { + scope: 's3://bucket/prefix1/*', + permission: 'READWRITE', + type: 'PREFIX', + [requiredKey]: undefined, + }, + ], + }); + + await expect(listLocationsHandler(input)).rejects.toThrow( + `Required keys missing for AccessGrantLocation #1: ${requiredKey}.` + ); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/download.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/download.ts index 4942523165..be9ebd5686 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/download.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/download.ts @@ -1,4 +1,5 @@ import { getUrl } from '../../storage-internal'; +import { checkRequiredKeys } from './integrity'; import { TaskData, TaskHandler, @@ -54,7 +55,9 @@ export const downloadHandler: DownloadHandler = ({ expectedBucketOwner: accountId, }, }) - .then(({ url }) => { + .then((result) => { + checkRequiredKeys(result, 'StorageGetUrlOutput', ['url']); + const { url } = result; downloadFromUrl(key, url.toString()); return { status: 'COMPLETE' as const, value: { url } }; }) diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/checkRequiredKeys.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/checkRequiredKeys.ts new file mode 100644 index 0000000000..b2cc7446ef --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/checkRequiredKeys.ts @@ -0,0 +1,17 @@ +export const checkRequiredKeys = ( + objectToCheck: T, + objectName: string, + requiredKeys: Extract[] +): void => { + const missingValues = requiredKeys.filter( + (key) => objectToCheck[key] === undefined || objectToCheck[key] === null + ); + + if (missingValues.length > 0) { + throw Error( + `Required keys missing for ${objectName}: ${[...missingValues].join( + ', ' + )}.\nObject: ${JSON.stringify(objectToCheck)}` + ); + } +}; diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/index.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/index.ts new file mode 100644 index 0000000000..1e9abc95f3 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/integrity/index.ts @@ -0,0 +1 @@ +export * from './checkRequiredKeys'; diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocationItems.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocationItems.ts index 29608d2fc2..baf8d8203e 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocationItems.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocationItems.ts @@ -4,6 +4,7 @@ import { ListPaginateInput, ListOutput, } from '../../storage-internal'; +import { checkRequiredKeys } from './integrity'; import { ListHandler, ListHandlerInput, @@ -90,6 +91,17 @@ export const parseResult = ( prefix ); +const validateResult = (output: ListOutput) => { + checkRequiredKeys(output, `ListOutput`, ['items']); + output.items.forEach((item, i) => + checkRequiredKeys(item, `ListOutputItem #${i}`, [ + 'path', + 'lastModified', + 'size', + ]) + ); +}; + export const listLocationItemsHandler: ListLocationItemsHandler = async ( input ) => { @@ -143,6 +155,8 @@ export const listLocationItemsHandler: ListLocationItemsHandler = async ( }; const output = await list(listInput); + validateResult(output); + nextNextToken = output.nextToken; const items = parseResult(output, prefix); diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocations.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocations.ts index b85ba3c346..d0723f4359 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocations.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/listLocations.ts @@ -1,8 +1,10 @@ import { listCallerAccessGrants, LocationCredentialsProvider, + ListLocationsOutput as ListCallerAccessGrantsOutput, } from '../../storage-internal'; import { assertAccountId } from '../../validators'; +import { checkRequiredKeys } from './integrity'; import { ListHandlerOptions, @@ -53,6 +55,17 @@ export interface ListLocationsHandlerOutput { export interface ListLocationsHandler extends ListHandler {} +const validateResult = (output: ListCallerAccessGrantsOutput) => { + checkRequiredKeys(output, 'ListLocationsOutput', ['locations']); + output.locations.forEach((location, i) => + checkRequiredKeys(location, `AccessGrantLocation #${i}`, [ + 'scope', + 'type', + 'permission', + ]) + ); +}; + export const listLocationsHandler: ListLocationsHandler = async (input) => { const { config, options } = input; const { accountId, credentials, customEndpoint, region } = config; @@ -74,6 +87,7 @@ export const listLocationsHandler: ListLocationsHandler = async (input) => { pageSize: remainingPageSize, region, }); + validateResult(output); const parsedOutput = getFilteredLocations(output.locations, exclude);