fix(cfn-include): correctly parse Fn::Sub expressions containing seri…

…alized JSON (aws#14512)

Our logic for parsing `${}` expressions inside `Fn::Sub` incorrectly looked for closing braces everywhere in the string,
instead of doing it to the right of the opening `${`.
This would fail if the string passed to `Fn::Sub` was serialized JSON
(as those would contain closing braces completely unrelated to the `${`).

Fixes aws#14095

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/@aws-cdk/cloudformation-include/test/test-templates/yaml/json-in-fn-sub.yaml

@@ -0,0 +1,26 @@
+AWSTemplateFormatVersion: 2010-09-09
+Parameters:
+  Stage:
+    Type: String
+Resources:
+  Dashboard:
+    Type: AWS::CloudWatch::Dashboard
+    Properties:
+      DashboardName: !Sub ${Stage}-Dashboard
+      DashboardBody: !Sub |
+        {
+            "widgets": [
+                {
+                    "type": "text",
+                    "properties": {
+                        "markdown": "${Stage} ${Stage}"
+                    }
+                },
+                {
+                    "type": "text",
+                    "properties": {
+                        "markdown": "${Stage} ${Stage}"
+                    }
+                }
+            ]
+        }

packages/@aws-cdk/cloudformation-include/test/yaml-templates.test.ts

@@ -1,5 +1,6 @@
 import * as path from 'path';
 import '@aws-cdk/assert-internal/jest';
+import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
 import * as core from '@aws-cdk/core';
 import * as constructs from 'constructs';
 import * as inc from '../lib';
@@ -397,32 +398,59 @@ describe('CDK Include', () => {
     );
   });
 
-  test('can ingest a YAML tempalte with Fn::Sub in string form and output it unchanged', () => {
+  test('can ingest a YAML template with Fn::Sub in string form and output it unchanged', () => {
     includeTestTemplate(stack, 'short-form-fnsub-string.yaml');
 
     expect(stack).toMatchTemplate(
       loadTestFileToJsObject('short-form-fnsub-string.yaml'),
     );
   });
 
-  test('can ingest a YAML tmeplate with Fn::Sub in map form and output it unchanged', () => {
+  test('can ingest a YAML template with Fn::Sub in map form and output it unchanged', () => {
     includeTestTemplate(stack, 'short-form-sub-map.yaml');
 
     expect(stack).toMatchTemplate(
       loadTestFileToJsObject('short-form-sub-map.yaml'),
     );
   });
 
-  test('the parser throws an error on a YAML tmeplate with short form import value that uses short form sub', () => {
+  test('can correctly substitute values inside a string containing JSON passed to Fn::Sub', () => {
+    const cfnInclude = includeTestTemplate(stack, 'json-in-fn-sub.yaml', {
+      Stage: 'test',
+    });
+
+    const dashboard = cfnInclude.getResource('Dashboard') as cloudwatch.CfnDashboard;
+    // we need to resolve the Fn::Sub expression to get to its argument
+    const resolvedDashboardBody = stack.resolve(dashboard.dashboardBody)['Fn::Sub'];
+    expect(JSON.parse(resolvedDashboardBody)).toStrictEqual({
+      "widgets": [
+        {
+          "type": "text",
+          "properties": {
+            "markdown": "test test",
+          },
+        },
+        {
+          "type": "text",
+          "properties": {
+            "markdown": "test test",
+          },
+        },
+      ],
+    });
+  });
+
+  test('the parser throws an error on a YAML template with short form import value that uses short form sub', () => {
     expect(() => {
       includeTestTemplate(stack, 'invalid/short-form-import-sub.yaml');
     }).toThrow(/A node can have at most one tag/);
   });
 });
 
-function includeTestTemplate(scope: constructs.Construct, testTemplate: string): inc.CfnInclude {
+function includeTestTemplate(scope: constructs.Construct, testTemplate: string, parameters?: { [key: string]: string }): inc.CfnInclude {
   return new inc.CfnInclude(scope, 'MyScope', {
     templateFile: _testTemplateFilePath(testTemplate),
+    parameters,
   });
 }
 

packages/@aws-cdk/core/lib/cfn-parse.ts

@@ -687,17 +687,23 @@ export class CfnParser {
 
     function go(value: string): string {
       const leftBrace = value.indexOf('${');
-      const rightBrace = value.indexOf('}') + 1;
-      // don't include left and right braces when searching for the target of the reference
-      if (leftBrace === -1 || leftBrace >= rightBrace) {
+      if (leftBrace === -1) {
+        return value;
+      }
+      // search for the closing brace to the right of the opening '${'
+      // (in theory, there could be other braces in the string,
+      // for example if it represents a JSON object)
+      const rightBrace = value.indexOf('}', leftBrace);
+      if (rightBrace === -1) {
         return value;
       }
 
       const leftHalf = value.substring(0, leftBrace);
-      const rightHalf = value.substring(rightBrace);
-      const refTarget = value.substring(leftBrace + 2, rightBrace - 1).trim();
+      const rightHalf = value.substring(rightBrace + 1);
+      // don't include left and right braces when searching for the target of the reference
+      const refTarget = value.substring(leftBrace + 2, rightBrace).trim();
       if (refTarget[0] === '!') {
-        return value.substring(0, rightBrace) + go(rightHalf);
+        return value.substring(0, rightBrace + 1) + go(rightHalf);
       }
 
       // lookup in map