Repository navigation
Attributes: Don't stringify values as it breaks Trusted Types #4948
Description
Originally posted by @koto in #4409 (comment)
I noticed one violation still in jQuery.attr, where the value of the attribute is stringified before calling setAttribute:
const policy = trustedTypes.createPolicy('a', {createScriptURL: s=>s})
jQuery(aScript, policy.createScriptURL('foo'));
Uncaught TypeError: Failed to execute 'setAttribute' on 'Element': This document requires 'TrustedScriptURL' assignment.
at attr (jquery.js:6997)
at access (jquery.js:3467)
at jQuery.fn.init.attr (jquery.js:6955)
at <anonymous>:1:6
There is a workaround via .attr hooks, but it might be worth addressing nonetheless.
It seems like this was introduced in ff75767, I suspect to workaround an IE <= 9 bug, which incorrectly stringified objects passed to setAttribute.
I'm not sure what the most elegant solution would be here, I guess it depends on whether jQuery 4 aims to support IE9. If not, it's safe not to stringify values (browser API would). If yes, then there's only a less-than-ideal option of testing for the bug? IIRC this would be a good test:
with (document.createElement('div')) {
setAttribute('title', {toString:()=>''});
getAttribute('title') === ''
}
Activity
mgol commentedon Oct 7, 2021
jQuery 4.x will only support IE 11 so if this was just to please IE <=9 we should be good to remove the manual stringification.
Attributes: Don't stringify attributes in the setter
mgol commentedon Oct 7, 2021
A draft PR: #4949. Setting the attribute now works but the script will still be blocked. This is because in
domManipscripts are not inserted directly but instead first disabled and then theirsrcattributes are read and inserted in fresh scripts.There's probably not much we can do when the scripts are deep inside of the inserted HTML string - natively scripts would not fire then but jQuery does execute them which will not work here. However, we could at least make
.append(scriptElem)work by forking the code path and treating such top-level scripts independently.I think we could land #4949 as-is, just without the part where it checks if the script was actually invoked and then we can tackle the remaining issue separately.
koto commentedon Oct 8, 2021
I agree. In general the behavior where jQuery takes the script nodes from the HTML string to add them separately feels like an antipattern nowadays (for example, this used to be a CSP strict-dynamic bypass vector), but I understand how it was useful back when the API was created (heck, I used it myself :) ).
In my personal opinion
.append(scriptElem)could be a good workaround, anddomManipcould stop special-casing script nodes, but there are backward compatibility issues that I don't know the full impact of.mgol commentedon Oct 8, 2021
Yeah, I'd also love to take out all that script execution stuff, at least as a default, but this may be too big a compatibility break. We'll think if anything can be done to that more generic case.
TBH it's quite weird that Web APIs don't behave like jQuery here natively; it was surprising to me when I first learnt appending script elements directly executes them but not when done via an HTML string or if they're not the top-level element.
Attributes: Don't stringify attributes in the setter
Attributes: Don't stringify attributes in the setter
Attributes: Don't stringify attributes in the setter
Attributes: Don't stringify attributes in the setter
[-]jQuery.attr: Don't stringify values as it breaks Trusted Types[/-][+]Attributes: Don't stringify values as it breaks Trusted Types[/+]mgol commentedon Nov 1, 2021
The fix has landed; I extracted the script execution issue to #4963.