Merge pull request #1152 from glimmerjs/bugfix/prevent-dynamic-invoca…

…tion

[BUGFIX] Prevent accidental dynamic component lookup

packages/@glimmer/integration-tests/lib/suites/components.ts

@@ -556,6 +556,21 @@ export class BasicComponents extends RenderTest {
     this.assertStableRerender();
   }
 
+  @test({
+    kind: 'glimmer',
+  })
+  'invoking dynamic component (path) via angle brackets does not work for string'() {
+    class TestHarness extends EmberishGlimmerComponent {
+      public Foo: any;
+    }
+    this.registerComponent('Glimmer', 'TestHarness', '<this.Foo />', TestHarness);
+    this.registerComponent('Glimmer', 'Foo', 'hello world!');
+
+    this.assert.throws(() => {
+      this.render('<TestHarness @Foo="Foo" />');
+    }, /Expected a curried component definition, but received Foo. You may have accidentally done <this.Foo>, where \"this.Foo\"/);
+  }
+
   @test({
     kind: 'glimmer',
   })

packages/@glimmer/interfaces/lib/compile/encoder.d.ts

@@ -167,6 +167,7 @@ export interface DynamicComponentOp {
     args: WireFormat.Core.Hash;
     blocks: WireFormat.Core.Blocks | NamedBlocks;
     atNames: boolean;
+    curried: boolean;
   };
 }
 

packages/@glimmer/interfaces/lib/vm-opcodes.d.ts

@@ -77,26 +77,27 @@ export const enum Op {
   PushDynamicComponentInstance = 78,
   PushCurriedComponent = 79,
   ResolveDynamicComponent = 80,
-  PushArgs = 81,
-  PushEmptyArgs = 82,
-  PopArgs = 83,
-  PrepareArgs = 84,
-  CaptureArgs = 85,
-  CreateComponent = 86,
-  RegisterComponentDestructor = 87,
-  PutComponentOperations = 88,
-  GetComponentSelf = 89,
-  GetComponentTagName = 90,
-  GetComponentLayout = 91,
-  BindEvalScope = 92,
-  SetupForEval = 93,
-  PopulateLayout = 94,
-  InvokeComponentLayout = 95,
-  BeginComponentTransaction = 96,
-  CommitComponentTransaction = 97,
-  DidCreateElement = 98,
-  DidRenderLayout = 99,
-  InvokePartial = 100,
+  ResolveCurriedComponent = 81,
+  PushArgs = 82,
+  PushEmptyArgs = 83,
+  PopArgs = 84,
+  PrepareArgs = 85,
+  CaptureArgs = 86,
+  CreateComponent = 87,
+  RegisterComponentDestructor = 88,
+  PutComponentOperations = 89,
+  GetComponentSelf = 90,
+  GetComponentTagName = 91,
+  GetComponentLayout = 92,
+  BindEvalScope = 93,
+  SetupForEval = 94,
+  PopulateLayout = 95,
+  InvokeComponentLayout = 96,
+  BeginComponentTransaction = 97,
+  CommitComponentTransaction = 98,
+  DidCreateElement = 99,
+  DidRenderLayout = 100,
+  InvokePartial = 101,
   ResolveMaybeLocal = 102,
   Debugger = 103,
   Size = 104,

packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts

@@ -52,6 +52,7 @@ interface AnyComponent {
 export interface DynamicComponent extends AnyComponent {
   definition: WireFormat.Expression;
   atNames: boolean;
+  curried: boolean;
 }
 
 // (component)
@@ -250,7 +251,7 @@ export function InvokeStaticComponent({
 
 export function InvokeDynamicComponent(
   meta: ContainingMetadata,
-  { definition, attrs, params, hash, atNames, blocks }: DynamicComponent
+  { definition, attrs, params, hash, atNames, blocks, curried }: DynamicComponent
 ): StatementCompileActions {
   return Replayable({
     args: () => {
@@ -263,7 +264,9 @@ export function InvokeDynamicComponent(
     body: () => {
       return [
         op(Op.JumpUnless, label('ELSE')),
-        op(Op.ResolveDynamicComponent, templateMeta(meta.referrer)),
+        curried
+          ? op(Op.ResolveCurriedComponent)
+          : op(Op.ResolveDynamicComponent, templateMeta(meta.referrer)),
         op(Op.PushDynamicComponentInstance),
         InvokeComponent({
           capabilities: true,

packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts

@@ -236,6 +236,7 @@ export function populateBuiltins(
       args: hash,
       atNames: false,
       blocks,
+      curried: false,
     });
   });
 
@@ -259,6 +260,7 @@ export function populateBuiltins(
       hash,
       atNames: false,
       blocks: EMPTY_BLOCKS,
+      curried: false,
     });
   });
 

packages/@glimmer/opcode-compiler/lib/syntax/push-compile.ts

@@ -66,7 +66,7 @@ function DynamicComponent(
   context: TemplateCompilationContext,
   action: DynamicComponentOp
 ): StatementCompileActions {
-  let { definition, attrs, params, args, blocks, atNames } = action.op1;
+  let { definition, attrs, params, args, blocks, atNames, curried } = action.op1;
 
   let attrsBlock = attrs && attrs.length > 0 ? compilableBlock(attrs, context.meta) : null;
 
@@ -80,6 +80,7 @@ function DynamicComponent(
     hash: args,
     atNames,
     blocks: compiled,
+    curried,
   });
 }
 

packages/@glimmer/opcode-compiler/lib/syntax/statements.ts

@@ -142,6 +142,7 @@ STATEMENTS.add(SexpOpcodes.Component, ([, tag, attrs, args, blocks]) => {
       args,
       blocks,
       atNames: true,
+      curried: true,
     });
   }
 });

packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts

@@ -31,6 +31,10 @@ import { Tag, COMPUTE } from '@glimmer/validator';
 import { PartialScopeImpl } from '../../scope';
 import { VMArgumentsImpl } from '../../vm/arguments';
 import { ComponentInstance, ComponentElementOperations } from './component';
+import {
+  CurriedComponentDefinition,
+  isCurriedComponentDefinition,
+} from '../../component/curried-component';
 
 export const CheckTag: Checker<Tag> = CheckInterface({
   [COMPUTE]: CheckFunction,
@@ -106,6 +110,20 @@ export const CheckComponentDefinition: Checker<ComponentDefinition> = CheckInter
   manager: CheckComponentManager,
 });
 
+class CurriedComponentDefinitionChecker implements Checker<CurriedComponentDefinition> {
+  type!: CurriedComponentDefinition;
+
+  validate(value: unknown): value is CurriedComponentDefinition {
+    return isCurriedComponentDefinition(value);
+  }
+
+  expected(): string {
+    return `CurriedComponentDefinition`;
+  }
+}
+
+export const CheckCurriedComponentDefinition = new CurriedComponentDefinitionChecker();
+
 export const CheckInvocation: Checker<Invocation> = CheckInterface({
   handle: CheckNumber,
   symbolTable: CheckProgramSymbolTable,

packages/@glimmer/runtime/lib/compiled/opcodes/component.ts

@@ -5,6 +5,8 @@ import {
   CheckInstanceof,
   CheckInterface,
   CheckProgramSymbolTable,
+  CheckString,
+  CheckOr,
 } from '@glimmer/debug';
 import {
   Bounds,
@@ -74,6 +76,7 @@ import {
   CheckFinishedComponentInstance,
   CheckInvocation,
   CheckReference,
+  CheckCurriedComponentDefinition,
 } from './-debug-strip';
 import { UpdateDynamicAttributeOpcode } from './dom';
 import { DEBUG } from '@glimmer/env';
@@ -163,7 +166,10 @@ APPEND_OPCODES.add(Op.PushComponentDefinition, (vm, { op1: handle }) => {
 
 APPEND_OPCODES.add(Op.ResolveDynamicComponent, (vm, { op1: _meta }) => {
   let stack = vm.stack;
-  let component = valueForRef(check(stack.popJs(), CheckReference)) as Maybe<Dict>;
+  let component = check(
+    valueForRef(check(stack.popJs(), CheckReference)),
+    CheckOr(CheckString, CheckCurriedComponentDefinition)
+  );
   let meta = vm[CONSTANTS].getValue(_meta);
 
   vm.loadValue($t1, null); // Clear the temp register
@@ -174,12 +180,27 @@ APPEND_OPCODES.add(Op.ResolveDynamicComponent, (vm, { op1: _meta }) => {
     let resolvedDefinition = resolveComponent(vm.runtime.resolver, component, meta);
 
     definition = expect(resolvedDefinition, `Could not find a component named "${component}"`);
-  } else if (isCurriedComponentDefinition(component)) {
-    definition = component;
   } else {
-    throw unreachable();
+    definition = component;
+  }
+
+  stack.pushJs(definition);
+});
+
+APPEND_OPCODES.add(Op.ResolveCurriedComponent, (vm) => {
+  let stack = vm.stack;
+  let ref = check(stack.popJs(), CheckReference);
+  let value = valueForRef(ref);
+
+  if (DEBUG && !isCurriedComponentDefinition(value)) {
+    throw new Error(
+      `Expected a curried component definition, but received ${value}. You may have accidentally done <${ref.debugLabel}>, where "${ref.debugLabel}" was a string instead of a curried component definition. You must use the {{component}} helper to create a component definition when invoking dynamically.`
+    );
   }
 
+  let definition = value as CurriedComponentDefinition;
+
+  vm.loadValue($t1, null); // Clear the temp register
   stack.pushJs(definition);
 });