feat: remove BrowserWindow option inheritance (#28550)

This commit is contained in:
Jeremy Rose 2021-04-21 10:55:17 -07:00 committed by GitHub
parent c4931ff70e
commit 4ca518468d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 128 additions and 118 deletions

View file

@ -203,9 +203,9 @@ Returns:
* `frameName` String - Name given to the created window in the * `frameName` String - Name given to the created window in the
`window.open()` call. `window.open()` call.
* `options` BrowserWindowConstructorOptions - The options used to create the * `options` BrowserWindowConstructorOptions - The options used to create the
BrowserWindow. They are merged in increasing precedence: options inherited BrowserWindow. They are merged in increasing precedence: parsed options
from the parent, parsed options from the `features` string from from the `features` string from `window.open()`, security-related
`window.open()`, and options given by webPreferences inherited from the parent, and options given by
[`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler). [`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler).
Unrecognized options are not filtered out. Unrecognized options are not filtered out.
* `referrer` [Referrer](structures/referrer.md) - The referrer that will be * `referrer` [Referrer](structures/referrer.md) - The referrer that will be
@ -1148,8 +1148,11 @@ Ignore application menu shortcuts while this web contents is focused.
without a recognized 'action' value will result in a console error and have without a recognized 'action' value will result in a console error and have
the same effect as returning `{action: 'deny'}`. the same effect as returning `{action: 'deny'}`.
Called before creating a window when `window.open()` is called from the Called before creating a window a new window is requested by the renderer, e.g.
renderer. See [`window.open()`](window-open.md) for more details and how to use this in conjunction with `did-create-window`. by `window.open()`, a link with `target="_blank"`, shift+clicking on a link, or
submitting a form with `<form target="_blank">`. See
[`window.open()`](window-open.md) for more details and how to use this in
conjunction with `did-create-window`.
#### `contents.setAudioMuted(muted)` #### `contents.setAudioMuted(muted)`

View file

@ -22,9 +22,8 @@ BrowserWindow in the main process by using `webContents.setWindowOpenHandler()`
for renderer-created windows. for renderer-created windows.
BrowserWindow constructor options are set by, in increasing precedence BrowserWindow constructor options are set by, in increasing precedence
order: options inherited from the parent, parsed options order: parsed options from the `features` string from `window.open()`,
from the `features` string from `window.open()`, security-related webPreferences security-related webPreferences inherited from the parent, and options given by
inherited from the parent, and options given by
[`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler). [`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler).
Note that `webContents.setWindowOpenHandler` has final say and full privilege Note that `webContents.setWindowOpenHandler` has final say and full privilege
because it is invoked in the main process. because it is invoked in the main process.

View file

@ -38,6 +38,27 @@ to open synchronously scriptable child windows, among other incompatibilities.
See the documentation for [window.open in Electron](api/window-open.md) See the documentation for [window.open in Electron](api/window-open.md)
for more details. for more details.
### Removed: BrowserWindowConstructorOptions inheriting from parent windows
Prior to Electron 14, windows opened with `window.open` would inherit
BrowserWindow constructor options such as `transparent` and `resizable` from
their parent window. Beginning with Electron 14, this behavior is removed, and
windows will not inherit any BrowserWindow constructor options from their
parents.
Instead, explicitly set options for the new window with `setWindowOpenHandler`:
```js
webContents.setWindowOpenHandler((details) => {
return {
action: 'allow',
overrideBrowserWindowOptions: {
// ...
}
}
})
```
### Removed: `additionalFeatures` ### Removed: `additionalFeatures`
The deprecated `additionalFeatures` property in the `new-window` and The deprecated `additionalFeatures` property in the `new-window` and

View file

@ -198,39 +198,37 @@ const securityWebPreferences: { [key: string]: boolean } = {
enableWebSQL: false enableWebSQL: false
}; };
function makeBrowserWindowOptions ({ embedder, features, overrideOptions, useDeprecatedBehaviorForOptionInheritance = true }: { function makeBrowserWindowOptions ({ embedder, features, overrideOptions }: {
embedder: WebContents, embedder: WebContents,
features: string, features: string,
overrideOptions?: BrowserWindowConstructorOptions, overrideOptions?: BrowserWindowConstructorOptions,
useDeprecatedBehaviorForOptionInheritance?: boolean
}) { }) {
const { options: parsedOptions, webPreferences: parsedWebPreferences } = parseFeatures(features); const { options: parsedOptions, webPreferences: parsedWebPreferences } = parseFeatures(features);
const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder);
return { return {
options: { options: {
...(useDeprecatedBehaviorForOptionInheritance && deprecatedInheritedOptions),
show: true, show: true,
width: 800, width: 800,
height: 600, height: 600,
...parsedOptions, ...parsedOptions,
...overrideOptions, ...overrideOptions,
webPreferences: makeWebPreferences({ embedder, insecureParsedWebPreferences: parsedWebPreferences, secureOverrideWebPreferences: overrideOptions && overrideOptions.webPreferences, useDeprecatedBehaviorForOptionInheritance: true }) webPreferences: makeWebPreferences({
embedder,
insecureParsedWebPreferences: parsedWebPreferences,
secureOverrideWebPreferences: overrideOptions && overrideOptions.webPreferences
})
} as Electron.BrowserViewConstructorOptions } as Electron.BrowserViewConstructorOptions
}; };
} }
export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {}, insecureParsedWebPreferences: parsedWebPreferences = {}, useDeprecatedBehaviorForOptionInheritance = true }: { export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {}, insecureParsedWebPreferences: parsedWebPreferences = {} }: {
embedder: WebContents, embedder: WebContents,
insecureParsedWebPreferences?: ReturnType<typeof parseFeatures>['webPreferences'], insecureParsedWebPreferences?: ReturnType<typeof parseFeatures>['webPreferences'],
// Note that override preferences are considered elevated, and should only be // Note that override preferences are considered elevated, and should only be
// sourced from the main process, as they override security defaults. If you // sourced from the main process, as they override security defaults. If you
// have unvetted prefs, use parsedWebPreferences. // have unvetted prefs, use parsedWebPreferences.
secureOverrideWebPreferences?: BrowserWindowConstructorOptions['webPreferences'], secureOverrideWebPreferences?: BrowserWindowConstructorOptions['webPreferences'],
useDeprecatedBehaviorForOptionInheritance?: boolean
}) { }) {
const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder);
const parentWebPreferences = embedder.getLastWebPreferences(); const parentWebPreferences = embedder.getLastWebPreferences();
const securityWebPreferencesFromParent = (Object.keys(securityWebPreferences).reduce((map, key) => { const securityWebPreferencesFromParent = (Object.keys(securityWebPreferences).reduce((map, key) => {
if (securityWebPreferences[key] === parentWebPreferences[key as keyof Electron.WebPreferences]) { if (securityWebPreferences[key] === parentWebPreferences[key as keyof Electron.WebPreferences]) {
@ -241,7 +239,6 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
const openerId = parentWebPreferences.nativeWindowOpen ? null : embedder.id; const openerId = parentWebPreferences.nativeWindowOpen ? null : embedder.id;
return { return {
...(useDeprecatedBehaviorForOptionInheritance && deprecatedInheritedOptions ? deprecatedInheritedOptions.webPreferences : null),
...parsedWebPreferences, ...parsedWebPreferences,
// Note that order is key here, we want to disallow the renderer's // Note that order is key here, we want to disallow the renderer's
// ability to change important security options but allow main (via // ability to change important security options but allow main (via
@ -254,25 +251,6 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
}; };
} }
/**
* Current Electron behavior is to inherit all options from the parent window.
* In practical use, this is kind of annoying because consumers have to know
* about the parent window's preferences in order to unset them and makes child
* windows even more of an anomaly. In 11.0.0 we will remove this behavior and
* only critical security preferences will be inherited by default.
*/
function getDeprecatedInheritedOptions (embedder: WebContents) {
if (!embedder.browserWindowOptions) {
// If it's a webview, return just the webPreferences.
return {
webPreferences: embedder.getLastWebPreferences()
};
}
const { type, show, ...inheritableOptions } = embedder.browserWindowOptions;
return inheritableOptions;
}
function formatPostDataHeaders (postData: PostData) { function formatPostDataHeaders (postData: PostData) {
if (!postData) return; if (!postData) return;

View file

@ -135,7 +135,7 @@ handleMessage(
if (!windowMethods.has(method)) { if (!windowMethods.has(method)) {
console.error( console.error(
`Blocked ${event.sender.getURL()} from calling method: ${method}` `Blocked ${event.senderFrame.url} from calling method: ${method}`
); );
throw new Error(`Invalid method: ${method}`); throw new Error(`Invalid method: ${method}`);
} }

View file

@ -270,7 +270,24 @@ export const windowSetup = (
if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['open'], window.open); if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['open'], window.open);
} }
if (openerId != null) { // If this window uses nativeWindowOpen, but its opener window does not, we
// need to proxy window.opener in order to let the page communicate with its
// opener.
// Additionally, windows opened from a nativeWindowOpen child of a
// non-nativeWindowOpen parent will initially have their WebPreferences
// copied from their opener before having them updated, meaning openerId is
// initially incorrect. We detect this situation by checking for
// window.opener, which will be non-null for a natively-opened child, so we
// can ignore the openerId in that case, since it's incorrectly copied from
// the parent. This is, uh, confusing, so here's a diagram that will maybe
// help?
//
// [ grandparent window ] --> [ parent window ] --> [ child window ]
// n.W.O = false n.W.O = true n.W.O = true
// id = 1 id = 2 id = 3
// openerId = null openerId = 1 openerId = 1 <- !!wrong!!
// opener = null opener = null opener = [parent window]
if (openerId != null && !window.opener) {
window.opener = getOrCreateProxy(openerId); window.opener = getOrCreateProxy(openerId);
if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['opener'], window.opener); if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['opener'], window.opener);
} }

View file

@ -78,10 +78,6 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
api_web_contents_->AddObserver(this); api_web_contents_->AddObserver(this);
Observe(api_web_contents_->web_contents()); Observe(api_web_contents_->web_contents());
// Keep a copy of the options for later use.
gin_helper::Dictionary(isolate, web_contents.ToV8().As<v8::Object>())
.Set("browserWindowOptions", options);
// Associate with BrowserWindow. // Associate with BrowserWindow.
web_contents->SetOwnerWindow(window()); web_contents->SetOwnerWindow(window());

View file

@ -2383,7 +2383,7 @@ describe('BrowserWindow module', () => {
} }
}); });
let childWc: WebContents | null = null; let childWc: WebContents | null = null;
w.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { preload } } })); w.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { preload, contextIsolation: false } } }));
w.webContents.on('did-create-window', (win) => { w.webContents.on('did-create-window', (win) => {
childWc = win.webContents; childWc = win.webContents;
@ -2598,6 +2598,10 @@ describe('BrowserWindow module', () => {
preload preload
} }
}); });
w.webContents.setWindowOpenHandler(() => ({
action: 'allow',
overrideBrowserWindowOptions: { show: false, webPreferences: { contextIsolation: false, webviewTag: true, nativeWindowOpen: true, nodeIntegrationInSubFrames: true } }
}));
w.webContents.once('new-window', (event, url, frameName, disposition, options) => { w.webContents.once('new-window', (event, url, frameName, disposition, options) => {
options.show = false; options.show = false;
}); });
@ -2676,7 +2680,9 @@ describe('BrowserWindow module', () => {
action: 'allow', action: 'allow',
overrideBrowserWindowOptions: { overrideBrowserWindowOptions: {
webPreferences: { webPreferences: {
preload: path.join(fixtures, 'api', 'window-open-preload.js') preload: path.join(fixtures, 'api', 'window-open-preload.js'),
contextIsolation: false,
nodeIntegrationInSubFrames: true
} }
} }
})); }));

View file

@ -1042,7 +1042,7 @@ describe('chromium features', () => {
const parentCode = `new Promise((resolve) => { const parentCode = `new Promise((resolve) => {
// This is context (3), a child window of the WebView. // This is context (3), a child window of the WebView.
const child = window.open(${JSON.stringify(child)}, "", "show=no") const child = window.open(${JSON.stringify(child)}, "", "show=no,contextIsolation=no,nodeIntegration=yes")
window.addEventListener("message", e => { window.addEventListener("message", e => {
resolve(e.data) resolve(e.data)
}) })

View file

@ -8,27 +8,23 @@
"frame-name", "frame-name",
"new-window", "new-window",
{ {
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true,
"nodeIntegration": false,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": true, "show": true,
"width": 800,
"height": 600, "height": 600,
"top": 5, "top": 5,
"left": 10, "left": 10,
"resizable": false, "resizable": false,
"x": 10, "x": 10,
"y": 5, "y": 5,
"webPreferences": {
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"webContents": "[WebContents]" "webContents": "[WebContents]"
}, },
[], [],
@ -47,26 +43,22 @@
"frame-name", "frame-name",
"new-window", "new-window",
{ {
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"zoomFactor": "2",
"contextIsolation": true,
"nodeIntegration": false,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": true, "show": true,
"width": 800,
"height": 600, "height": 600,
"resizable": false, "resizable": false,
"x": 0, "x": 0,
"y": 10, "y": 10,
"webPreferences": {
"zoomFactor": "2",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"webContents": "[WebContents]" "webContents": "[WebContents]"
}, },
[], [],
@ -85,22 +77,20 @@
"frame-name", "frame-name",
"new-window", "new-window",
{ {
"show": true,
"width": 800, "width": 800,
"title": "cool", "height": 600,
"backgroundColor": "gray", "backgroundColor": "gray",
"focusable": false,
"webPreferences": { "webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "gray",
"contextIsolation": true, "contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false, "nodeIntegration": false,
"sandbox": true,
"webviewTag": false, "webviewTag": false,
"nodeIntegrationInSubFrames": false, "nodeIntegrationInSubFrames": false,
"openerId": null "openerId": null,
"backgroundColor": "gray"
}, },
"show": true,
"height": 600,
"x": 100, "x": 100,
"y": 100, "y": 100,
"webContents": "[WebContents]" "webContents": "[WebContents]"
@ -121,24 +111,21 @@
"frame-name", "frame-name",
"new-window", "new-window",
{ {
"show": true,
"width": 800, "width": 800,
"height": 600,
"x": 50,
"y": 20,
"title": "sup", "title": "sup",
"backgroundColor": "blue",
"focusable": false,
"webPreferences": { "webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true, "contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false, "nodeIntegration": false,
"sandbox": true,
"webviewTag": false, "webviewTag": false,
"nodeIntegrationInSubFrames": false, "nodeIntegrationInSubFrames": false,
"openerId": null "openerId": null
}, },
"show": true,
"height": 600,
"x": 50,
"y": 20,
"webContents": "[WebContents]" "webContents": "[WebContents]"
}, },
[], [],
@ -157,26 +144,22 @@
"frame-name", "frame-name",
"new-window", "new-window",
{ {
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true,
"nodeIntegration": false,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": false, "show": false,
"width": 800,
"height": 600, "height": 600,
"top": 1, "top": 1,
"left": 1, "left": 1,
"x": 1, "x": 1,
"y": 1, "y": 1,
"webPreferences": {
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"webContents": "[WebContents]" "webContents": "[WebContents]"
}, },
[], [],

View file

@ -91,7 +91,7 @@ describe('chromium feature', () => {
slashes: true slashes: true
}); });
const message = waitForEvent(window, 'message'); const message = waitForEvent(window, 'message');
const b = window.open(windowUrl, '', 'nodeIntegration=no,show=no'); const b = window.open(windowUrl, '', 'nodeIntegration=no,contextIsolation=no,show=no');
const event = await message; const event = await message;
b.close(); b.close();
expect(event.data.isProcessGlobalUndefined).to.be.true(); expect(event.data.isProcessGlobalUndefined).to.be.true();
@ -107,7 +107,7 @@ describe('chromium feature', () => {
slashes: true slashes: true
}); });
const message = waitForEvent(window, 'message'); const message = waitForEvent(window, 'message');
const b = window.open(windowUrl, '', 'webviewTag=no,nodeIntegration=yes,show=no'); const b = window.open(windowUrl, '', 'webviewTag=no,contextIsolation=no,nodeIntegration=yes,show=no');
const event = await message; const event = await message;
b.close(); b.close();
expect(event.data.isWebViewGlobalUndefined).to.be.true(); expect(event.data.isWebViewGlobalUndefined).to.be.true();
@ -218,7 +218,7 @@ describe('chromium feature', () => {
it('delivers messages that match the origin', async () => { it('delivers messages that match the origin', async () => {
const message = waitForEvent(window, 'message'); const message = waitForEvent(window, 'message');
const b = window.open(serverURL, '', 'show=no'); const b = window.open(serverURL, '', 'show=no,contextIsolation=no,nodeIntegration=yes');
const event = await message; const event = await message;
b.close(); b.close();
expect(event.data).to.equal('deliver'); expect(event.data).to.equal('deliver');

View file

@ -1,5 +1,5 @@
<script> <script>
const child = window.open(`./window-open-postMessage.html`, '', 'show=no') const child = window.open(`./window-open-postMessage.html`, '', 'show=no,nodeIntegration=yes,contextIsolation=no')
window.onmessage = (e) => { window.onmessage = (e) => {
if (e.data === 'ready') { if (e.data === 'ready') {
child.postMessage('testing', '*') child.postMessage('testing', '*')

View file

@ -2,18 +2,25 @@
<body> <body>
<script type="text/javascript" charset="utf-8"> <script type="text/javascript" charset="utf-8">
const url = require('url') const url = require('url')
function tryPostMessage(...args) {
try {
window.opener.postMessage(...args)
} catch (e) {
console.error(e)
}
}
if (url.parse(window.location.href, true).query.opened != null) { if (url.parse(window.location.href, true).query.opened != null) {
// Ensure origins are properly checked by removing a single character from the end // Ensure origins are properly checked by removing a single character from the end
window.opener.postMessage('do not deliver substring origin', window.location.origin.substring(0, window.location.origin.length - 1)) tryPostMessage('do not deliver substring origin', window.location.origin.substring(0, window.location.origin.length - 1))
window.opener.postMessage('do not deliver file://', 'file://') tryPostMessage('do not deliver file://', 'file://')
window.opener.postMessage('do not deliver http without port', 'http://127.0.0.1') tryPostMessage('do not deliver http without port', 'http://127.0.0.1')
window.opener.postMessage('do not deliver atom', 'atom://') tryPostMessage('do not deliver atom', 'atom://')
window.opener.postMessage('do not deliver null', 'null') tryPostMessage('do not deliver null', 'null')
window.opener.postMessage('do not deliver \\:/', '\\:/') tryPostMessage('do not deliver \\:/', '\\:/')
window.opener.postMessage('do not deliver empty', '') tryPostMessage('do not deliver empty', '')
window.opener.postMessage('deliver', window.location.origin) window.opener.postMessage('deliver', window.location.origin)
} else { } else {
const opened = window.open(`${window.location.href}?opened=true`, '', 'show=no') const opened = window.open(`${window.location.href}?opened=true`, '', 'show=no,contextIsolation=no,nodeIntegration=yes')
window.addEventListener('message', function (event) { window.addEventListener('message', function (event) {
window.opener.postMessage(event.data, '*') window.opener.postMessage(event.data, '*')
opened.close() opened.close()