From 0513574237583f8d3bc83feaa65ebdf59b4c3fee Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 4 Nov 2019 11:16:51 -0800 Subject: [PATCH] fix: capture the promise global to avoid userland mutation (#20925) --- build/webpack/webpack.config.base.js | 7 +++++-- filenames.auto.gni | 6 ++++++ lib/common/asar.js | 2 ++ lib/common/webpack-globals-provider.ts | 8 ++++++++ spec/api-remote-spec.js | 18 ++++++++++++++++++ spec/static/main.js | 1 + 6 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 lib/common/webpack-globals-provider.ts diff --git a/build/webpack/webpack.config.base.js b/build/webpack/webpack.config.base.js index 0adf1f7ec681a..47385fa311bed 100644 --- a/build/webpack/webpack.config.base.js +++ b/build/webpack/webpack.config.base.js @@ -74,7 +74,10 @@ module.exports = ({ global: ['@electron/internal/renderer/webpack-provider', '_global'], Buffer: ['@electron/internal/renderer/webpack-provider', 'Buffer'], }) - ] : []) + ] : []), + new webpack.ProvidePlugin({ + Promise: ['@electron/internal/common/webpack-globals-provider', 'Promise'], + }), ] }) -} \ No newline at end of file +} diff --git a/filenames.auto.gni b/filenames.auto.gni index ecbf266473604..3eb7b8d34431e 100644 --- a/filenames.auto.gni +++ b/filenames.auto.gni @@ -140,6 +140,7 @@ auto_filenames = { "lib/common/error-utils.ts", "lib/common/is-promise.ts", "lib/common/web-view-methods.ts", + "lib/common/webpack-globals-provider.ts", "lib/renderer/api/crash-reporter.js", "lib/renderer/api/desktop-capturer.ts", "lib/renderer/api/ipc-renderer.js", @@ -174,6 +175,7 @@ auto_filenames = { isolated_bundle_deps = [ "lib/common/electron-binding-setup.ts", "lib/common/error-utils.ts", + "lib/common/webpack-globals-provider.ts", "lib/isolated_renderer/init.js", "lib/renderer/ipc-renderer-internal-utils.ts", "lib/renderer/ipc-renderer-internal.ts", @@ -188,6 +190,7 @@ auto_filenames = { content_script_bundle_deps = [ "lib/common/electron-binding-setup.ts", "lib/common/error-utils.ts", + "lib/common/webpack-globals-provider.ts", "lib/content_script/init.js", "lib/renderer/chrome-api.ts", "lib/renderer/extensions/event.ts", @@ -275,6 +278,7 @@ auto_filenames = { "lib/common/parse-features-string.js", "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.ts", + "lib/common/webpack-globals-provider.ts", "lib/renderer/ipc-renderer-internal-utils.ts", "lib/renderer/ipc-renderer-internal.ts", "package.json", @@ -299,6 +303,7 @@ auto_filenames = { "lib/common/is-promise.ts", "lib/common/reset-search-paths.ts", "lib/common/web-view-methods.ts", + "lib/common/webpack-globals-provider.ts", "lib/renderer/api/crash-reporter.js", "lib/renderer/api/desktop-capturer.ts", "lib/renderer/api/exports/electron.js", @@ -348,6 +353,7 @@ auto_filenames = { "lib/common/init.ts", "lib/common/is-promise.ts", "lib/common/reset-search-paths.ts", + "lib/common/webpack-globals-provider.ts", "lib/renderer/api/crash-reporter.js", "lib/renderer/api/desktop-capturer.ts", "lib/renderer/api/exports/electron.js", diff --git a/lib/common/asar.js b/lib/common/asar.js index 67b7b813f8515..f9313a9158180 100644 --- a/lib/common/asar.js +++ b/lib/common/asar.js @@ -8,6 +8,8 @@ const path = require('path') const util = require('util') + const Promise = global.Promise + const envNoAsar = process.env.ELECTRON_NO_ASAR && process.type !== 'browser' && process.type !== 'renderer' diff --git a/lib/common/webpack-globals-provider.ts b/lib/common/webpack-globals-provider.ts new file mode 100644 index 0000000000000..f7d64270776ae --- /dev/null +++ b/lib/common/webpack-globals-provider.ts @@ -0,0 +1,8 @@ +// Captures original globals into a scope to ensure that userland modifications do +// not impact Electron. Note that users doing: +// +// global.Promise.resolve = myFn +// +// Will mutate this captured one as well and that is OK. + +export const Promise = global.Promise diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index 4564560d74ea8..9dbd0a2c54677 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -508,4 +508,22 @@ describe('remote module', () => { w.loadURL('about:blank') }) }) + + describe('with an overriden global Promise constrctor', () => { + let original + + before(() => { + original = Promise + }) + + it('using a promise based method resolves correctly', async () => { + expect(await remote.getGlobal('returnAPromise')(123)).to.equal(123) + global.Promise = { resolve: () => ({}) } + expect(await remote.getGlobal('returnAPromise')(456)).to.equal(456) + }) + + after(() => { + global.Promise = original + }) + }) }) diff --git a/spec/static/main.js b/spec/static/main.js index b463a39440bfa..d32e398ba16d6 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -72,6 +72,7 @@ ipcMain.on('echo', function (event, msg) { }) global.setTimeoutPromisified = util.promisify(setTimeout) +global.returnAPromise = (value) => new Promise((resolve) => setTimeout(() => resolve(value), 100)) global.permissionChecks = { allow: () => electron.session.defaultSession.setPermissionCheckHandler(null),