fix(lib): escape newlines in terraform functions

Previously newlines were added to the synthesised JSON, which JSON only accepts if escaped

Closes #1165

packages/cdktf/lib/tfExpression.ts

@@ -29,15 +29,25 @@ class TFExpression extends Intrinsic implements IResolvable {
     return resolvedArg;
   }
 
+  /**
+   * Escape string removes characters from the string that are not allowed in Terraform or JSON
+   * It must only be used on non-token values
+   */
+  private escapeString(str: string) {
+    return str
+      .replace(/\n/g, "\\n") // escape newlines
+      .replace(/\${/g, "$$${"); // escape ${ to $${
+  }
+
   private resolveString(str: string, resolvedArg: any) {
     const tokenList = Tokenization.reverseString(str);
     const numberOfTokens = tokenList.tokens.length + tokenList.intrinsic.length;
 
     // String literal
     if (numberOfTokens === 0) {
       return resolvedArg.startsWith('"') && resolvedArg.endsWith('"')
-        ? resolvedArg
-        : `"${resolvedArg}"`;
+        ? this.escapeString(resolvedArg)
+        : `"${this.escapeString(resolvedArg)}"`;
     }
 
     // Only a token reference
@@ -52,10 +62,14 @@ class TFExpression extends Intrinsic implements IResolvable {
         const rightTokens = Tokenization.reverse(right);
 
         const leftValue =
-          leftTokens.length === 0 ? left : `\${${leftTokens[0]}}`;
+          leftTokens.length === 0
+            ? this.escapeString(left)
+            : `\${${leftTokens[0]}}`;
 
         const rightValue =
-          rightTokens.length === 0 ? right : `\${${rightTokens[0]}}`;
+          rightTokens.length === 0
+            ? this.escapeString(right)
+            : `\${${rightTokens[0]}}`;
 
         return `${leftValue}${rightValue}`;
       },

packages/cdktf/test/tfExpression.test.ts

@@ -1,3 +1,4 @@
+import { Token } from "../lib";
 import {
   addOperation,
   andOperation,
@@ -16,9 +17,12 @@ import {
   orOperation,
   propertyAccess,
   ref,
+  call,
   subOperation,
+  Expression,
 } from "../lib/tfExpression";
 import { resolve } from "../lib/_tokens";
+const resolveExpression = (expr: Expression) => resolve(null as any, expr);
 
 test("can render reference", () => {
   expect(
@@ -28,8 +32,7 @@ test("can render reference", () => {
 
 test("propertyAccess renders correctly", () => {
   expect(
-    resolve(
-      null as any,
+    resolveExpression(
       propertyAccess(ref("some_resource.my_resource.some_attribute_array"), [
         0,
         "name",
@@ -41,83 +44,122 @@ test("propertyAccess renders correctly", () => {
 });
 
 test("conditional renders correctly", () => {
-  expect(resolve(null as any, conditional(true, 1, 0))).toMatchInlineSnapshot(
+  expect(resolveExpression(conditional(true, 1, 0))).toMatchInlineSnapshot(
     `"\${true ? 1 : 0}"`
   );
 });
 
 test("notOperation renders correctly", () => {
-  expect(resolve(null as any, notOperation(true))).toMatchInlineSnapshot(
+  expect(resolveExpression(notOperation(true))).toMatchInlineSnapshot(
     `"\${!true}"`
   );
 });
 test("negateOperation renders correctly", () => {
-  expect(resolve(null as any, negateOperation(1))).toMatchInlineSnapshot(
+  expect(resolveExpression(negateOperation(1))).toMatchInlineSnapshot(
     `"\${-1}"`
   );
 });
 test("mulOperation renders correctly", () => {
-  expect(resolve(null as any, mulOperation(2, 3))).toMatchInlineSnapshot(
+  expect(resolveExpression(mulOperation(2, 3))).toMatchInlineSnapshot(
     `"\${(2 * 3)}"`
   );
 });
 test("divOperation renders correctly", () => {
-  expect(resolve(null as any, divOperation(4, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(divOperation(4, 2))).toMatchInlineSnapshot(
     `"\${(4 / 2)}"`
   );
 });
 test("modOperation renders correctly", () => {
-  expect(resolve(null as any, modOperation(5, 3))).toMatchInlineSnapshot(
+  expect(resolveExpression(modOperation(5, 3))).toMatchInlineSnapshot(
     `"\${(5 % 3)}"`
   );
 });
 test("addOperation renders correctly", () => {
-  expect(resolve(null as any, addOperation(1, 1))).toMatchInlineSnapshot(
+  expect(resolveExpression(addOperation(1, 1))).toMatchInlineSnapshot(
     `"\${(1 + 1)}"`
   );
 });
 test("subOperation renders correctly", () => {
-  expect(resolve(null as any, subOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(subOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 - 2)}"`
   );
 });
 test("gtOperation renders correctly", () => {
-  expect(resolve(null as any, gtOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(gtOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 > 2)}"`
   );
 });
 test("gteOperation renders correctly", () => {
-  expect(resolve(null as any, gteOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(gteOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 >= 2)}"`
   );
 });
 test("ltOperation renders correctly", () => {
-  expect(resolve(null as any, ltOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(ltOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 < 2)}"`
   );
 });
 test("lteOperation renders correctly", () => {
-  expect(resolve(null as any, lteOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(lteOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 <= 2)}"`
   );
 });
 test("eqOperation renders correctly", () => {
-  expect(resolve(null as any, eqOperation(1, 1))).toMatchInlineSnapshot(
+  expect(resolveExpression(eqOperation(1, 1))).toMatchInlineSnapshot(
     `"\${(1 == 1)}"`
   );
 });
 test("neqOperation renders correctly", () => {
-  expect(resolve(null as any, neqOperation(1, 2))).toMatchInlineSnapshot(
+  expect(resolveExpression(neqOperation(1, 2))).toMatchInlineSnapshot(
     `"\${(1 != 2)}"`
   );
 });
 test("andOperation renders correctly", () => {
-  expect(resolve(null as any, andOperation(true, true))).toMatchInlineSnapshot(
+  expect(resolveExpression(andOperation(true, true))).toMatchInlineSnapshot(
     `"\${(true && true)}"`
   );
 });
 test("orOperation renders correctly", () => {
-  expect(resolve(null as any, orOperation(true, true))).toMatchInlineSnapshot(
+  expect(resolveExpression(orOperation(true, true))).toMatchInlineSnapshot(
     `"\${(true || true)}"`
   );
 });
+
+test("functions escape newlines", () => {
+  expect(
+    resolveExpression(
+      call("length", [
+        `
+This
+is
+a
+multi
+line
+string
+  `,
+      ])
+    )
+  ).toMatchInlineSnapshot(
+    `"\${length(\\"\\\\nThis\\\\nis\\\\na\\\\nmulti\\\\nline\\\\nstring\\\\n  \\")}"`
+  );
+});
+
+test("functions escape terraform reference like strings", () => {
+  expect(resolveExpression(call("length", [`\${}`]))).toMatchInlineSnapshot(
+    `"\${length(\\"$\${}\\")}"`
+  );
+});
+
+test("functions don't escape terraform references", () => {
+  expect(
+    resolveExpression(call("length", [ref("docker_container.foo.bar")]))
+  ).toMatchInlineSnapshot(`"\${length(docker_container.foo.bar)}"`);
+});
+
+test("functions don't escape terraform references that have been tokenized", () => {
+  expect(
+    resolveExpression(
+      call("length", [Token.asString(ref("docker_container.foo.bar"))])
+    )
+  ).toMatchInlineSnapshot(`"\${length(docker_container.foo.bar)}"`);
+});