fix(s3-deployment): handle empty string in Source.data() #35818
Conversation
- Add explicit handling for empty string input in renderData - Prevent potential errors when processing empty string data - Add regression test for empty string Source.data scenario - Resolve issue aws#35809 related to empty string data handling
There was a problem hiding this comment.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.
✅ A exemption request has been requested. Please wait for a maintainer's review.
|
Exemption Request - we have unit tests to cover the edge case handling. |
aws-cdk-automation
added
the
pr-linter/exemption-requested
abidhasan-aws
left a comment
abidhasan-aws
left a comment
There was a problem hiding this comment.
We should additionally add the case for empty string in the integ test as well.
integ.bucket-deployment-data.ts has different use cases of Source.data. we can add another one with empty string.
Thanks for the quick pr :)
| }, | ||
| }); | ||
| }); | ||
| test('empty string data', () => { |
There was a problem hiding this comment.
we can rename this to be more specific: 'renderData can render empty string'
| '<<marker:0xbaba:0>>': token, | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
lint: should add an blank line here.
| }); | ||
| }); | ||
|
|
||
| test('Source.data with empty string - issue #35809 regression test', () => { |
There was a problem hiding this comment.
lets rename this: 'Source.data does not throw error for empty string'
|
Closing this in favour of #35824 |
|
Comments on closed issues and PRs are hard for our team to see. |
4 participants
Issue # (if applicable)
Closes #35809.
Reason for this change
Source.data()fails with a TypeError when called with an empty string as the data parameter in CDK v2.207.0+. This is a regression from v2.206.0 where empty string deployment worked correctly.The issue manifests as:
This prevents users from deploying empty S3 objects using
Source.data("path", ""), which is a valid use case for placeholder files, configuration templates, or initialization markers.Description of changes
Fixed the empty string handling issue by adding explicit empty string handling in the S3 deployment
renderData()function. This targeted approach avoids modifying core CDK libraries and keeps the fix scoped to where the issue manifests.Root cause: When
Source.data("path", "")is processed during synthesis, the empty string goes through therenderData("")function. For empty strings,TokenizedStringFragmentshas zero fragments, causingjoin()to callStringConcat.join(undefined, undefined), which returnsundefinedinstead of the expected empty string. Thisundefinedvalue eventually reachesfs.writeFileSync(), causing the TypeError.Technical changes:
renderData()inpackages/aws-cdk-lib/aws-s3-deployment/lib/render-data.tsif (data === '') return { text: '', markers: {} }Source.data('path/to/empty', '')Impact: Restores v2.206.0 behavior for empty string deployment while maintaining a minimal, targeted fix with no risk to other CDK functionality.
Describe any new or updated permissions being added
N/A - No IAM permissions or resource access changes. This is an internal S3 deployment module fix that only affects synthesis-time data rendering for empty strings.
Description of how you validated changes
renderData()functionSource.data('path/to/empty', '')inpackages/aws-cdk-lib/aws-s3-deployment/test/content.test.tsSource.data("empty.txt", "")works without throwing TypeErrorChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license