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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add worldSafe flag for executeJS results #24114
Changes from all commits
6e49e97
7d92ae5
e105af3
4a36562
cf96382
9615819
fa51bf8
42162d2
e407449
b82a906
149b3ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -348,6 +348,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. | |
You can access this context in the dev tools by selecting the | ||
'Electron Isolated Context' entry in the combo box at the top of the | ||
Console tab. | ||
* `worldSafeExecuteJavaScript` Boolean (optional) - If true, values returned from `webFrame.executeJavaScript` will be sanitized to ensure JS values | ||
can't unsafely cross between worlds when using `contextIsolation`. The default | ||
is `false`. In Electron 12, the default will be changed to `true`. _Deprecated_ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MarshallOfSound Why is this marked There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @poiru This option is a migration feature, it's kinda weird and you're right we should probably document it better. The flow here will be
We aren't going to leave this flag around forever / for long at all |
||
* `nativeWindowOpen` Boolean (optional) - Whether to use native | ||
`window.open()`. Defaults to `false`. Child windows will always have node | ||
integration disabled unless `nodeIntegrationInSubFrames` is true. **Note:** This option is currently | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
const { ipcRenderer, webFrame } = require('electron'); | ||
|
||
webFrame.executeJavaScript(`(() => { | ||
return Object(Symbol('a')); | ||
})()`).catch((err) => { | ||
// Considered safe if the object is constructed in this world | ||
ipcRenderer.send('executejs-safe', err); | ||
}).then(() => { | ||
ipcRenderer.send('executejs-safe', null); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
const { ipcRenderer, webFrame } = require('electron'); | ||
|
||
webFrame.executeJavaScript(`(() => { | ||
return {}; | ||
})()`).then((obj) => { | ||
// Considered safe if the object is constructed in this world | ||
ipcRenderer.send('executejs-safe', obj.constructor === Object); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this might be a good test case for the
base::Feature
-based stuff we've been talking about in api-wg?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but I don't super want to figure out that system / pioneer it in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone's gotta be the first :\