From 777704e6593e9fca4b34fc1d3d405eda5df07167 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 23 Mar 2016 16:53:20 -0700 Subject: [PATCH 01/17] Add failing spec for nodeIntegration inheritance --- spec/chromium-spec.js | 21 +++++++++++++++++++ .../window-opener-no-node-integration.html | 15 +++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 spec/fixtures/pages/window-opener-no-node-integration.html diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 27660156e325..bb76cfb2a8e0 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -182,6 +182,27 @@ describe('chromium feature', function () { b = window.open('file://' + fixtures + '/pages/window-open-size.html', '', 'show=no') }) + it('disables node integration when it is disabled on the parent window', function (done) { + var b + listener = function (event) { + assert.equal(event.data, 'undefined') + b.close() + done() + } + window.addEventListener('message', listener) + + var windowUrl = require('url').format({ + pathname: fixtures + "/pages/window-opener-no-node-integration.html", + protocol: 'file', + query: { + p: fixtures + "/pages/window-opener-node.html" + }, + slashes: true + }) + console.log(windowUrl) + b = window.open(windowUrl, 'nodeIntegration=no,show=no') + }) + it('does not override child options', function (done) { var b, size size = { diff --git a/spec/fixtures/pages/window-opener-no-node-integration.html b/spec/fixtures/pages/window-opener-no-node-integration.html new file mode 100644 index 000000000000..e146a2804ae5 --- /dev/null +++ b/spec/fixtures/pages/window-opener-no-node-integration.html @@ -0,0 +1,15 @@ + + + + + From 463e077c3af77f47312ec736206d886e9089a47c Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 23 Mar 2016 17:36:26 -0700 Subject: [PATCH 02/17] Disable node on child when disabled on parent --- lib/browser/guest-window-manager.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 0426ea51583e..467e819fa826 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -28,6 +28,11 @@ var mergeBrowserWindowOptions = function (embedder, options) { if (embedder.browserWindowOptions != null) { // Inherit the original options if it is a BrowserWindow. mergeOptions(options, embedder.browserWindowOptions) + + // Disable node integration on child window if disabled on parent window + if (!embedder.browserWindowOptions.webPreferences.nodeIntegration) { + options.webPreferences.nodeIntegration = false + } } else { // Or only inherit web-preferences if it is a webview. if (options.webPreferences == null) { From 2713580d09aacee4fa352dbab26ee55781bdee01 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 23 Mar 2016 17:38:30 -0700 Subject: [PATCH 03/17] Remove stray log --- spec/chromium-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index bb76cfb2a8e0..9a2ba80fb073 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -199,7 +199,6 @@ describe('chromium feature', function () { }, slashes: true }) - console.log(windowUrl) b = window.open(windowUrl, 'nodeIntegration=no,show=no') }) From d7b1792503753fde417f47772e6d21ebd97f512f Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 23 Mar 2016 17:40:25 -0700 Subject: [PATCH 04/17] Use template strings --- spec/chromium-spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 9a2ba80fb073..507101dc93b9 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -192,10 +192,10 @@ describe('chromium feature', function () { window.addEventListener('message', listener) var windowUrl = require('url').format({ - pathname: fixtures + "/pages/window-opener-no-node-integration.html", + pathname: `${fixtures}/pages/window-opener-no-node-integration.html`, protocol: 'file', query: { - p: fixtures + "/pages/window-opener-node.html" + p: `${fixtures}/pages/window-opener-node.html` }, slashes: true }) From 9e66df23d0bbee093d7b32327cb6bf5e044d394c Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 24 Mar 2016 12:29:54 -0700 Subject: [PATCH 05/17] Add clearer assertion of process being undefined --- spec/chromium-spec.js | 4 ++-- spec/fixtures/pages/window-opener-node.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 507101dc93b9..aa4e5c5fdaf5 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -160,7 +160,7 @@ describe('chromium feature', function () { it('accepts "nodeIntegration" as feature', function (done) { var b listener = function (event) { - assert.equal(event.data, 'undefined') + assert.equal(event.data.isProcessGlobalUndefined, true) b.close() done() } @@ -185,7 +185,7 @@ describe('chromium feature', function () { it('disables node integration when it is disabled on the parent window', function (done) { var b listener = function (event) { - assert.equal(event.data, 'undefined') + assert.equal(event.data.isProcessGlobalUndefined, true) b.close() done() } diff --git a/spec/fixtures/pages/window-opener-node.html b/spec/fixtures/pages/window-opener-node.html index 118603c82d3b..130360728767 100644 --- a/spec/fixtures/pages/window-opener-node.html +++ b/spec/fixtures/pages/window-opener-node.html @@ -1,7 +1,7 @@ From dbe1c1d4e42ef87bd87090db05eb42ad2499fd71 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 24 Mar 2016 13:22:13 -0700 Subject: [PATCH 06/17] Check nodeIntegration on embedder's webPreferences --- lib/browser/guest-window-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 467e819fa826..f27bb8329fa4 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -30,7 +30,7 @@ var mergeBrowserWindowOptions = function (embedder, options) { mergeOptions(options, embedder.browserWindowOptions) // Disable node integration on child window if disabled on parent window - if (!embedder.browserWindowOptions.webPreferences.nodeIntegration) { + if (!embedder.getWebPreferences().nodeIntegration) { options.webPreferences.nodeIntegration = false } } else { From e5164d2255d82cd8a33e7ce91a28d5e2f5df96e3 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 24 Mar 2016 13:27:03 -0700 Subject: [PATCH 07/17] Check of nodeIntegration is strictly equal to false --- lib/browser/guest-window-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index f27bb8329fa4..5782670ac639 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -30,7 +30,7 @@ var mergeBrowserWindowOptions = function (embedder, options) { mergeOptions(options, embedder.browserWindowOptions) // Disable node integration on child window if disabled on parent window - if (!embedder.getWebPreferences().nodeIntegration) { + if (embedder.getWebPreferences().nodeIntegration === false) { options.webPreferences.nodeIntegration = false } } else { From 4890734f66fc358c004a719d39ac247ae2327b33 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 24 Mar 2016 14:12:46 -0700 Subject: [PATCH 08/17] Add missing title param --- spec/chromium-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index aa4e5c5fdaf5..1d37d0fe051d 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -199,7 +199,7 @@ describe('chromium feature', function () { }, slashes: true }) - b = window.open(windowUrl, 'nodeIntegration=no,show=no') + b = window.open(windowUrl, '', 'nodeIntegration=no,show=no') }) it('does not override child options', function (done) { From fd12e1f50653df17eb6fd6d5918c2bca1138eb1f Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 30 Mar 2016 10:33:15 -0700 Subject: [PATCH 09/17] Add failing spec for webview nodeIntegration inheritance --- .../webview-opener-no-node-integration.html | 9 ++++++ spec/webview-spec.js | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 spec/fixtures/pages/webview-opener-no-node-integration.html diff --git a/spec/fixtures/pages/webview-opener-no-node-integration.html b/spec/fixtures/pages/webview-opener-no-node-integration.html new file mode 100644 index 000000000000..cd95f9bb9fb7 --- /dev/null +++ b/spec/fixtures/pages/webview-opener-no-node-integration.html @@ -0,0 +1,9 @@ + + + + + diff --git a/spec/webview-spec.js b/spec/webview-spec.js index e51162935634..3d45d51e6ac2 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -3,6 +3,9 @@ const path = require('path') const http = require('http') const url = require('url') +const {remote} = require('electron') +const {BrowserWindow} = remote + describe(' tag', function () { this.timeout(10000) @@ -74,6 +77,32 @@ describe(' tag', function () { document.body.appendChild(webview) }) + + it('disables node integration on child windows when it is disabled on the webview', function (done) { + webview.addEventListener('console-message', function (e) { + assert.equal(e.message, 'window opened') + const sourceId = remote.getCurrentWindow().id + const windows = BrowserWindow.getAllWindows().filter(function (window) { + return window.id !== sourceId + }) + assert.equal(windows.length, 1) + assert.equal(windows[0].webContents.getWebPreferences().nodeIntegration, false) + done() + }) + + webview.setAttribute('allowpopups', 'on') + + webview.src = require('url').format({ + pathname: `${fixtures}/pages/webview-opener-no-node-integration.html`, + protocol: 'file', + query: { + p: `${fixtures}/pages/window-opener-node.html` + }, + slashes: true + }) + document.body.appendChild(webview) + }) + if (process.platform !== 'win32' || process.execPath.toLowerCase().indexOf('\\out\\d\\') === -1) { it('loads native modules when navigation happens', function (done) { var listener = function () { From eafe9c245b6b8c74fb1e49b0a458f546696176d3 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 30 Mar 2016 10:51:56 -0700 Subject: [PATCH 10/17] Disable guest node integration when embedder has it disabled --- lib/browser/guest-window-manager.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 5782670ac639..7508b6f16a2e 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -28,11 +28,6 @@ var mergeBrowserWindowOptions = function (embedder, options) { if (embedder.browserWindowOptions != null) { // Inherit the original options if it is a BrowserWindow. mergeOptions(options, embedder.browserWindowOptions) - - // Disable node integration on child window if disabled on parent window - if (embedder.getWebPreferences().nodeIntegration === false) { - options.webPreferences.nodeIntegration = false - } } else { // Or only inherit web-preferences if it is a webview. if (options.webPreferences == null) { @@ -40,6 +35,12 @@ var mergeBrowserWindowOptions = function (embedder, options) { } mergeOptions(options.webPreferences, embedder.getWebPreferences()) } + + // Disable node integration on child window if disabled on parent window + if (embedder.getWebPreferences().nodeIntegration === false) { + options.webPreferences.nodeIntegration = false + } + return options } From 55b12c184b1c3c4b2d5d32a8944d7234d88e709a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 30 Mar 2016 10:55:16 -0700 Subject: [PATCH 11/17] Doc node integration inheritance --- docs/api/web-view-tag.md | 3 +++ docs/api/window-open.md | 3 +++ 2 files changed, 6 insertions(+) diff --git a/docs/api/web-view-tag.md b/docs/api/web-view-tag.md index ff9013023c15..47e23dc9943f 100644 --- a/docs/api/web-view-tag.md +++ b/docs/api/web-view-tag.md @@ -84,6 +84,9 @@ than the minimum values or greater than the maximum. If "on", the guest page in `webview` will have node integration and can use node APIs like `require` and `process` to access low level system resources. +**Note:** Node integration will always be disabled in the `webview` if it is +disabled on the parent window. + ### `plugins` ```html diff --git a/docs/api/window-open.md b/docs/api/window-open.md index 5d298e61e75b..46e74327741f 100644 --- a/docs/api/window-open.md +++ b/docs/api/window-open.md @@ -23,6 +23,9 @@ Creates a new window and returns an instance of `BrowserWindowProxy` class. The `features` string follows the format of standard browser, but each feature has to be a field of `BrowserWindow`'s options. +**Note:** Node integration will always be disabled in the opened `window` if it +is disabled on the parent window. + ### `window.opener.postMessage(message, targetOrigin)` * `message` String From ef2a28ca8643a88546f42fb39d4851f7a421f90d Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 30 Mar 2016 11:17:24 -0700 Subject: [PATCH 12/17] Listen for browser-window-created event for asserts --- spec/webview-spec.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 3d45d51e6ac2..122c22ad2895 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -4,7 +4,7 @@ const http = require('http') const url = require('url') const {remote} = require('electron') -const {BrowserWindow} = remote +const {app} = remote describe(' tag', function () { this.timeout(10000) @@ -79,14 +79,8 @@ describe(' tag', function () { it('disables node integration on child windows when it is disabled on the webview', function (done) { - webview.addEventListener('console-message', function (e) { - assert.equal(e.message, 'window opened') - const sourceId = remote.getCurrentWindow().id - const windows = BrowserWindow.getAllWindows().filter(function (window) { - return window.id !== sourceId - }) - assert.equal(windows.length, 1) - assert.equal(windows[0].webContents.getWebPreferences().nodeIntegration, false) + app.once('browser-window-created', function (event, window) { + assert.equal(window.webContents.getWebPreferences().nodeIntegration, false) done() }) From 230ed78dd677204d03c5b278690ab23825f2e1b9 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 30 Mar 2016 11:56:31 -0700 Subject: [PATCH 13/17] Remove lint warnings --- spec/webview-spec.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 122c22ad2895..e47c71828831 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -2,9 +2,7 @@ const assert = require('assert') const path = require('path') const http = require('http') const url = require('url') - -const {remote} = require('electron') -const {app} = remote +const {app, session} = require('electron') describe(' tag', function () { this.timeout(10000) @@ -704,7 +702,6 @@ describe(' tag', function () { describe('permission-request event', function () { function setUpRequestHandler (webview, requested_permission) { - const session = require('electron').remote.session var listener = function (webContents, permission, callback) { if (webContents.getId() === webview.getId()) { assert.equal(permission, requested_permission) From 909ed54befd25e8b070d283467c7a90c3d4c121d Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 1 Apr 2016 16:52:28 -0700 Subject: [PATCH 14/17] Remove stray log --- spec/fixtures/pages/webview-opener-no-node-integration.html | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/fixtures/pages/webview-opener-no-node-integration.html b/spec/fixtures/pages/webview-opener-no-node-integration.html index cd95f9bb9fb7..1275a99b0bfd 100644 --- a/spec/fixtures/pages/webview-opener-no-node-integration.html +++ b/spec/fixtures/pages/webview-opener-no-node-integration.html @@ -3,7 +3,6 @@ From 7734f6af621a56993f0698b3165316552c83dd16 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 1 Apr 2016 16:52:55 -0700 Subject: [PATCH 15/17] Remove semicolons --- spec/fixtures/pages/window-opener-no-node-integration.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/pages/window-opener-no-node-integration.html b/spec/fixtures/pages/window-opener-no-node-integration.html index e146a2804ae5..b587e9792270 100644 --- a/spec/fixtures/pages/window-opener-no-node-integration.html +++ b/spec/fixtures/pages/window-opener-no-node-integration.html @@ -5,11 +5,11 @@ var opened = window.open('file://' + windowUrl, '', 'nodeIntegration=yes,show=no') window.addEventListener('message', function (event) { try { - opened.close(); + opened.close() } finally { - window.opener.postMessage(event.data, '*'); + window.opener.postMessage(event.data, '*') } - }); + }) From 3695e3871930a2be17e106511b2beedf48f6f5a9 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 1 Apr 2016 16:53:44 -0700 Subject: [PATCH 16/17] Remove duplicate require --- spec/webview-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index e47c71828831..583d98131d47 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -84,7 +84,7 @@ describe(' tag', function () { webview.setAttribute('allowpopups', 'on') - webview.src = require('url').format({ + webview.src = url.format({ pathname: `${fixtures}/pages/webview-opener-no-node-integration.html`, protocol: 'file', query: { From 527ff66115dcbab596067593fe436457d1cd62c2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 1 Apr 2016 16:59:13 -0700 Subject: [PATCH 17/17] Pull app/session from remote --- spec/webview-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 583d98131d47..0d32138906e3 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -2,7 +2,7 @@ const assert = require('assert') const path = require('path') const http = require('http') const url = require('url') -const {app, session} = require('electron') +const {app, session} = require('electron').remote describe(' tag', function () { this.timeout(10000)