(core): Fn::Split delimiter is escaped when stringified

[BenChaimberg]

Consider a Fn::Split intrinsic that is encoded as JSON. For example, a Split that is used in the definition of a State Machine or in the arguments to a Custom Resource. If the Split delimiter contains some JSON-reserved characters (such as " or \), the token-aware stringification still quotes the delimiter as if it were a true string literal. This means that values originating in CloudFormation (such as CfnParameters or CfnWaitConditions) cannot be "parsed" using JSON-reserved characters.

Reproduction Steps

import { CfnOutput, CfnParameter, Construct, Fn, Stack, StackProps } from '@aws-cdk/core';

export class CdkStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const parameter = new CfnParameter(this, 'Parameter', {
      default: 'abc"def"ghi'
    });
    const split = Fn.split('"', parameter.valueAsString);
    const select = Fn.select(1, split);
    const encodedString = this.toJsonString(select);
    new CfnOutput(this, 'Output', {
      value: encodedString,
    });
  }
}

What did you expect to happen?

Stack successfully deploys, produces output with value "def".

What actually happened?

Stack fails to deploy, produces CloudFormation error:

Template error: Fn::Select cannot select nonexistent value at index 1

Environment

• CDK CLI Version : 1.102.0 (build a75d52f)
• Framework Version: 1.102.0
• Node.js Version: v15.3.0
• OS : macOS 10.15.7
• Language (Version): TypeScript (3.9.9)

Other

Switching the " in both the parameter and the split delimiter to some other character such as : produces the correct result.

The concrete example that provided the impetus for this bug report was due to #13074 which modified custom resources to accept their Create/Update/Delete parameters as JSON-encoded strings. Previously, we were using a CfnWaitCondition to allow the deployer to manually create a WorkDocs site in the middle of stack deployment (since there is no API or CFN resource to do so). Then, the response sent to CloudFormation to signal successful creation was used in other WorkDocs resources (such as users and folders) to both populate the site organization ID, as well as set an implicit CloudFormation dependency on the creation of the site. Because the signal is presented to the template as a JSON object (see the very bottom of this documentation page), some string parsing within the template was necessary to grab the ID from the response. We simply used the Split and Select intrinsics to separate the data into a list of strings and get the ID. The templates that are synthesized before and after #13074 show how the delimiter is escaped from "\" to "\\\"", while the value cannot be similarly escaped since it itself is an intrinsic. The CfnWaitCondition logical ID is ActiveDirectoryWorkDocs8BACF9E0, and an example of the value returned from {Fn::GetAtt: ["ActiveDirectoryWorkDocs8BACF9E0", "Data"]} is {"UniqueId123":"d-83630ac3db"}

Before:

"Create": {                                                                                                                                                                          
  "service": "WorkDocs",                                                                                                                                                             
  "action": "createUser",                                                                                                                                                            
  "parameters": {                                                                                                                                                                    
    "OrganizationId": {                                                                                                                                                              
      "Fn::Select": [                                                                                                                                                                
        3,                                                                                                                                                                           
        {                                                                                                                                                                            
          "Fn::Split": [                                                                                                                                                             
            "\"",                                                                                                                                                                    
            {                                                                                                                                                                        
              "Fn::GetAtt": [                                                                                                                                                        
                "ActiveDirectoryWorkDocs8BACF9E0",                                                                                                                                   
                "Data"                                                                                                                                                               
              ]                                                                                                                                                                      
            }                                                                                                                                                                        
          ]                                                                                                                                                                          
        }                                                                                                                                                                            
      ]                                                                                                                                                                              
    },                                                                                                                                                                               
    "Username": "username",                                                                                                                                                      
    "Password": "<omitted>",                                                                                                                                                 
    "GivenName": "First",                                                                                                                                                              
    "Surname": "Last",                                                                                                                                                           
    "StorageRule": {                                                                                                                                                                 
      "StorageType": "UNLIMITED"                                                                                                                                                     
    }                                                                                                                                                                                
  },                                                                                                                                                                                 
  "physicalResourceId": {                                                                                                                                                            
    "responsePath": "User.Id"                                                                                                                                                        
  }                                                                                                                                                                                  
},

After:

"Create": {
  "Fn::Join": [
    "",
    [
      "{\"service\":\"WorkDocs\",\"action\":\"createUser\",\"parameters\":{\"OrganizationId\":\"",
      {
        "Fn::Select": [
          3,
          {
            "Fn::Split": [
              "\\\"",
              {
                "Fn::GetAtt": [
                  "ActiveDirectoryWorkDocs8BACF9E0",
                  "Data"
                ]
              }
            ]
          }
        ]
      },
      "\",\"Username\":\"username\",\"Password\":\"<omitted>\",\"GivenName\":\"First\",\"Surname\":\"Last\",\"StorageRule\":{\"StorageType\":\"UNLIMITED\"}},\"physicalResourceId\":\
{\"responsePath\":\"User.Id\"}}"
    ]
  ]
},

---

This is :bug: Bug Report

[comcalvi]

working example

Outputs:
  Output:
    Value:
      Fn::Join:
        - ""
        - - '"'
          - Fn::Select:
              - 1
              - Fn::Split:
                  - a
                  - Ref: Parameter
          - '"'

failing:

Outputs:
  Output:
    Value:
      Fn::Join:
        - ""
        - - '"'
          - Fn::Select:
              - 1
              - Fn::Split:
                  - \"
                  - Ref: Parameter
          - '"'

json form of failure:

         "Fn::Split": [
          "\\\"",
          {
           "Ref": "Parameter"
          }
         ]

[comcalvi]

weirdly, this fails to deploy from the CDK:

 "Outputs": {
  "Output": {
   "Value": {
    "Fn::Join": [
     "",
     [
      "\"",
      {
       "Fn::Select": [
        1,
        {
         "Fn::Split": [
          "\"",
          {
           "Ref": "Parameter"
          }
         ]
        }
       ]
      },
      "\""
     ]
    ]
   }
  }
 },

but succeeds when manually uploaded as a component of a template to CFN.

[mrgrain]

Further investigation reveals that the nested intrinsic functions are not evaluated as intrinsics, but treated as regular data objects.

Particularly here and here, instead of recursing into the quote helper, a intrinsic should be detected and treated accordingly.

However this revealed another edge case. In the provided reproduction, Ref: Parameter can actually not be safely converted to a JSON string as it might contain a character that needs escaping. Both examples return the invalid JSON string "abc"def"ghi"

Full example:

AWSTemplateFormatVersion: 2010-09-09
Resources:
  NullResource:
    Type: 'AWS::CloudFormation::WaitConditionHandle'
Outputs:
  Output:
    Value: !Join 
      - ''
      - - '"'
        - !Select 
          - 1
          - !Split 
            - '"'
            - !Ref Parameter
        - '"'
Parameters:
  Parameter:
    Type: String
    Default: abc"d\e\f"ghi

Simplified example:

import { CfnOutput, CfnParameter, Construct, Fn, Stack, StackProps } from '@aws-cdk/core';

export class CdkStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const parameter = new CfnParameter(this, 'Parameter', {
      default: 'abc"def"ghi'
    });
    const encodedString = this.toJsonString(select);
    new CfnOutput(this, 'Output', {
      value: encodedString,
    });
  }
}

AWSTemplateFormatVersion: 2010-09-09
Resources:
  NullResource:
    Type: 'AWS::CloudFormation::WaitConditionHandle'
Outputs:
  Output:
    Value: !Join 
      - ''
      - - '"'
        - !Ref Parameter
        - '"'
Parameters:
  Parameter:
    Type: String
    Default: abc"d\e\f"ghi

---

While this might not be a huge problem, it proves we cannot reliably produce a JSON string locally for any unresolvable Reference (Ref and Fn::GetAtt).

[mrgrain]

Going past the general inconsistency, and accepting for a second that References are not always correct. The issue here appears to be that any nested Intrinsic functions are not treated as Intrinsic functions, but as regular objects (for which the quoting would be correct). For these we should quote strings differently, depending on the contents.

[mrgrain]

Update: Not actively working on this.

Basically the general problem cannot be solved locally since the values are not going to be available locally, only in the cloud. A specific fix for this issue would likely cause the issue to just flip around.

A good solution could be using the new Fn::ToJsonString extension but there are issues preventing us from using it: https://github.com/aws-cloudformation/cfn-language-discussion/issues/105 https://github.com/aws-cloudformation/cfn-language-discussion/issues/103