8000 fix(compiler): avoid errors for inputs with Object-builtin names (#47… · angular/angular@cf0c53a · GitHub
[go: up one dir, main page]
Skip to content

Commit cf0c53a

Browse files
JoostKAndrewKushnir
JoostK
authored and
AndrewKushnir
committed
fix(compiler): avoid errors for inputs with Object-builtin names (#47220)
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
1 parent 4a53b7f commit cf0c53a

File tree

3 files changed

+52
-33
lines changed

3 files changed

+52
-33
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ class TcbDomSchemaCheckerOp extends TcbOp {
864864
if (binding.type === BindingType.Property) {
865865
if (binding.name !== 'style' && binding.name !== 'class') {
866866
// A direct binding to a property.
867-
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
867+
const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
868868
this.tcb.domSchemaChecker.checkProperty(
869869
this.tcb.id, this.element, propertyName, binding.sourceSpan, this.tcb.schemas,
870870
this.tcb.hostIsStandalone);
@@ -880,14 +880,14 @@ class TcbDomSchemaCheckerOp extends TcbOp {
880880
* Mapping between attributes names that don't correspond to their element property names.
881881
* Note: this mapping has to be kept in sync with the equally named mapping in the runtime.
882882
*/
883-
const ATTR_TO_PROP: {[name: string]: string} = {
883+
const ATTR_TO_PROP = new Map(Object.entries({
884884
'class': 'className',
885885
'for': 'htmlFor',
886886
'formaction': 'formAction',
887887
'innerHtml': 'innerHTML',
888888
'readonly': 'readOnly',
889889
'tabindex': 'tabIndex',
890-
};
890+
}));
891891

892892
/**
893893
* A `TcbOp` which generates code to check "unclaimed inputs" - bindings on an element which were
@@ -930,7 +930,7 @@ class TcbUnclaimedInputsOp extends TcbOp {
930930
elId = this.scope.resolve(this.element);
931931
}
932932
// A direct binding to a property.
933-
const propertyName = ATTR_TO_PROP[binding.name] || binding.name;
933+
const propertyName = ATTR_TO_PROP.get(binding.name) ?? binding.name;
934934
const prop = ts.factory.createElementAccessExpression(
935935
elId, ts.factory.createStringLiteral(propertyName));
936936
const stmt = ts.factory.createBinaryExpression(

‎packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3669,6 +3669,24 @@ function allTests(os: string) {
36693669
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
36703670
});
36713671

3672+
// https://github.com/angular/angular/issues/46936
3673+
it('should support bindings with Object builtin names', () => {
3674+
env.write('test.ts', `
3675+
import {Component} from '@angular/core';
3676+
3677+
@Component({
3678+
selector: 'test-cmp',
3679+
template: '<div [valueOf]="123"></div>',
3680+
})
3681+
export class TestCmp {}
3682+
`);
3683+
3684+
const errors = env.driveDiagnostics();
3685+
expect(errors.length).toBe(1);
3686+
expect(errors[0].messageText)
3687+
.toContain(`Can't bind to 'valueOf' since it isn't a known property of 'div'.`);
3688+
});
3689+
36723690
it('should handle $any used inside a listener', () => {
36733691
env.write('test.ts', `
36743692
import {Component} from '@angular/core';

‎packages/compiler/src/schema/dom_element_schema_registry.ts

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata, SecurityContext} from '../core';
10-
1110
import {isNgContainer, isNgContent} from '../ml_parser/tags';
1211
import {dashCaseToCamelCase} from '../util';
1312

@@ -232,46 +231,46 @@ const SCHEMA: string[] = [
232231
':svg:cursor^:svg:|',
233232
];
234233

235-
const _ATTR_TO_PROP: {[name: string]: string} = {
234+
const _ATTR_TO_PROP = new Map(Object.entries({
236235
'class': 'className',
237236
'for': 'htmlFor',
238237
'formaction': 'formAction',
239238
'innerHtml': 'innerHTML',
240239
'readonly': 'readOnly',
241240
'tabindex': 'tabIndex',
242-
};
241+
}));
243242

244243
// Invert _ATTR_TO_PROP.
245-
const _PROP_TO_ATTR: {[name: string]: string} =
246-
Object.keys(_ATTR_TO_PROP).reduce((inverted, attr) => {
247-
inverted[_ATTR_TO_PROP[attr]] = attr;
244+
const _PROP_TO_ATTR =
245+
Array.from(_ATTR_TO_PROP).reduce((inverted, [propertyName, attributeName]) => {
246+
inverted.set(propertyName, attributeName);
248247
return inverted;
249-
}, {} as {[prop: string]: string});
248+
}, new Map<string, string>());
250249

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

257256
constructor() {
258257
super();
259258
SCHEMA.forEach(encodedType => {
260-
const type: {[property: string]: string} = {};
259+
const type = new Map<string, string>();
261260
const events: Set<string> = new Set();
262261
const [strType, strProperties] = encodedType.split('|');
263262
const properties = strProperties.split(',');
264263
const [typeNames, superName] = strType.split('^');
265264
typeNames.split(',').forEach(tag => {
266-
this._schema[tag.toLowerCase()] = type;
267-
this._eventSchema[tag.toLowerCase()] = events;
265+
this._schema.set(tag.toLowerCase(), type);
266+
this._eventSchema.set(tag.toLowerCase(), events);
268267
});
269-
const superType = superName && 10000 ; this._schema[superName.toLowerCase()];
268+
const superType = superName && this._schema.get(superName.toLowerCase());
270269
if (superType) {
271-
Object.keys(superType).forEach((prop: string) => {
272-
type[prop] = superType[prop];
273-
});
274-
for (const superEvent of this._eventSchema[superName.toLowerCase()]) {
270+
for (const [prop, value] of superType) {
271+
type.set(prop, value);
272+
}
273+
for (const superEvent of this._eventSchema.get(superName.toLowerCase())!) {
275274
events.add(superEvent);
276275
}
277276
}
@@ -282,16 +281,16 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
282281
events.add(property.substring(1));
283282
break;
284283
case '!':
285-
type[property.substring(1)] = BOOLEAN;
284+
type.set(property.substring(1), BOOLEAN);
286285
break;
287286
case '#':
288-
type[property.substring(1)] = NUMBER;
287+
type.set(property.substring(1), NUMBER);
289288
break;
290289
case '%':
291-
type[property.substring(1)] = OBJECT;
290+
type.set(property.substring(1), OBJECT);
292291
break;
293292
default:
294-
type[property] = STRING;
293+
type.set(property, STRING);
295294
}
296295
}
297296
});
@@ -315,8 +314,9 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
315314
}
316315
}
317316

318-
const elementProperties = this._schema[tagName.toLowerCase()] || this._schema['unknown'];
319-
return !!elementProperties[propName];
317+
const elementProperties =
318+
this._schema.get(tagName.toLowerCase()) || this._schema.get('unknown')!;
319+
return elementProperties.has(propName);
320320
}
321321

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

338-
return !!this._schema[tagName.toLowerCase()];
338+
return this._schema.has(tagName.toLowerCase());
339339
}
340340

341341
/**
@@ -368,7 +368,7 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
368368
}
369369

370370
override getMappedPropName(propName: string): string {
371-
return _ATTR_TO_PROP[propName] || propName;
371+
return _ATTR_TO_PROP.get(propName) ?? propName;
372372
}
373373

374374
override getDefaultComponentElementName(): string {
@@ -398,17 +398,18 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
398398
}
399399

400400
override allKnownElementNames(): string[] {
401-
return Object.keys(this._schema);
401+
return Array.from(this._schema.keys());
402402
}
403403

404404
allKnownAttributesOfElement(tagName: string): string[] {
405-
const elementProperties = this._schema[tagName.toLowerCase()] || this._schema['unknown'];
405+
const elementProperties =
406+
this._schema.get(tagName.toLowerCase()) || this._schema.get('unknown')!;
406407
// Convert properties to attributes.
407-
return Object.keys(elementProperties).map(prop => _PROP_TO_ATTR[prop] ?? prop);
408+
return Array.from(elementProperties.keys()).map(prop => _PROP_TO_ATTR.get(prop) ?? prop);
408409
}
409410

410411
allKnownEventsOfElement(tagName: string): string[] {
411-
return Array.from(this._eventSchema[tagName.toLowerCase()] ?? []);
412+
return Array.from(this._eventSchema.get(tagName.toLowerCase()) ?? []);
412413
}
413414

414415
override normalizeAnimationStyleProperty(propName: string): string {

0 commit comments

Comments
 (0)
0