Skip to content

Commit 9c8f549

Browse files
authored
chore: hide diffs of mangled unicode strings (#25912)
I am reopening this from #25525 and following up on my comments here: #24557 (comment) #24557 (comment) #25008 (comment) #25008 (comment) #25008 (comment) #25008 (comment) #25008 (comment) #25008 (comment) #25525 (comment) #25525 (comment) 🫠 #25525 (comment) 🫠 --- Fixes #25309 Fixes #22203 Fixes #20212 Fixes #13634 Fixes #10523 Fixes #10219 See also: aws-cloudformation/cloudformation-coverage-roadmap#1220 See also: aws-cloudformation/cloudformation-coverage-roadmap#814 --- 👻 I have retitled this PR as a `chore` instead of a `fix` because @aws-cdk-automation keeps closing my PRs as abandoned even though they are clearly not abandoned. > This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. --- @otaviomacedo @rix0rrr @TheRealAmazonKendra - I'm happy to adjust the approach, add more tests, or do what else needs to be done. I'm not getting any feedback from the team so I'm not sure how to proceed. The diff noise with non-ASCII information in cdk diff makes it difficult to find meaningful changes to our stacks. 🗿🗞️📬 **Crucially, this change only affects the CLI output and therefore an integration test isn't possible.** --- CloudFormation's `GetStackTemplate` irrecoverably mangles any character not in the 7-bit ASCII range. This causes noisy output from `cdk diff` when a template contains non-English languages or emoji. We can detect this case and consider these strings equal. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Many AWS services accept non-ASCII input, eg many "description" fields. CloudFormation will correctly dispatch these templates but when invoking `GetStackTemplate` the result is mangled. This causes annoying noise in the output of `cdk diff`: ``` Resources [~] AWS::Lambda::Function Lambda/Resource └─ [~] Description ├─ [-] ????? └─ [+] 🤦🏻‍♂️ ``` This change modifies the diff algorithm to consider the string equal if the lvalue is a mangled version of the rvalue. Of course this runs the risk of hiding changesets which modify only a single non-ASCII character to another non-ASCII character, but these fields already tend to be informative in nature.
1 parent 28df618 commit 9c8f549

File tree

5 files changed

+39
-3
lines changed

5 files changed

+39
-3
lines changed

packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts

+12
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,18 @@ export function unionOf(lv: string[] | Set<string>, rv: string[] | Set<string>):
134134
return new Array(...result);
135135
}
136136

137+
/**
138+
* GetStackTemplate flattens any codepoint greater than "\u7f" to "?". This is
139+
* true even for codepoints in the supplemental planes which are represented
140+
* in JS as surrogate pairs, all the way up to "\u{10ffff}".
141+
*
142+
* This function implements the same mangling in order to provide diagnostic
143+
* information in `cdk diff`.
144+
*/
145+
export function mangleLikeCloudFormation(payload: string) {
146+
return payload.replace(/[\u{80}-\u{10ffff}]/gu, '?');
147+
}
148+
137149
/**
138150
* A parseFloat implementation that does the right thing for
139151
* strings like '0.0.0'
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
export * from './diff-template';
22
export * from './format';
33
export * from './format-table';
4-
export { deepEqual } from './diff/util';
4+
export { deepEqual, mangleLikeCloudFormation } from './diff/util';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { mangleLikeCloudFormation } from '../lib/diff/util';
2+
3+
test('mangled strings', () => {
4+
expect(mangleLikeCloudFormation('foo')).toEqual('foo');
5+
expect(mangleLikeCloudFormation('文字化け')).toEqual('????');
6+
expect(mangleLikeCloudFormation('🤦🏻‍♂️')).toEqual('?????');
7+
expect(mangleLikeCloudFormation('\u{10ffff}')).toEqual('?');
8+
expect(mangleLikeCloudFormation('\u007f')).toEqual('\u007f');
9+
expect(mangleLikeCloudFormation('\u0080')).toEqual('?');
10+
});

packages/aws-cdk/lib/cli.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ async function parseCommandLineArguments(args: string[]) {
257257
.option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only diff requested stacks, don\'t include dependencies' })
258258
.option('context-lines', { type: 'number', desc: 'Number of context lines to include in arbitrary JSON diff rendering', default: 3, requiresArg: true })
259259
.option('template', { type: 'string', desc: 'The path to the CloudFormation template to compare with', requiresArg: true })
260-
.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources', default: false })
260+
.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources or mangled non-ASCII characters', default: false })
261261
.option('security-only', { type: 'boolean', desc: 'Only diff for broadened security changes', default: false })
262262
.option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff' })
263263
.option('processed', { type: 'boolean', desc: 'Whether to compare against the template with Transforms already processed', default: false }))

packages/aws-cdk/lib/diff.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,18 @@ export function printStackDiff(
2121
context: number,
2222
stream?: cfnDiff.FormatStream): number {
2323

24-
const diff = cfnDiff.diffTemplate(oldTemplate, newTemplate.template);
24+
let diff = cfnDiff.diffTemplate(oldTemplate, newTemplate.template);
25+
26+
// detect and filter out mangled characters from the diff
27+
let filteredChangesCount = 0;
28+
if (diff.differenceCount && !strict) {
29+
const mangledNewTemplate = JSON.parse(cfnDiff.mangleLikeCloudFormation(JSON.stringify(newTemplate.template)));
30+
const mangledDiff = cfnDiff.diffTemplate(oldTemplate, mangledNewTemplate);
31+
filteredChangesCount = Math.max(0, diff.differenceCount - mangledDiff.differenceCount);
32+
if (filteredChangesCount > 0) {
33+
diff = mangledDiff;
34+
}
35+
}
2536

2637
// filter out 'AWS::CDK::Metadata' resources from the template
2738
if (diff.resources && !strict) {
@@ -41,6 +52,9 @@ export function printStackDiff(
4152
} else {
4253
print(chalk.green('There were no differences'));
4354
}
55+
if (filteredChangesCount > 0) {
56+
print(chalk.yellow(`Omitted ${filteredChangesCount} changes because they are likely mangled non-ASCII characters. Use --strict to print them.`));
57+
}
4458

4559
return diff.differenceCount;
4660
}

0 commit comments

Comments
 (0)