Fix CSP violation: use DOM style API instead of setAttribute("style")#288
Fix CSP violation: use DOM style API instead of setAttribute("style")#288eagle-head wants to merge 1 commit intopatrick-steele-idem:masterfrom
Conversation
|
Looks good to me. It would be interesting to know if there's a meaningful difference in morph time on a page that uses a lot of style attributes. If yes, it may be best to add a flag to enable/disable this. |
setAttribute("style", ...) is blocked by Content Security Policy (CSP)
style-src without 'unsafe-inline'. Replace it with style.cssText assignment,
which uses the CSSOM API and is never blocked by CSP.
The fix compares via getAttribute("style") (cheap string read) and writes
via style.cssText (CSP-safe CSSOM setter). This keeps the fast-path for
unchanged styles identical to the original code.
Includes a Docker-based benchmark suite (bench/) for reproducible
performance comparison. See bench/RESULTS.md for full results.
0038617 to
5ff0e67
Compare
|
Thanks for the feedback @SteffenDE! Great call on the benchmark — it revealed an important issue with the initial approach. What changedThe initial implementation (property-by-property // Before (master):
fromNode.setAttribute(attrName, attrValue);
// After (this PR):
fromNode.style.cssText = attrValue;
The comparison still uses Benchmark resultsRan via Docker (Chromium headless, 30 iterations, trimmed mean). Full results in
The common case in frameworks like LiveView (incremental re-renders, 0–10% of elements change style) shows negligible overhead. The 100% changed worst case (all 1000 elements change all 20 style properties simultaneously) is 1.5–2.5x — this is the intrinsic cost of the CSSOM Based on these numbers, I don't think a flag is necessary — but happy to add one if you feel differently. How to reproducenpm run build
docker build -t morphdom-bench ./bench
docker run --rm \
-v "$(pwd)/dist:/morphdom/dist:ro" \
-v "$(pwd)/bench/style-benchmark.html:/bench/style-benchmark.html:ro" \
morphdom-benchSee Let me know what you think — open to any suggestions on the approach. |
|
That sure is an interesting CSP workaround. And yeah, I agree that there's no need to make it configurable. See also w3c/webappsec-csp#774. |
|
Thanks @SteffenDE! Great find on the W3C issue — it's indeed the core of why this works. As discussed there, The ideal long-term solution would be pure CSS — using mechanisms like Until then, |
Fixes #287
Problem
morphAttrsusessetAttribute("style", ...)to sync inline styles during DOM patching. This is blocked by Content Security Policystyle-srcdirectives that do not include'unsafe-inline', producing violations on every morph that touches a style attribute.Solution
Add a
syncStyle()function tomorphAttrs.jsthat uses the DOM style API instead ofsetAttribute:style.removeProperty()(iterating backwards sinceCSSStyleDeclarationis a live list)style.setProperty(name, value, priority), only when value or priority differsfromNode.style.cssTextwithtoNode.style.cssTextfirst — if they match, skip the sync entirely. This comparison uses browser-normalized strings, though shorthand expansion differences between attached and detached nodes may cause false positives (harmless, sincesyncStyleguards each property individually)The DOM style API (
style.setProperty,style.removeProperty) is explicitly exempt from CSP restrictions per the W3C CSP3 spec and MDN documentation.What changed
src/morphAttrs.js—syncStyle()function + style branch in the attribute looptest/browser/test.js— 8 new tests indescribe('CSP-safe style handling'), including a spy verifyingsetAttribute("style")is never calledtest/fixtures/autotest/style-{change,add,remove,css-variable}/— 4 autotest fixturesWhat did NOT change
setAttribute— no behavioral changeremoveAttribute("style")— this is not restricted by CSPmorphdom.js,specialElHandlers.js, or any other filedist/not included — left for the maintainer to rebuildHandles
--value,--size, etc.)!importantpriority (preserved and cleared correctly)Context
This issue was identified while implementing CSP Level 3 support for Phoenix LiveView. Phoenix LiveView uses morphdom as its DOM patching engine, and
setAttribute("style")is the only CSP-violating call in the morph pipeline. This fix unblocks strict CSP adoption for all morphdom consumers.Discussion: Elixir Forum — CSP Level 3 support for Phoenix
@SteffenDE (who contributed recent morphdom PRs #279, #277, #269, #268) confirmed interest in this approach on the Elixir Forum.