Skip to content

Commit 67b85f2

Browse files
authored
fix(s3-deployment): Source.jsonData() fails with null JSON values (#36054)
In cdk-lib aws-s3-deployment, creating a Source.jsonData() will fail if the JSON has any null fields in it due to `escapeTokens` trying to build an object from it, resulting in `TypeError: Cannot convert undefined or null to object` ### Issue # (if applicable) Closes #36052. ### Reason for this change Can't create a Source.jsonData() when `null` values are present in the JSON data itself ### Description of changes Added a null check around `escapeTokens()` to prevent it from trying to process `null` values ### Describe any new or updated permissions being added N/A ### Description of how you validated changes Tested by hand via modifying the compiled JS in my project to add the check, and was able to successfully upload a JSON file to S3 with a null value contained inside. Also added coverage via unit tests/integration tests for the PR build to chew through. ### 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*
1 parent 56f22aa commit 67b85f2

File tree

11 files changed

+1839
-206
lines changed

11 files changed

+1839
-206
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/asset.8de7dc173740857dd772ca214c7ac8381483f7ee406d94fb9ff9e95ad407d876/my-json/config-with-null.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1685 additions & 173 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/integtestbucketdeploymentdataDefaultTestDeployAssert6FF3075D.assets.json

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/integtestbucketdeploymentdataDefaultTestDeployAssert6FF3075D.template.json

Lines changed: 53 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/manifest.json

Lines changed: 50 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/test-bucket-deployment-data.assets.json

Lines changed: 17 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/test-bucket-deployment-data.template.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@
218218
{
219219
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
220220
},
221+
{
222+
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
223+
},
221224
{
222225
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
223226
}
@@ -230,7 +233,8 @@
230233
"4ba8cfb53da242e91a423f3333c95db70ac0b5c2481a2d7e5c51142e5738403f.zip",
231234
"4458891c23c9754723db261b607985ee4fce5a087343fca93e9608fb49e083ba.zip",
232235
"c3f91ed893cc759513d80ebd56bcaae5061b08bff3dd8ad10daeb62558bac52e.zip",
233-
"10cb9bf43614add407e699e4af8e1ba4a9789363f48fa816662b15885c354e1a.zip"
236+
"10cb9bf43614add407e699e4af8e1ba4a9789363f48fa816662b15885c354e1a.zip",
237+
"8de7dc173740857dd772ca214c7ac8381483f7ee406d94fb9ff9e95ad407d876.zip"
234238
],
235239
"SourceMarkers": [
236240
{},
@@ -334,6 +338,7 @@
334338
]
335339
}
336340
},
341+
{},
337342
{}
338343
],
339344
"SourceMarkersConfig": [
@@ -346,6 +351,7 @@
346351
"jsonEscape": true
347352
},
348353
{},
354+
{},
349355
{}
350356
],
351357
"DestinationBucketName": {

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.js.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ class TestBucketDeploymentData extends Stack {
6464
// Test empty string handling
6565
const file8 = Source.data('file8.txt', '');
6666

67+
// Test null JSON data value
68+
const file9 = Source.jsonData('my-json/config-with-null.json', { hello: 'there', goodbye: null });
69+
6770
const deployment = new BucketDeployment(this, 'DeployWithDataSources', {
6871
destinationBucket: this.bucket,
6972
sources: [file1, file2],
@@ -77,6 +80,7 @@ class TestBucketDeploymentData extends Stack {
7780
deployment.addSource(file6);
7881
deployment.addSource(file7);
7982
deployment.addSource(file8);
83+
deployment.addSource(file9);
8084

8185
new CfnOutput(this, 'BucketName', { value: this.bucket.bucketName });
8286
}
@@ -105,6 +109,17 @@ assertionProvider.expect(ExpectedResult.objectLike({
105109
Body: '{"secret_value":"test\\"with\\"quotes"}',
106110
}));
107111

112+
// Assert that JSON data with a null value is represented properly
113+
const jsonNullAssertionProvider = integTest.assertions.awsApiCall('S3', 'getObject', {
114+
Bucket: testCase.bucket.bucketName,
115+
Key: path.join('deploy/here', 'my-json/config-with-null.json'),
116+
});
117+
118+
// Verify the content is valid JSON and both null and non-null fields are present
119+
jsonNullAssertionProvider.expect(ExpectedResult.objectLike({
120+
Body: '{"hello":"there","goodbye":null}',
121+
}));
122+
108123
// Add assertions to verify the YAML file
109124
const yamlAssertionProvider = integTest.assertions.awsApiCall('S3', 'getObject', {
110125
Bucket: testCase.bucket.bucketName,

packages/aws-cdk-lib/aws-s3-deployment/lib/source.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ export class Source {
280280
return obj.map(v => Source.escapeTokens(scope, v));
281281
}
282282

283-
if (typeof obj === 'object') {
283+
if (obj !== null && typeof obj === 'object') {
284284
return Object.fromEntries(
285285
Object.entries(obj).map(([key, value]) => {
286286
// As JSON keys are always strings, keys are assumed to be either regular strings or string tokens.

0 commit comments

Comments
 (0)