From fd51e01335337d95d684458a3d78283cbe014f19 Mon Sep 17 00:00:00 2001
From: Keen Yee Liau <kyliau@google.com>
Date: Wed, 29 Jul 2020 13:25:17 -0700
Subject: [PATCH] fix(compiler): Metadata should not include methods on
 Object.prototype (#38292)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit fixes a bug in View Engine whereby the compiler errorneously
thinks that a method of a component has decorator metadata when that
method is one of those in `Object.prototype`, for example `toString`.

This bug is discovered in v10.0.4 of `@angular/language-service` after
the default bundle format was switched from ES5 to ES2015.

ES5 output:
```js
if (propMetadata[propName]) {
    decorators.push.apply(decorators, __spread(propMetadata[propName]));
}
```

ES2015 output:
```js
if (propMetadata[propName]) {
    decorators.push(...propMetadata[propName]);
}
```

The bug was not discovered in ES5 because the polyfill for the spread
operator happily accepts parameters that do not have the `iterable`
symbol:

```js
function __spread() {
    for (var ar = [], i = 0; i < arguments.length; i++)
        ar = ar.concat(__read(arguments[i]));
    return ar;
}
```

whereas in es2015 it’ll fail since the iterable symbol is not present in
`propMetadata['toString']` which evaluates to a function.

Fixes https://github.com/angular/vscode-ng-language-service/issues/859

PR Close #38292
---
 packages/compiler/src/aot/static_reflector.ts |  4 ++-
 .../test/aot/static_reflector_spec.ts         | 29 +++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/packages/compiler/src/aot/static_reflector.ts b/packages/compiler/src/aot/static_reflector.ts
index c60b6dbe24..9964474086 100644
--- a/packages/compiler/src/aot/static_reflector.ts
+++ b/packages/compiler/src/aot/static_reflector.ts
@@ -235,7 +235,9 @@ export class StaticReflector implements CompileReflector {
         const prop = (<any[]>propData)
                          .find(a => a['__symbolic'] == 'property' || a['__symbolic'] == 'method');
         const decorators: any[] = [];
-        if (propMetadata![propName]) {
+        // hasOwnProperty() is used here to make sure we do not look up methods
+        // on `Object.prototype`.
+        if (propMetadata?.hasOwnProperty(propName)) {
           decorators.push(...propMetadata![propName]);
         }
         propMetadata![propName] = decorators;
diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts
index d1ae5dd5a4..cf56450fbe 100644
--- a/packages/compiler/test/aot/static_reflector_spec.ts
+++ b/packages/compiler/test/aot/static_reflector_spec.ts
@@ -777,6 +777,35 @@ describe('StaticReflector', () => {
           .toEqual({});
     });
 
+    it('should not inherit methods from Object.prototype', () => {
+      const filePath = '/tmp/test.ts';
+      init({
+        ...DEFAULT_TEST_DATA,
+        [filePath]: `
+          import {Component} from '@angular/core';
+
+          @Component({
+            selector: 'test-component',
+          })
+          export class TestComponent {
+            title = 'Hello World';
+
+            toString() {
+              return 'Test Component';
+            }
+          }
+        `,
+      });
+      const declaration = reflector.getStaticSymbol(filePath, 'TestComponent');
+      expect(declaration.filePath).toBe(filePath);
+      expect(declaration.name).toBe('TestComponent');
+      const propMetadata = reflector.propMetadata(declaration);
+      // 'toString' is a member of TestComponent so it should be part of the metadata.
+      expect(propMetadata.hasOwnProperty('toString')).toBe(true);
+      // There are no decorators on 'toString' so it should be an empty array.
+      expect(propMetadata['toString']).toEqual([]);
+    });
+
     it('should inherit lifecycle hooks', () => {
       initWithDecorator({
         '/tmp/src/main.ts': `