fix: correctly emit quoted YAML for account numbers (#1105)

Switch back to the newly-fixed 'yaml' package so that we can get
both correct quoting of strings with leading '0' characters and
correct quoting of strings with colon ':' characters.

Fixes #1100, fixes #1098.

Author: rix0rrr

packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts

@@ -4,9 +4,9 @@ import 'source-map-support/register';
 import cdk = require('@aws-cdk/cdk');
 import child_process = require('child_process');
 import fs = require('fs-extra');
-import YAML = require('js-yaml');
 import os = require('os');
 import path = require('path');
+import YAML = require('yaml');
 
 import { isStackConstructor, parseApplet } from '../lib/applet-helpers';
 
@@ -25,7 +25,7 @@ async function main() {
   }
 
   // read applet(s) properties from the provided file
-  const fileContents = YAML.safeLoad(await fs.readFile(appletFile, { encoding: 'utf-8' }));
+  const fileContents = YAML.parse(await fs.readFile(appletFile, { encoding: 'utf-8' }));
   if (typeof fileContents !== 'object') {
     throw new Error(`${appletFile}: should contain a YAML object`);
   }

packages/@aws-cdk/applet-js/package.json

@@ -23,15 +23,15 @@
   "license": "Apache-2.0",
   "devDependencies": {
     "@types/fs-extra": "^5.0.4",
-    "@types/js-yaml": "^3.11.2",
+    "@types/yaml": "^1.0.0",
     "cdk-build-tools": "^0.15.1",
     "pkglint": "^0.15.1"
   },
   "dependencies": {
     "@aws-cdk/cdk": "^0.15.1",
     "fs-extra": "^7.0.0",
     "source-map-support": "^0.5.6",
-    "js-yaml": "^3.12.0"
+    "yaml": "^1.0.1"
   },
   "repository": {
     "url": "https://github.com/awslabs/aws-cdk.git",

packages/aws-cdk/bin/cdk.ts

@@ -4,7 +4,6 @@ import 'source-map-support/register';
 import cxapi = require('@aws-cdk/cx-api');
 import colors = require('colors/safe');
 import fs = require('fs-extra');
-import YAML = require('js-yaml');
 import minimatch = require('minimatch');
 import util = require('util');
 import yargs = require('yargs');
@@ -19,6 +18,7 @@ import { interactive } from '../lib/interactive';
 import { data, debug, error, highlight, print, setVerbose, success, warning } from '../lib/logging';
 import { PluginHost } from '../lib/plugin';
 import { parseRenames } from '../lib/renames';
+import { deserializeStructure, serializeStructure } from '../lib/serialize';
 import { DEFAULTS, PER_USER_DEFAULTS, Settings } from '../lib/settings';
 import { VERSION } from '../lib/version';
 
@@ -609,11 +609,7 @@ async function initCommandLine() {
 
     /* Attempt to parse YAML, fall back to JSON. */
     function parseTemplate(text: string): any {
-      try {
-        return YAML.safeLoad(text);
-      } catch (e) {
-        return JSON.parse(text);
-      }
+      return deserializeStructure(text);
     }
   }
 
@@ -679,13 +675,7 @@ async function initCommandLine() {
   }
 
   function toJsonOrYaml(object: any): string {
-    if (argv.json) {
-      const noFiltering = undefined;
-      const indentWidth = 2;
-      return JSON.stringify(object, noFiltering, indentWidth);
-    } else {
-      return YAML.safeDump(object);
-    }
+    return serializeStructure(object, argv.json);
   }
 }
 

packages/aws-cdk/integ-tests/test-cdk-synth.sh

@@ -8,17 +8,17 @@ setup
 
 assert "cdk synth cdk-toolkit-integration-test-1" <<HERE
 Resources:
-    topic69831491:
-        Type: 'AWS::SNS::Topic'
+  topic69831491:
+    Type: AWS::SNS::Topic
 
 HERE
 
 assert "cdk synth cdk-toolkit-integration-test-2" <<HERE
 Resources:
-    topic152D84A37:
-        Type: 'AWS::SNS::Topic'
-    topic2A4FB547F:
-        Type: 'AWS::SNS::Topic'
+  topic152D84A37:
+    Type: AWS::SNS::Topic
+  topic2A4FB547F:
+    Type: AWS::SNS::Topic
 
 HERE
 

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -1,10 +1,10 @@
 import cxapi = require('@aws-cdk/cx-api');
 import aws = require('aws-sdk');
 import colors = require('colors/safe');
-import YAML = require('js-yaml');
 import uuid = require('uuid');
 import { prepareAssets } from '../assets';
 import { debug, error, print } from '../logging';
+import { toYAML } from '../serialize';
 import { Mode } from './aws-auth/credentials';
 import { ToolkitInfo } from './toolkit-info';
 import { describeStack, stackExists, stackFailedCreating, waitForChangeSet, waitForStack } from './util/cloudformation';
@@ -113,7 +113,7 @@ async function getStackOutputs(cfn: aws.CloudFormation, stackName: string): Prom
  * @param toolkitInfo information about the toolkit stack
  */
 async function makeBodyParameter(stack: cxapi.SynthesizedStack, toolkitInfo?: ToolkitInfo): Promise<TemplateBodyParameter> {
-  const templateJson = YAML.safeDump(stack.template, { indent: 4, flowLevel: 16 });
+  const templateJson = toYAML(stack.template);
   if (toolkitInfo) {
     const s3KeyPrefix = `cdk/${stack.name}/`;
     const s3KeySuffix = '.yml';

packages/aws-cdk/lib/serialize.ts

@@ -0,0 +1,38 @@
+import YAML = require('yaml');
+
+/**
+ * Stringify to YAML
+ */
+export function toYAML(obj: any): string {
+    return YAML.stringify(obj, { schema: 'yaml-1.1' });
+}
+
+/**
+ * Parse YAML
+ */
+export function fromYAML(str: string): any {
+    return YAML.parse(str, { schema: 'yaml-1.1' });
+}
+
+/**
+ * Parse either YAML or JSON
+ */
+export function deserializeStructure(str: string) {
+  try {
+    return fromYAML(str);
+  } catch (e) {
+    // This shouldn't really ever happen I think, but it's the code we had so I'm leaving it.
+    return JSON.parse(str);
+  }
+}
+
+/**
+ * Serialize to either YAML or JSON
+ */
+export function serializeStructure(object: any, json: boolean) {
+  if (json) {
+    return JSON.stringify(object, undefined, 2);
+  } else {
+    return toYAML(object);
+  }
+}

packages/aws-cdk/package.json

@@ -39,7 +39,7 @@
     "@types/semver": "^5.5.0",
     "@types/uuid": "^3.4.3",
     "@types/yargs": "^8.0.3",
-    "@types/js-yaml": "^3.11.2",
+    "@types/yaml": "^1.0.0",
     "cdk-build-tools": "^0.15.1",
     "mockery": "^2.1.0",
     "pkglint": "^0.15.1"
@@ -54,7 +54,7 @@
     "colors": "^1.2.1",
     "decamelize": "^2.0.0",
     "fs-extra": "^4.0.2",
-    "js-yaml": "^3.12.0",
+    "yaml": "^1.0.1",
     "json-diff": "^0.3.1",
     "minimatch": ">=3.0",
     "promptly": "^0.2.0",

packages/aws-cdk/test/test.yaml.ts

@@ -1,44 +1,47 @@
 import { Test } from 'nodeunit';
+import { toYAML } from '../lib/serialize';
 
-import YAML = require('js-yaml');
-
-function yamlStringify(obj: any) {
-  return YAML.dump(obj);
-}
+// Preferred quote of the YAML library
+const q = '"';
 
 export = {
   'quote the word "ON"'(test: Test) {
     // NON NEGOTIABLE! If not quoted, will be interpreted as the boolean TRUE
 
     // tslint:disable-next-line:no-console
-    const output = yamlStringify({
+    const output = toYAML({
       notABoolean: "ON"
     });
 
-    test.equals(output.trim(), `notABoolean: 'ON'`);
+    test.equals(output.trim(), `notABoolean: ${q}ON${q}`);
 
     test.done();
   },
 
   'quote number-like strings with a leading 0'(test: Test) {
-    const output = yamlStringify({
+    const output = toYAML({
       leadingZero: "012345"
     });
 
-    test.equals(output.trim(), `leadingZero: '012345'`);
+    test.equals(output.trim(), `leadingZero: ${q}012345${q}`);
 
     test.done();
   },
 
   'do not quote octal numbers that arent really octal'(test: Test) {
-    // Under contention: this seems to be okay, pyyaml parses it
-    // correctly. Unsure of what CloudFormation does about it.
+    // This is a contentious one, and something that might have changed in YAML1.2 vs YAML1.1
+    //
+    // One could make the argument that a sequence of characters that couldn't ever
+    // be an octal value doesn't need to be quoted, and pyyaml parses it correctly.
+    //
+    // However, CloudFormation's parser interprets it as a decimal number (eating the
+    // leading 0) if it's unquoted, so that's the behavior we're testing for.
 
-    const output = yamlStringify({
+    const output = toYAML({
       leadingZero: "0123456789"
     });
 
-    test.equals(output.trim(), `leadingZero: 0123456789`);
+    test.equals(output.trim(), `leadingZero: ${q}0123456789${q}`);
 
     test.done();
   },
@@ -48,14 +51,14 @@ export = {
     //
     // 'yaml' fails this.
 
-    const output = yamlStringify({
+    const output = toYAML({
       colons: ['arn', ':', 'aws']
     });
 
     test.equals(output.trim(), [
       'colons:',
       '  - arn',
-      `  - ':'`,
+      `  - ${q}:${q}`,
       '  - aws'
     ].join('\n'));