Skip to content

Commit

Permalink
fix(s3-assets): throw if path property is empty (#29425)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #29410.

### Reason for this change

It was reported that a `Code.fromAsset('')` was creating an infinite loop by including itself through `cdk.out`. This is caused by the following line:

https://github.com/aws/aws-cdk/blob/730fe63efc461c14f6e2b4aa9206c10f9b0f4cd9/packages/aws-cdk-lib/aws-s3-assets/lib/asset.ts#L145

If an empty string is given to `path.resolve()`, the current working directory is returned.

### Description of changes

I've added a check that verifies that the given `path` property is not empty.

### Description of how you validated changes

I've added a test for both the `aws-lambda` package, where the issue was originally reported, and `aws-s3-assets`, where the fix was implemented

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
nmussy committed Apr 4, 2024
1 parent 069013e commit 2814011
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
14 changes: 14 additions & 0 deletions packages/aws-cdk-lib/aws-lambda/test/code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ describe('code', () => {
});

describe('lambda.Code.fromAsset', () => {
test('fails if path is empty', () => {
// GIVEN
const fileAsset = lambda.Code.fromAsset('');

// THEN
expect(() => defineFunction(fileAsset)).toThrow(/Asset path cannot be empty/);
});
test('fails if path does not exist', () => {
// GIVEN
const fileAsset = lambda.Code.fromAsset('/path/not/found/' + Math.random() * 999999);

// THEN
expect(() => defineFunction(fileAsset)).toThrow(/Cannot find asset/);
});
test('fails if a non-zip asset is used', () => {
// GIVEN
const fileAsset = lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler', 'index.py'));
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/aws-s3-assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ export class Asset extends Construct implements cdk.IAsset {
constructor(scope: Construct, id: string, props: AssetProps) {
super(scope, id);

if (!props.path) {
throw new Error('Asset path cannot be empty');
}

this.isBundled = props.bundling != null;

// stage the asset source (conditionally).
Expand Down
9 changes: 8 additions & 1 deletion packages/aws-cdk-lib/aws-s3-assets/test/asset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,18 @@ test('"readers" or "grantRead" can be used to grant read permissions on the asse
});
});

test('fails if path is empty', () => {
const stack = new cdk.Stack();
expect(() => new Asset(stack, 'MyDirectory', {
path: '',
})).toThrow(/Asset path cannot be empty/);
});

test('fails if directory not found', () => {
const stack = new cdk.Stack();
expect(() => new Asset(stack, 'MyDirectory', {
path: '/path/not/found/' + Math.random() * 999999,
})).toThrow();
})).toThrow(/Cannot find asset/);
});

test('multiple assets under the same parent', () => {
Expand Down

0 comments on commit 2814011

Please sign in to comment.