krausest / js-framework-benchmark

A comparison of the performance of a few popular javascript frameworks
https://krausest.github.io/js-framework-benchmark/
Apache License 2.0
6.77k stars 839 forks source link

Check #1139 flag #1148

Closed krausest closed 11 months ago

krausest commented 1 year ago

@mmichlin66 reported that mimbl might be flagged incorrectly.

The flag was added in #1142 after a manual check.

I thought about creating an automated check. The idea would be to serve the index.html with Content-Security-Policy: default-src 'self' Then those frameworks using unsafe eval or new Function() should fail due to the CSP (dead code containing an eval would not be detected) @WebReflection Do you consider this a valid check?

krausest commented 1 year ago

The following frameworks fail with CSP (= marked as 1139 an found in the test, < only in 1142 no CSP violation in test, > CSP check fails not in 1142)

Update: I implemented the CSP with report-uri. This allows me to catch cases where the CSP is printed only as a error in the console. I failed to get those errors from only puppet.

= alpine-v3.10.2-keyed 
> bdc-v1.0.0-keyed
= blazor-wasm-v6.0.1-keyed 
= blazor-wasm-aot-v6.0.1-keyed
= dioxus-v0.2.4-keyed
= dojo
= dominator-v0.5.0-keyed
= doohtml-keyed
= doz
= elm-v0.19.1-3-keyed 
< ember
< glimmer-2
< glimmer
< gryon
< imba
= knockout-v3.5.0-keyed
= ko-jsx-v0.16.1-keyed
= leptos-v0.0.17-keyed
= lui-v1.2.3-keyed 
= mikado-v0.7.64-keyed 
< marionette-backbone
< marionette
= mikado
< mimbl
= miso
= misojs-v1.1.0.0-keyed 
< mithril
< ractive
< react-easy-state
< react-hooks-use-transition
< react-hooks
< react-lab
= react-mylyn
< react-mobX
<react-recoil
<react-redux-hooks-immutable
<react-redux-hooks
<react-redux-rematch
<react-redux
<react-rxjs
<react-starbeam
<react-tagged-state
<react-tracked
<react-zustand
<reagent
> react-mlyn-v0.5.11-keyed 
= resonatejs-keyed 
=san-v3.12.0-keyed 
=san-composition-v3.10.1 + 1.1.0-keyed 
=san-store-v3.12.0 + 2.1.3-keyed 
=scarlets-frame-v0.34.6-keyed 
=sifrr-v0.0.5-keyed 
= sledgehammer-v1.0.0-keyed 
= spair-v0.0.8-keyed 
= spair-qr-v0.0.8-keyed 
= stencil
> stdweb-v0.4.17-keyed
= sycamore-v0.8.0-keyed 
> ui5-webcomponents
= voby-v0.43.8-keyed 
< vue
= wasm-bindgen-v0.2.47-keyed 
< whatsup
= yew-v0.20.0-keyed 
= yew-hooks-v0.19.3-keyed 

...non-keyed left out

WebReflection commented 1 year ago

@krausest as discussed, even if it’s the bundler adding eval the library should be flagged (hint to update their tools) but my metrics weren’t strictly testing CSP so this seems to me a better approach.

my only thinking is that some library uses eval in other paths this benchmark might not touch and, as developer, I’d rather like to be informed. I don’t have an automated solution for this though.

krausest commented 1 year ago

I worked around the cases I couldn't detect with playwright by adding a CSP with a report-uri and collecting the urls on the serverside. I updated the table above (i.e. mostly some < changed into =)

@WebReflection Can you please check mimbl again? It works with a CSP and I couldn't find an eval or Function in the dist code.

krausest commented 1 year ago

Flag for mimbl was removed.

krausest commented 1 year ago

@yyx990803 asked why the flag was added for vue. So I decided to use the automated test instead of the manual check.

The test works in the following way:

  1. The server can be switched into a mode in which a CSP header is added (call http://localhost:8080/startCSP and to stop adding the CSP header call http://localhost:8080/endCSP)
  2. The CSP header contains a CSP report URI that reports violations back to the server. (This was necessary since I couldn't detect all CSP errors on the client side). The server records the list of frameworks that violated the CSP.
  3. There's a new target npm run checkCSP or (npm run checkCSP -- --headless) that opens the page for each framework, clicks the #add button and checks that 1000 rows were added
  4. The checkCSP driver fetches the list of frameworks with CSP violations and prints those.

Currently it reports violations for all the frameworks below: keyed/alpine/ keyed/bdc/ keyed/blazor-wasm/ keyed/blazor-wasm-aot/ keyed/dioxus/ keyed/dojo/ keyed/dominator/ keyed/doohtml/ keyed/doz/ keyed/elm/ keyed/knockout/ keyed/ko-jsx/ keyed/leptos/ keyed/lui/ keyed/mikado/ keyed/miso/ keyed/misojs/ keyed/mogwai/ keyed/react-mlyn/ keyed/resonatejs/ keyed/riot/ keyed/san/ keyed/san-composition/ keyed/san-store/ keyed/scarlets-frame/ keyed/sifrr/ keyed/silkenweb/ keyed/sledgehammer/ keyed/spair/ keyed/spair-qr/ keyed/stdweb/ keyed/stencil/ keyed/sycamore/ keyed/udomsay-tpl/ keyed/ui5-webcomponents/ keyed/voby/ keyed/wasm-bindgen/ keyed/yew/ keyed/yew-hooks/ non-keyed/aurelia/ non-keyed/bdc/ non-keyed/delorean/ non-keyed/dojo/ non-keyed/doohtml/ non-keyed/doz/ non-keyed/elm/ non-keyed/mikado/ non-keyed/miso/ non-keyed/mogwai/ non-keyed/san/ non-keyed/sauron/ non-keyed/scarlets-frame/ non-keyed/seed/ non-keyed/sifrr/ non-keyed/slim-js/ non-keyed/stdweb/ non-keyed/ui5-webcomponents/

I've updated the results according to that list.

@WebReflection Maybe we're losing some frameworks that carry an eval in the code that you found, but as can be seen by the report from Evan or Michael I feel I must be able to automatically test for that behaviour (there enough potential for errors in those automated tests...)

@yyx990803 Sorry for adding the flag to vue!

WebReflection commented 1 year ago

@krausest I’m ok with real-world results on CSP, maybe there could be a flag for “potentially need evaluation without tooling” but admittedly that’s out of this benchmark scope.

krausest commented 1 year ago

@schell asked why mogwai is flagged.

Loading mogwai with CSP enabled causes the following error:

index.html:13 Refused to execute inline script because it violates the following Content Security Policy directive: "default-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-1R96SQxWO1t9o2YhsMsU4k8Jj7WL/Ec8F/JRyOkUByI='), or a nonce ('nonce-...') is required to enable inline execution. Note also that 'script-src' was not explicitly set, so 'default-src' is used as a fallback.

Looks like this means all WASM implementations fail with CSP enabled.

So I suggest that we change the CSP to default-src 'self' 'wasm-unsafe-eval'; report-uri /csp.

krausest commented 1 year ago

Seems like wasm currently needs default-src 'self' 'unsafe-eval' 'unsafe-inline'; report-uri /csp. So I'd say the flag is justified for the webassembly implementations.

WebReflection commented 1 year ago

Yes, the flag is there to underline special CSP rules are needed and WASM needs that ... I think it's fair to have those flags around, as that's (unfortunately) the current state of WASM out there.

schell commented 1 year ago

Thanks for the explanation.