From 48efa249a62df4fbf34d544a2f63ccd87d7aa211 Mon Sep 17 00:00:00 2001
From: Elad Ben-Israel <benisrae@amazon.com>
Date: Tue, 7 Jan 2020 10:23:23 +0200
Subject: [PATCH] fix(core): nested Fn.join with token fails

Fn.join has an optimization to flatten nested joins with the same delimiter:

    Fn.join(",", [ Fn.join(",", [ "a", "b" ]), "c" ]) == Fn.join(",", [ "a", "b", "c" ])

The logic in `isSplicableFnJoinIntrinsic` checks if the object is an Fn::Join which uses the same delimiter, and then splices (`...`) the inner value onto the outer Fn::Join instead of nesting the inner Fn::Join. This can only work if the inner value is a real array (otherwise, we get `Found non-callable @@iterator`).

The fix is to add an additional check to `isSplicableFnJoinIntrinsic` which verifies the the inner value is indeed an array.

Fixes #5655
---
 .../core/lib/private/cloudformation-lang.ts     | 15 +++++++++++----
 packages/@aws-cdk/core/test/test.fn.ts          | 17 ++++++++++++++++-
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
index a6b184bb0cd5e..941f33155ff31 100644
--- a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
+++ b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
@@ -1,6 +1,6 @@
 import { Lazy } from "../lazy";
 import { Reference } from "../reference";
-import { DefaultTokenResolver, IFragmentConcatenator, IPostProcessor, IResolvable, IResolveContext  } from "../resolvable";
+import { DefaultTokenResolver, IFragmentConcatenator, IPostProcessor, IResolvable, IResolveContext } from "../resolvable";
 import { TokenizedStringFragments } from "../string-fragments";
 import { Token } from "../token";
 import { Intrinsic } from "./intrinsic";
@@ -171,9 +171,16 @@ export function minimalCloudFormationJoin(delimiter: string, values: any[]): any
   }
 
   function isSplicableFnJoinIntrinsic(obj: any): boolean {
-    return isIntrinsic(obj)
-      && Object.keys(obj)[0] === 'Fn::Join'
-      && obj['Fn::Join'][0] === delimiter;
+    if (!isIntrinsic(obj)) { return false; }
+    if (Object.keys(obj)[0] !== 'Fn::Join') { return false;  }
+
+    const [ delim, list ] = obj['Fn::Join'];
+    if (delim !== delimiter) { return false; }
+
+    if (Token.isUnresolved(list)) { return false; }
+    if (!Array.isArray(list)) { return false; }
+
+    return true;
   }
 }
 
diff --git a/packages/@aws-cdk/core/test/test.fn.ts b/packages/@aws-cdk/core/test/test.fn.ts
index 7a5b05e08524f..05b513a03dc95 100644
--- a/packages/@aws-cdk/core/test/test.fn.ts
+++ b/packages/@aws-cdk/core/test/test.fn.ts
@@ -176,7 +176,22 @@ export = nodeunit.testCase({
       });
       test.done();
     }
-  }
+  },
+  'nested Fn::Join with list token'(test: nodeunit.Test) {
+    const stack = new Stack();
+    const inner = Fn.join(',', Token.asList({ NotReallyList: true }));
+    const outer = Fn.join(',', [ inner, 'Foo' ]);
+    test.deepEqual(stack.resolve(outer), {
+      'Fn::Join': [
+        ',',
+        [
+          { 'Fn::Join': [ ',', { NotReallyList: true } ] },
+          'Foo'
+        ]
+      ]
+    });
+    test.done();
+  },
 });
 
 function stringListToken(o: any): string[] {