From 55168021423e58895cb0bba956883cab04c641c4 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Mon, 30 Mar 2020 22:35:29 -0700 Subject: [PATCH] fix(compiler): avoid undefined expressions in holey array (#36343) From G3 bug ID: 116443331 The View Engine compiler crashes when it tries to compile a test in JIT mode that includes the d3-scale-chromatic library [1]. The d3 package initializes some arrays using the following pattern: ```js export var scheme = new Array(3).concat( "d8b365f5f5f55ab4ac", // ... more entries ).map(colors); ``` The stack trace from the crash is as follows: ``` TypeError: Cannot read property 'visitExpression' of undefined at ../../../third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts:505:39 at JitEmitterVisitor.AbstractEmitterVisitor.visitAllObjects third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=526 at JitEmitterVisitor.AbstractEmitterVisitor.visitAllExpressions third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=505 at JitEmitterVisitor.AbstractEmitterVisitor.visitLiteralArrayExpr third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=484 at LiteralArrayExpr.visitExpression third_party/javascript/angular2/rc/packages/compiler/src/output/output_ast.ts?l=791 at ../../../third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts:492:19 at JitEmitterVisitor.AbstractEmitterVisitor.visitAllObjects third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=526 at JitEmitterVisitor.AbstractEmitterVisitor.visitLiteralMapExpr third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=490 at LiteralMapExpr.visitExpression third_party/javascript/angular2/rc/packages/compiler/src/output/output_ast.ts?l=819 at ../../../third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts:505:39 at JitEmitterVisitor.AbstractEmitterVisitor.visitAllObjects third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=526 at JitEmitterVisitor.AbstractEmitterVisitor.visitAllExpressions third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=505 at JitEmitterVisitor.AbstractEmitterVisitor.visitInvokeFunctionExpr third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_emitter.ts?l=318 at JitEmitterVisitor.AbstractJsEmitterVisitor.visitInvokeFunctionExpr third_party/javascript/angular2/rc/packages/compiler/src/output/abstract_js_emitter.ts?l=112 at InvokeFunctionExpr.visitExpression third_party/javascript/angular2/rc/packages/compiler/src/output/output_ast.ts?l=440 ``` This is because the corresponding output AST for the array is of the form ```ts [ undefined, undefined, undefined, o.LiteralExpr, // ... ] ``` The output AST is clearly malformed and breaks the type representation of `LiteralArrayExpr` in which every entry is expected to be of type `Expression`. This commit fixes the bug by using a plain `for` loop to iterate over the entire length of the holey array and convert undefined elements to `LiteralExpr`. [1]: https://github.com/d3/d3-scale-chromatic/blob/master/src/diverging/BrBG.js PR Close #36343 --- packages/compiler/src/output/value_util.ts | 11 ++++++++- .../compiler/test/output/value_util_spec.ts | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 packages/compiler/test/output/value_util_spec.ts diff --git a/packages/compiler/src/output/value_util.ts b/packages/compiler/src/output/value_util.ts index c043800a6f..dd621c3817 100644 --- a/packages/compiler/src/output/value_util.ts +++ b/packages/compiler/src/output/value_util.ts @@ -21,7 +21,16 @@ export function convertValueToOutputAst( class _ValueOutputAstTransformer implements ValueTransformer { constructor(private ctx: OutputContext) {} visitArray(arr: any[], type: o.Type): o.Expression { - return o.literalArr(arr.map(value => visitValue(value, this, null)), type); + const values: o.Expression[] = []; + // Note Array.map() must not be used to convert the values because it will + // skip over empty elements in arrays constructed using `new Array(length)`, + // resulting in `undefined` elements. This breaks the type guarantee that + // all values in `o.LiteralArrayExpr` are of type `o.Expression`. + // See test case in `value_util_spec.ts`. + for (let i = 0; i < arr.length; ++i) { + values.push(visitValue(arr[i], this, null /* context */)); + } + return o.literalArr(values, type); } visitStringMap(map: {[key: string]: any}, type: o.MapType): o.Expression { diff --git a/packages/compiler/test/output/value_util_spec.ts b/packages/compiler/test/output/value_util_spec.ts new file mode 100644 index 0000000000..536cf81527 --- /dev/null +++ b/packages/compiler/test/output/value_util_spec.ts @@ -0,0 +1,23 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as o from '../../src/output/output_ast'; +import {convertValueToOutputAst} from '../../src/output/value_util'; + +describe('convertValueToOutputAst', () => { + it('should convert all array elements, including undefined', () => { + const ctx = null; + const value = new Array(3).concat('foo'); + const expr = convertValueToOutputAst(ctx !, value) as o.LiteralArrayExpr; + expect(expr instanceof o.LiteralArrayExpr); + expect(expr.entries.length).toBe(4); + for (let i = 0; i < 4; ++i) { + expect(expr.entries[i] instanceof o.Expression).toBe(true); + } + }); +});