From 3db16d9d414b557dd909230f5ce6f67ac278ec53 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 25 Jul 2018 15:47:25 +0300 Subject: [PATCH 1/2] token-aware-jsonify: Stringify resolved tokens `token-aware-jsonify` is expected to return a stringified JSON object. This means that any string within the object must be escaped (e.g. not include "\n" or quotes). The function currently stringifies the primary string but this still leaves room for non-allowed characters in the resolved tokens. For example, if a token resolves to `{ "Fn::Join": [ "", [ "Hello,\nWorld!" ] ] }` then the deploy-time value will be "Hello,\nWorld!" which must be represented in JSON as "Hello,\\nWorld!". This change eagerly stringifies all string values in the resolved tokens. Theoretically this might cause trouble in the case where token strings have special characters AND not emitted, but this seems like a long shot and we optimize for the common case. --- .../lib/cloudformation/token-aware-jsonify.ts | 46 +++++++++++++++---- .../cdk/test/test.token-aware-jsonify.ts | 17 ++++++- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/token-aware-jsonify.ts b/packages/@aws-cdk/cdk/lib/cloudformation/token-aware-jsonify.ts index dac05570ab008..af36eaad9c4c9 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/token-aware-jsonify.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/token-aware-jsonify.ts @@ -4,14 +4,21 @@ import { FnSub } from './fn'; /** * Jsonify a deep structure to a string while preserving tokens * - * Sometimes we have JSON structures that contain CloudFormation - * intrinsics like { Ref } and { Fn::GetAtt }, but the model requires - * that we stringify the JSON structure and pass it into the parameter. + * Sometimes we have JSON structures that contain CloudFormation intrinsics like + * { Ref } and { Fn::GetAtt }, but the model requires that we stringify the JSON + * structure and pass it into the parameter. * * Doing this makes it so that CloudFormation does not resolve the intrinsics - * anymore, since it does not look into every string. To resolve this, - * we stringify into a string and put placeholders in wich we substitute - * with the resolved references using { Fn::Sub }. + * anymore, since it does not look into every string. To resolve this, we + * stringify into a string and put placeholders in wich we substitute with the + * resolved references using { Fn::Sub }. + * + * Since the result is expected to be a stringified JSON, we need to make sure + * any textual values resolved from tokens are also stringified, so we also + * stringify any string values in resolved tokens (for example, "\n" will be + * replaced by "\\n", quotes will be escaped, etc). This might not be needed (or + * even could be harmful) for certain tokens (e.g. Fn::GetAtt), but we prefer to + * make the common case fool-proof, and hope for the best. * * Will only work correctly for intrinsics that return a string value. */ @@ -30,12 +37,35 @@ export function tokenAwareJsonify(structure: any): any { const tokenId: {[key: string]: string} = {}; const substitutionMap: {[key: string]: any} = {}; + function stringifyStrings(x: any): any { + if (typeof(x) === 'string') { + const jsonS = JSON.stringify(x); + return jsonS.substr(1, jsonS.length - 2); // trim quotes + } + + if (Array.isArray(x)) { + return x.map(stringifyStrings); + } + + if (typeof(x) === 'object') { + const result: any = {}; + for (const key of Object.keys(x)) { + result[key] = stringifyStrings(x[key]); + } + + return result; + } + + return x; + } + function rememberToken(x: Token) { // Get a representation of the resolved Token that we can use as a hash key. - const reprKey = JSON.stringify(resolve(x)); + const resolved = resolve(x); + const reprKey = JSON.stringify(resolved); if (!(reprKey in tokenId)) { tokenId[reprKey] = `ref${counter}`; - substitutionMap[tokenId[reprKey]] = x; + substitutionMap[tokenId[reprKey]] = stringifyStrings(resolved); counter += 1; } return `<<>>`; diff --git a/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts b/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts index 5898dcabd0312..4896710db5b23 100644 --- a/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts +++ b/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts @@ -1,5 +1,5 @@ import { Test } from 'nodeunit'; -import { AwsRegion, resolve, tokenAwareJsonify } from '../lib'; +import { AwsRegion, resolve, tokenAwareJsonify, FnConcat } from '../lib'; export = { 'substitutes tokens'(test: Test) { @@ -56,4 +56,19 @@ export = { test.done(); }, + + 'string values in resolved tokens should be represented as stringified strings'(test: Test) { + // WHEN + const result = tokenAwareJsonify({ + test1: new FnConcat('Hello', 'This\nIs', 'Very "cool"'), + }); + + // THEN + test.deepEqual(resolve(result), { 'Fn::Sub': + [ '{"test1":"${ref0}"}', + { ref0: + { 'Fn::Join': [ '', [ 'Hello', 'This\\nIs', 'Very \\"cool\\"' ] ] } } ] }); + + test.done(); + } }; From e3a2b41514f92388f253e9acd25247c7407a74ac Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 25 Jul 2018 16:18:55 +0300 Subject: [PATCH 2/2] Reorder imports --- packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts b/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts index 4896710db5b23..1608c774bb8f1 100644 --- a/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts +++ b/packages/@aws-cdk/cdk/test/test.token-aware-jsonify.ts @@ -1,5 +1,5 @@ import { Test } from 'nodeunit'; -import { AwsRegion, resolve, tokenAwareJsonify, FnConcat } from '../lib'; +import { AwsRegion, FnConcat, resolve, tokenAwareJsonify } from '../lib'; export = { 'substitutes tokens'(test: Test) {