Repository navigation
fix: isolate dynamic JavaScript execution environments #7870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
189ddce
fix: isolate dynamic JavaScript execution environments
somiljain2006 1ba8aa4
Merged into Eval
somiljain2006 fd46188
Merge branch 'main' into Eval
somiljain2006 c7a072a
Fix playright tests and add token as a dependency
somiljain2006 bec9709
Merge branch 'main' into Eval
somiljain2006 d2233c4
style: format dynamic execution tests
somiljain2006 7930558
Merge branch 'main' into Eval
somiljain2006 4394cb1
Move tests
somiljain2006 6759608
Merge branch 'main' into Eval
somiljain2006 c868dde
Pass every state.js export to server-provided scripts
FarhanAliRaza 0c2cc28
Replace the two evaluators with one runScript helper
FarhanAliRaza 425e405
Describe the actual script scope in the news fragment
FarhanAliRaza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
1 change: 1 addition & 0 deletions
1
packages/reflex-base/news/+eval-minified-shared-chunk.performance.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Shrink the shared JavaScript chunk by about 88 KB (11 KB gzipped) when `frontend_lazy_bundled_libraries` is on: server-provided scripts and callbacks no longer use a direct `eval`, so the minifier can shorten every name in that chunk. Scripts passed to `rx.call_script` and `rx.call_function` now see only `addEvents`, `ReflexEvent`, `applyEventActions` and the globals, not other internals of the runtime module. | ||
|
FarhanAliRaza marked this conversation as resolved.
Resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| /** | ||
| * Server-provided JavaScript (`_call_script`, `_call_function` and their | ||
| * callbacks) runs through a Function-constructor helper, not a direct eval, so | ||
| * the minifier may mangle this module. These pin what the generated strings | ||
| * can see and what the script returns. | ||
| */ | ||
| import { describe, expect, test, vi } from "vitest"; | ||
|
|
||
| import { eventLoop } from "$/utils/context-registry"; | ||
| import { applyEvent, getToken } from "$/utils/state"; | ||
|
|
||
| const callScript = (code, callback, socket = null) => | ||
| applyEvent( | ||
| { name: "_call_script", payload: { javascript_code: code, callback } }, | ||
| socket, | ||
| null, | ||
| null, | ||
| ); | ||
|
|
||
| describe("_call_script", () => { | ||
| test("returns the completion value of a statement list to the callback", async () => { | ||
| const seen = []; | ||
| await callScript("var x = 20; x + 1", (result) => seen.push(result)); | ||
| expect(seen).toEqual([21]); | ||
| }); | ||
|
|
||
| test("callback strings dispatch through the late-bound addEvents", async () => { | ||
| eventLoop.addEvents = vi.fn(); | ||
| await callScript( | ||
| "42", | ||
| '((_result) => addEvents([ReflexEvent("state.set", {v: _result})], [_result], {}))', | ||
| ); | ||
| expect(eventLoop.addEvents).toHaveBeenCalledTimes(1); | ||
| const [events, args] = eventLoop.addEvents.mock.calls[0]; | ||
| expect(events[0].name).toBe("state.set"); | ||
| expect(events[0].payload).toEqual({ v: 42 }); | ||
| expect(args).toEqual([42]); | ||
| }); | ||
|
|
||
| test("runs strict and sees no other module-local names", async () => { | ||
| const onerror = vi.fn(); | ||
| window.onerror = onerror; | ||
| await callScript("undeclared = 1"); | ||
| await callScript("eventLoop"); | ||
| expect(onerror).toHaveBeenCalledTimes(2); | ||
| expect(onerror.mock.calls[0][4]).toBeInstanceOf(ReferenceError); | ||
| expect(onerror.mock.calls[1][4]).toBeInstanceOf(ReferenceError); | ||
| }); | ||
|
|
||
| test("scripts keep the socket, navigate, params and token names", async () => { | ||
| // Apps written against direct eval call socket.disconnect() and read the | ||
| // session token from a script. | ||
| const socket = { disconnect: vi.fn() }; | ||
| const token = getToken(); | ||
| expect(typeof token).toBe("string"); | ||
| const seen = []; | ||
| await callScript( | ||
| "socket.disconnect(); [typeof navigate, typeof params, token]", | ||
| (result) => seen.push(result), | ||
| socket, | ||
| ); | ||
| expect(socket.disconnect).toHaveBeenCalledTimes(1); | ||
| expect(seen).toEqual([["object", "object", token]]); | ||
| }); | ||
|
|
||
| test("callback strings can use every helper this module exports", async () => { | ||
| // Var operations inside a callback compile to helpers such as pyMod, | ||
| // isTrue or pyStrLength, which compiled pages import but a string cannot. | ||
| eventLoop.addEvents = vi.fn(); | ||
| await callScript( | ||
| "7", | ||
| '((_result) => addEvents([ReflexEvent("s.set", {x: pyMod(_result, 2), y: isTrue(_result), z: pyStrLength("ab")})]))', | ||
| ); | ||
| expect(eventLoop.addEvents).toHaveBeenCalledTimes(1); | ||
| expect(eventLoop.addEvents.mock.calls[0][0][0].payload).toEqual({ | ||
| x: 1, | ||
| y: true, | ||
| z: 2, | ||
| }); | ||
| }); | ||
|
|
||
| test("callbacks dispatch through the event loop mounted at call time", async () => { | ||
| const first = vi.fn(); | ||
| const second = vi.fn(); | ||
| const event = { payload: { callback: "(value) => addEvents([value])" } }; | ||
| eventLoop.addEvents = first; | ||
| await callScript("1", event.payload.callback); | ||
| eventLoop.addEvents = second; | ||
| await callScript("2", event.payload.callback); | ||
| expect(first).toHaveBeenCalledWith([1]); | ||
| expect(second).toHaveBeenCalledWith([2]); | ||
| }); | ||
|
|
||
| test("the evaluator is compiled on first use, not on import", async () => { | ||
| // A Content Security Policy without unsafe-eval rejects the Function | ||
| // constructor; apps that never send scripts must still load. | ||
| vi.resetModules(); | ||
| const RealFunction = globalThis.Function; | ||
| globalThis.Function = function () { | ||
| throw new EvalError("blocked by CSP"); | ||
| }; | ||
| try { | ||
| await expect(import("$/utils/state")).resolves.toBeTruthy(); | ||
| } finally { | ||
| globalThis.Function = RealFunction; | ||
| } | ||
| }); | ||
|
|
||
| test("_call_function evaluates a string and calls it", async () => { | ||
| const seen = []; | ||
| await applyEvent( | ||
| { | ||
| name: "_call_function", | ||
| payload: { function: "() => 7", callback: (r) => seen.push(r) }, | ||
| }, | ||
| null, | ||
| null, | ||
| null, | ||
| ); | ||
| expect(seen).toEqual([7]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.