fix(ivy): elements properties should not be stringified #22683
Conversation
marclaval
added
action: review
|
|
||
| t.update(() => elementProperty(0, 'hidden', false)); | ||
| // The hidden property would be true if `false` was stringified into `"false"`. | ||
| expect((t.hostNode.native as HTMLElement).querySelector('div') !.hidden).toEqual(false); |
There was a problem hiding this comment.
Wouldn't this allow someone to create `{toString:() => 'evel text'} and bypass sanitization?
Or is your argument that user can't do that only bad programing practice?
Or maybe we just skip sanatiazation for primitives and sanitizae evrything else?
| @@ -746,7 +746,7 @@ export function elementProperty<T>( | |||
| setInputsForProperty(dataValue, value); | |||
| markDirtyIfOnPush(node); | |||
| } else { | |||
| value = (sanitizer != null ? sanitizer(value) : stringify(value)) as any; | |||
| value = typeof value == 'string' && sanitizer != null ? (sanitizer(value) as any) : value; | |||
There was a problem hiding this comment.
I would feel better if the check was typeof value !== 'number' && typeof value !== 'boolean' since we need to guard against {toString: () => 'evil text'}
marclaval
force-pushed
the
fixPropertyStringification
branch
from
a3d299c
to
bdadc66
Compare
|
Updated with the assumption that the compiler will only "inject" the sanitizer for properties that are not safe, PTAL |
mhevery
added
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR fixes a regression introduced by #22540