[DataGrid] Fix eval blocked by CSP

[romgrk]

Closes #9771

Feature-detect eval at runtime and only use it if available.

[romgrk]

Should we add a test for this? Do we have a test setup that can handle adding a CSP header?

[mui-bot]

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-9863--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	311.4	514.4	339.3	388	84.562
Sort 100k rows	ms	461.8	896.8	706.9	707.64	160.402
Select 100k rows	ms	128.9	214.8	185	181.18	29.311
Deselect 100k rows	ms	131	205.8	186.2	173.8	27.783

Generated by 🚫 dangerJS against 2f2351a

[cherniavskii]

Do we have a test setup that can handle adding a CSP header?

Should we add a test for this? It should be fairly easy to add an e2e test that would inject CSP meta tag into the default HTML template on mount 👍

[romgrk]

It should be fairly easy to add an e2e test that would inject CSP meta tag into the default HTML template on mount +1

The current implementation only detects eval once at startup. Should we move the check at each filter call? I don't love it but it's probably not very expensive.

[cherniavskii]

The current implementation only detects eval once at startup. Should we move the check at each filter call? I don't love it but it's probably not very expensive.

Oh, right. Yes, moving the check to the filter call sounds good to me.
Another option is to enable CSP for all e2e tests 🙂

[oliviertassinari]

Would using Function over eval be safer and yield similar performance?

[oliviertassinari]

This means we no longer run any of the assertions below in the tests, no? It seems it's not what we intended.

[romgrk]

AFAICT we're running them twice, eval and non-eval. testEval is synchronous. Unless I'm misunderstanding.

[oliviertassinari]

From what I understood, this would provide the intended behavior:

diff --git a/packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx b/packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx
index e5ab37fa5..2b42b9f7b 100644
--- a/packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx
+++ b/packages/grid/x-data-grid/src/tests/filtering.DataGrid.test.tsx
@@ -44,16 +44,14 @@ describe('<DataGrid /> - Filter', () => {
   let disableEval = false;

   function testEval(fn: Function) {
-    return () => {
-      disableEval = false;
-      fn();
-      disableEval = true;
-      fn();
-      disableEval = false;
-    };
+    fn();
+    disableEval = true;
+    fn();
+    disableEval = false;
   }

[cherniavskii]

I just noticed this when adding new tests 😅
#10587

[oliviertassinari]

Are we sure about this opt-out? eval could be a security hole.

[romgrk]

We have reviewed the eval'd code in #9635 and #9120 and we haven't found any remaining security risk. In particular, we ensure that no user inputted string can end up in the eval'd code without being properly escaped (aka no XSS). There are only two user inputs, applier.item.id and applier.item.field. We pass them both through JSON.stringify, which by the spec returns a valid & properly escaped string representation for JS/JSON.

If we do find a security risk, then it would be best to simply not include the feature.

[oliviertassinari]

Alright 👍

[romgrk]

Would using Function over eval be safer and yield similar performance?

Not safer, it's the same fundamental mechanism, string -> code. eval is needed here to preserve the lexical context, Function creates a function in the global scope.