From e4fb811c5b12de4d42fea71510e5bb3ee6b0a73e Mon Sep 17 00:00:00 2001
From: Romain Marcadier-Muller <rmuller@amazon.com>
Date: Fri, 7 Jun 2019 15:11:17 +0200
Subject: [PATCH] fix(core): Make filterUndefined null-safe (#2789)

There are a couple of places where fields accept values that are typed
as `Json` per the JSII type specification. This conveys that literal
`null` values may be passed and need to be preserved (as far as JSII is
concerned - see awslabs/jsii#523). However in the CloudFormation domain,
`null` is semantically equivalent to `undefined`.

Now enters Javascript's confusing type system, where `null` is an
`object` that cannot be converted to `object` (you read this correctly):

```js
typeof null === 'object'
// => true

Object.entries(null);
// => Thrown:
//    TypeError: Cannot convert undefined or null to object
//        at Function.entries (<canonymous>)
```

So this changes the `undefined` checks to the `null`-coercing way, so
that `null` and `undefined` are handled the same way.
---
 packages/@aws-cdk/cdk/lib/util.ts       |  6 +++---
 packages/@aws-cdk/cdk/test/test.util.ts | 20 ++++++++++++++++----
 packages/@aws-cdk/cdk/test/util.ts      |  2 +-
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/packages/@aws-cdk/cdk/lib/util.ts b/packages/@aws-cdk/cdk/lib/util.ts
index a0cbef73dd68f..f1edac35f5fc6 100644
--- a/packages/@aws-cdk/cdk/lib/util.ts
+++ b/packages/@aws-cdk/cdk/lib/util.ts
@@ -53,17 +53,17 @@ export function ignoreEmpty(obj: any): any {
 }
 
 /**
- * Returns a copy of `obj` without undefined values in maps or arrays.
+ * Returns a copy of `obj` without `undefined` (or `null`) values in maps or arrays.
  */
 export function filterUndefined(obj: any): any {
   if (Array.isArray(obj)) {
-    return obj.filter(x => x !== undefined).map(x => filterUndefined(x));
+    return obj.filter(x => x != null).map(x => filterUndefined(x));
   }
 
   if (typeof(obj) === 'object') {
     const ret: any = { };
     for (const [key, value] of Object.entries(obj)) {
-      if (value === undefined) {
+      if (value == null) {
         continue;
       }
       ret[key] = filterUndefined(value);
diff --git a/packages/@aws-cdk/cdk/test/test.util.ts b/packages/@aws-cdk/cdk/test/test.util.ts
index f21b4d59d13d6..ee4a4b0cef947 100644
--- a/packages/@aws-cdk/cdk/test/test.util.ts
+++ b/packages/@aws-cdk/cdk/test/test.util.ts
@@ -1,8 +1,8 @@
-import { Test } from 'nodeunit';
+import { Test, testCase } from 'nodeunit';
 import { Stack } from '../lib';
-import { capitalizePropertyNames, ignoreEmpty } from '../lib/util';
+import { capitalizePropertyNames, filterUndefined, ignoreEmpty } from '../lib/util';
 
-export = {
+export = testCase({
   'capitalizeResourceProperties capitalizes all keys of an object (recursively) from camelCase to PascalCase'(test: Test) {
     const c = new Stack();
 
@@ -71,8 +71,20 @@ export = {
       test.deepEqual(stack.resolve(ignoreEmpty({ xoo: { resolve: () => [ undefined, undefined ] }})), { xoo: [] });
       test.done();
     }
+  },
+
+  'filterUnderined': {
+    'is null-safe (aka treats null and undefined the same)'(test: Test) {
+      test.deepEqual(filterUndefined({ 'a null': null, 'a not null': true }), { 'a not null': true });
+      test.done();
+    },
+
+    'removes undefined, but leaves the rest'(test: Test) {
+      test.deepEqual(filterUndefined({ 'an undefined': undefined, 'yes': true }), { yes: true });
+      test.done();
+    }
   }
-};
+});
 
 class SomeToken {
   public foo = 60;
diff --git a/packages/@aws-cdk/cdk/test/util.ts b/packages/@aws-cdk/cdk/test/util.ts
index 33a55fc77dc98..de39d66917b16 100644
--- a/packages/@aws-cdk/cdk/test/util.ts
+++ b/packages/@aws-cdk/cdk/test/util.ts
@@ -2,4 +2,4 @@ import { ConstructNode, Stack } from '../lib';
 
 export function toCloudFormation(stack: Stack): any {
   return ConstructNode.synth(stack.node, { skipValidation: true }).getStack(stack.name).template;
-}
\ No newline at end of file
+}