Skip to content

Commit

Permalink
fix(compiler): avoid errors for inputs with Object-builtin names (#47220
Browse files Browse the repository at this point in the history
)

Using raw objects as a lookup structure will inadvertently find methods defined on
`Object`, where strings are expected. This causes errors downstream when string
operations are applied on functions.

This commit switches over to use `Map`s in the DOM element schema registry to fix
this category of issues.

Fixes #46936

PR Close #47220
  • Loading branch information
JoostK authored and AndrewKushnir committed Sep 6, 2022
1 parent 124a600 commit 0e35829
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 33 deletions.
Expand Up @@ -864,7 +864,7 @@ class TcbDomSchemaCheckerOp extends TcbOp {
if (binding.type === BindingType.Property) {
if (binding.name !== 'style' && binding.name !== 'class') {
// A direct binding to a property.
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
this.tcb.domSchemaChecker.checkProperty(
this.tcb.id, this.element, propertyName, binding.sourceSpan, this.tcb.schemas,
this.tcb.hostIsStandalone);
Expand All @@ -880,14 +880,14 @@ class TcbDomSchemaCheckerOp extends TcbOp {
* Mapping between attributes names that don't correspond to their element property names.
* Note: this mapping has to be kept in sync with the equally named mapping in the runtime.
*/
const ATTR_TO_PROP: {[name: string]: string} = {
const ATTR_TO_PROP = new Map(Object.entries({
'class': 'className',
'for': 'htmlFor',
'formaction': 'formAction',
'innerHtml': 'innerHTML',
'readonly': 'readOnly',
'tabindex': 'tabIndex',
};
}));

/**
* A `TcbOp` which generates code to check "unclaimed inputs" - bindings on an element which were
Expand Down Expand Up @@ -930,7 +930,7 @@ class TcbUnclaimedInputsOp extends TcbOp {
elId = this.scope.resolve(this.element);
}
// A direct binding to a property.
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
const prop = ts.factory.createElementAccessExpression(
elId, ts.factory.createStringLiteral(propertyName));
const stmt = ts.factory.createBinaryExpression(
Expand Down
18 changes: 18 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -3670,6 +3670,24 @@ function allTests(os: string) {
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
});

// https://github.com/angular/angular/issues/46936
it('should support bindings with Object builtin names', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-cmp',
template: '<div [valueOf]="123"></div>',
})
export class TestCmp {}
`);

const errors = env.driveDiagnostics();
expect(errors.length).toBe(1);
expect(errors[0].messageText)
.toContain(`Can't bind to 'valueOf' since it isn't a known property of 'div'.`);
});

it('should handle $any used inside a listener', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
Expand Down
59 changes: 30 additions & 29 deletions packages/compiler/src/schema/dom_element_schema_registry.ts
Expand Up @@ -7,7 +7,6 @@
*/

import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata, SecurityContext} from '../core';

import {isNgContainer, isNgContent} from '../ml_parser/tags';
import {dashCaseToCamelCase} from '../util';

Expand Down Expand Up @@ -232,46 +231,46 @@ const SCHEMA: string[] = [
':svg:cursor^:svg:|',
];

const _ATTR_TO_PROP: {[name: string]: string} = {
const _ATTR_TO_PROP = new Map(Object.entries({
'class': 'className',
'for': 'htmlFor',
'formaction': 'formAction',
'innerHtml': 'innerHTML',
'readonly': 'readOnly',
'tabindex': 'tabIndex',
};
}));

// Invert _ATTR_TO_PROP.
const _PROP_TO_ATTR: {[name: string]: string} =
Object.keys(_ATTR_TO_PROP).reduce((inverted, attr) => {
inverted[_ATTR_TO_PROP[attr]] = attr;
const _PROP_TO_ATTR =
Array.from(_ATTR_TO_PROP).reduce((inverted, [propertyName, attributeName]) => {
inverted.set(propertyName, attributeName);
return inverted;
}, {} as {[prop: string]: string});
}, new Map<string, string>());

export class DomElementSchemaRegistry extends ElementSchemaRegistry {
private _schema: {[element: string]: {[property: string]: string}} = {};
private _schema = new Map<string, Map<string, string>>();
// We don't allow binding to events for security reasons. Allowing event bindings would almost
// certainly introduce bad XSS vulnerabilities. Instead, we store events in a separate schema.
private _eventSchema: {[element: string]: Set<string>} = {};
private _eventSchema = new Map<string, Set<string>>;

constructor() {
super();
SCHEMA.forEach(encodedType => {
const type: {[property: string]: string} = {};
const type = new Map<string, string>();
const events: Set<string> = new Set();
const [strType, strProperties] = encodedType.split('|');
const properties = strProperties.split(',');
const [typeNames, superName] = strType.split('^');
typeNames.split(',').forEach(tag => {
this._schema[tag.toLowerCase()] = type;
this._eventSchema[tag.toLowerCase()] = events;
this._schema.set(tag.toLowerCase(), type);
this._eventSchema.set(tag.toLowerCase(), events);
});
const superType = superName && this._schema[superName.toLowerCase()];
const superType = superName && this._schema.get(superName.toLowerCase());
if (superType) {
Object.keys(superType).forEach((prop: string) => {
type[prop] = superType[prop];
});
for (const superEvent of this._eventSchema[superName.toLowerCase()]) {
for (const [prop, value] of superType) {
type.set(prop, value);
}
for (const superEvent of this._eventSchema.get(superName.toLowerCase())!) {
events.add(superEvent);
}
}
Expand All @@ -282,16 +281,16 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
events.add(property.substring(1));
break;
case '!':
type[property.substring(1)] = BOOLEAN;
type.set(property.substring(1), BOOLEAN);
break;
case '#':
type[property.substring(1)] = NUMBER;
type.set(property.substring(1), NUMBER);
break;
case '%':
type[property.substring(1)] = OBJECT;
type.set(property.substring(1), OBJECT);
break;
default:
type[property] = STRING;
type.set(property, STRING);
}
}
});
Expand All @@ -315,8 +314,9 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
}
}

const elementProperties = this._schema[tagName.toLowerCase()] || this._schema['unknown'];
return !!elementProperties[propName];
const elementProperties =
this._schema.get(tagName.toLowerCase()) || this._schema.get('unknown')!;
return elementProperties.has(propName);
}

override hasElement(tagName: string, schemaMetas: SchemaMetadata[]): boolean {
Expand All @@ -335,7 +335,7 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
}
}

return !!this._schema[tagName.toLowerCase()];
return this._schema.has(tagName.toLowerCase());
}

/**
Expand Down Expand Up @@ -368,7 +368,7 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
}

override getMappedPropName(propName: string): string {
return _ATTR_TO_PROP[propName] || propName;
return _ATTR_TO_PROP.get(propName) ?? propName;
}

override getDefaultComponentElementName(): string {
Expand Down Expand Up @@ -398,17 +398,18 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
}

override allKnownElementNames(): string[] {
return Object.keys(this._schema);
return Array.from(this._schema.keys());
}

allKnownAttributesOfElement(tagName: string): string[] {
const elementProperties = this._schema[tagName.toLowerCase()] || this._schema['unknown'];
const elementProperties =
this._schema.get(tagName.toLowerCase()) || this._schema.get('unknown')!;
// Convert properties to attributes.
return Object.keys(elementProperties).map(prop => _PROP_TO_ATTR[prop] ?? prop);
return Array.from(elementProperties.keys()).map(prop => _PROP_TO_ATTR.get(prop) ?? prop);
}

allKnownEventsOfElement(tagName: string): string[] {
return Array.from(this._eventSchema[tagName.toLowerCase()] ?? []);
return Array.from(this._eventSchema.get(tagName.toLowerCase()) ?? []);
}

override normalizeAnimationStyleProperty(propName: string): string {
Expand Down

0 comments on commit 0e35829

Please sign in to comment.