From 0890b0ca8e85f154cacefdc2e18229cd7491f76f Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 22 Jan 2019 11:24:46 -0800 Subject: [PATCH] feat: preloads and nodeIntegration in iframes (#16425) * feat: add support for node / preloads in subframes This feature has delibrately been built / implemented in such a way that it has minimum impact on existing apps / code-paths. Without enabling the new "nodeSupportInSubFrames" option basically none of this new code will be hit. The things that I believe need extra scrutiny are: * Introduction of `event.reply` for IPC events and usage of `event.reply` instead of `event.sender.send()` * Usage of `node::FreeEnvironment(env)` when the new option is enabled in order to avoid memory leaks. I have tested this quite a bit and haven't managed to cause a crash but it is still feature flagged behind the "nodeSupportInSubFrames" flag to avoid potential impact. Closes #10569 Closes #10401 Closes #11868 Closes #12505 Closes #14035 * feat: add support preloads in subframes for sandboxed renderers * spec: add tests for new nodeSupportInSubFrames option * spec: fix specs for .reply and ._replyInternal for internal messages * chore: revert change to use flag instead of environment set size * chore: clean up subframe impl * chore: apply suggestions from code review Co-Authored-By: MarshallOfSound * chore: clean up reply usage * chore: fix TS docs generation * chore: cleanup after rebase * chore: rename wrap to add in event fns --- atom/browser/api/atom_api_web_contents.cc | 4 +- atom/browser/api/event_emitter.cc | 5 +- atom/browser/web_contents_preferences.cc | 4 + atom/common/options_switches.cc | 6 ++ atom/common/options_switches.h | 2 + atom/renderer/atom_render_frame_observer.cc | 2 +- atom/renderer/atom_renderer_client.cc | 44 ++++++---- .../atom_sandboxed_renderer_client.cc | 21 ++++- docs/api/browser-window.md | 4 + docs/api/ipc-main.md | 17 +++- docs/api/process.md | 5 ++ docs/api/web-contents.md | 30 +++++++ lib/browser/api/web-contents.js | 20 +++++ lib/browser/chrome-extension.js | 4 +- lib/browser/desktop-capturer.js | 15 ++-- lib/browser/guest-view-manager.js | 5 +- lib/browser/guest-window-manager.js | 3 +- lib/browser/rpc-server.js | 2 +- lib/renderer/init.js | 6 +- lib/renderer/security-warnings.js | 1 + lib/renderer/window-setup.js | 2 +- lib/sandboxed_renderer/init.js | 1 + spec/api-app-spec.js | 2 +- spec/api-browser-window-spec.js | 4 +- spec/api-net-spec.js | 10 +-- spec/api-subframe-spec.js | 88 +++++++++++++++++++ spec/api-web-contents-spec.js | 2 +- spec/events-helpers.js | 15 +++- spec/fixtures/sub-frames/frame-container.html | 13 +++ .../frame-with-frame-container.html | 13 +++ .../fixtures/sub-frames/frame-with-frame.html | 13 +++ spec/fixtures/sub-frames/frame.html | 12 +++ spec/fixtures/sub-frames/preload.js | 7 ++ spec/static/main.js | 6 +- 34 files changed, 330 insertions(+), 58 deletions(-) create mode 100644 spec/api-subframe-spec.js create mode 100644 spec/fixtures/sub-frames/frame-container.html create mode 100644 spec/fixtures/sub-frames/frame-with-frame-container.html create mode 100644 spec/fixtures/sub-frames/frame-with-frame.html create mode 100644 spec/fixtures/sub-frames/frame.html create mode 100644 spec/fixtures/sub-frames/preload.js diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index fb48ac7617404..a8f0f91c09381 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1593,7 +1593,7 @@ bool WebContents::SendIPCMessageToFrame(bool internal, if (!(*iter)->IsRenderFrameLive()) return false; return (*iter)->Send(new AtomFrameMsg_Message( - frame_id, internal, send_to_all, channel, args, 0 /* sender_id */)); + frame_id, send_to_all, base::UTF8ToUTF16(channel), args)); } void WebContents::SendInputEvent(v8::Isolate* isolate, @@ -2103,7 +2103,7 @@ void WebContents::OnRendererMessage(content::RenderFrameHost* frame_host, const base::string16& channel, const base::ListValue& args) { // webContents.emit(channel, new Event(), args...); - EmitWithSender(channel, frame_host, nullptr, args); + EmitWithSender(base::UTF16ToUTF8(channel), frame_host, nullptr, args); } void WebContents::OnRendererMessageSync(content::RenderFrameHost* frame_host, diff --git a/atom/browser/api/event_emitter.cc b/atom/browser/api/event_emitter.cc index 4b4c0caa72614..a924225fccd5b 100644 --- a/atom/browser/api/event_emitter.cc +++ b/atom/browser/api/event_emitter.cc @@ -56,9 +56,10 @@ v8::Local CreateJSEvent(v8::Isolate* isolate, } else { event = CreateEventObject(isolate); } - mate::Dictionary(isolate, event).Set("sender", object); + mate::Dictionary dict(isolate, event); + dict.Set("sender", object); if (sender) - mate::Dictionary(isolate, event).Set("frameId", sender->GetRoutingID()); + dict.Set("frameId", sender->GetRoutingID()); return event; } diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 4a8fb10f95a41..63274721bc4a4 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -55,6 +55,7 @@ WebContentsPreferences::WebContentsPreferences( SetDefaultBoolIfUndefined(options::kExperimentalFeatures, false); SetDefaultBoolIfUndefined(options::kExperimentalCanvasFeatures, false); bool node = SetDefaultBoolIfUndefined(options::kNodeIntegration, true); + SetDefaultBoolIfUndefined(options::kNodeIntegrationInSubFrames, false); SetDefaultBoolIfUndefined(options::kNodeIntegrationInWorker, false); SetDefaultBoolIfUndefined(options::kWebviewTag, node); SetDefaultBoolIfUndefined(options::kSandbox, false); @@ -278,6 +279,9 @@ void WebContentsPreferences::AppendCommandLineSwitches( } } + if (IsEnabled(options::kNodeIntegrationInSubFrames)) + command_line->AppendSwitch(switches::kNodeIntegrationInSubFrames); + // We are appending args to a webContents so let's save the current state // of our preferences object so that during the lifetime of the WebContents // we can fetch the options used to initally configure the WebContents diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index 7de8d1747babd..23fc5d76f250a 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -152,6 +152,8 @@ const char kAllowRunningInsecureContent[] = "allowRunningInsecureContent"; const char kOffscreen[] = "offscreen"; +const char kNodeIntegrationInSubFrames[] = "nodeIntegrationInSubFrames"; + } // namespace options namespace switches { @@ -206,6 +208,10 @@ const char kWebviewTag[] = "webview-tag"; // Command switch passed to renderer process to control nodeIntegration. const char kNodeIntegrationInWorker[] = "node-integration-in-worker"; +// Command switch passed to renderer process to control whether node +// environments will be created in sub-frames. +const char kNodeIntegrationInSubFrames[] = "node-integration-in-subframes"; + // Widevine options // Path to Widevine CDM binaries. const char kWidevineCdmPath[] = "widevine-cdm-path"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index a6b1e404fa032..d6ef5d87a5d89 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -75,6 +75,7 @@ extern const char kSandbox[]; extern const char kWebSecurity[]; extern const char kAllowRunningInsecureContent[]; extern const char kOffscreen[]; +extern const char kNodeIntegrationInSubFrames[]; } // namespace options @@ -107,6 +108,7 @@ extern const char kHiddenPage[]; extern const char kNativeWindowOpen[]; extern const char kNodeIntegrationInWorker[]; extern const char kWebviewTag[]; +extern const char kNodeIntegrationInSubFrames[]; extern const char kWidevineCdmPath[]; extern const char kWidevineCdmVersion[]; diff --git a/atom/renderer/atom_render_frame_observer.cc b/atom/renderer/atom_render_frame_observer.cc index ea2355fd96833..19356ce6b1597 100644 --- a/atom/renderer/atom_render_frame_observer.cc +++ b/atom/renderer/atom_render_frame_observer.cc @@ -179,7 +179,7 @@ void AtomRenderFrameObserver::OnBrowserMessage(bool send_to_all, return; blink::WebLocalFrame* frame = render_frame_->GetWebFrame(); - if (!frame || !render_frame_->IsMainFrame()) + if (!frame) return; EmitIPCEvent(frame, channel, args); diff --git a/atom/renderer/atom_renderer_client.cc b/atom/renderer/atom_renderer_client.cc index b4521998d4656..1e0aeddef764a 100644 --- a/atom/renderer/atom_renderer_client.cc +++ b/atom/renderer/atom_renderer_client.cc @@ -82,23 +82,27 @@ void AtomRendererClient::DidCreateScriptContext( content::RenderFrame* render_frame) { RendererClientBase::DidCreateScriptContext(context, render_frame); - // Only allow node integration for the main frame, unless it is a devtools - // extension page. - if (!render_frame->IsMainFrame() && !IsDevToolsExtension(render_frame)) - return; - - // Don't allow node integration if this is a child window and it does not have - // node integration enabled. Otherwise we would have memory leak in the child - // window since we don't clean up node environments. - // - // TODO(zcbenz): We shouldn't allow node integration even for the top frame. - if (!render_frame->GetWebkitPreferences().node_integration && - render_frame->GetWebFrame()->Opener()) + // TODO(zcbenz): Do not create Node environment if node integration is not + // enabled. + + // Do not load node if we're aren't a main frame or a devtools extension + // unless node support has been explicitly enabled for sub frames + bool is_main_frame = + render_frame->IsMainFrame() && !render_frame->GetWebFrame()->Opener(); + bool is_devtools = IsDevToolsExtension(render_frame); + bool allow_node_in_subframes = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + bool should_load_node = + is_main_frame || is_devtools || allow_node_in_subframes; + if (!should_load_node) { return; + } injected_frames_.insert(render_frame); - // Prepare the node bindings. + // If this is the first environment we are creating, prepare the node + // bindings. if (!node_integration_initialized_) { node_integration_initialized_ = true; node_bindings_->Initialize(); @@ -117,6 +121,8 @@ void AtomRendererClient::DidCreateScriptContext( // Add Electron extended APIs. atom_bindings_->BindTo(env->isolate(), env->process_object()); AddRenderBindings(env->isolate(), env->process_object()); + mate::Dictionary process_dict(env->isolate(), env->process_object()); + process_dict.SetReadOnly("isMainFrame", render_frame->IsMainFrame()); // Load everything. node_bindings_->LoadEnvironment(env); @@ -148,11 +154,13 @@ void AtomRendererClient::WillReleaseScriptContext( if (env == node_bindings_->uv_env()) node_bindings_->set_uv_env(nullptr); - // Destroy the node environment. - // This is disabled because pending async tasks may still use the environment - // and would cause crashes later. Node does not seem to clear all async tasks - // when the environment is destroyed. - // node::FreeEnvironment(env); + // Destroy the node environment. We only do this if node support has been + // enabled for sub-frames to avoid a change-of-behavior / introduce crashes + // for existing users. + // TODO(MarshallOfSOund): Free the environment regardless of this switch + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames)) + node::FreeEnvironment(env); // AtomBindings is tracking node environments. atom_bindings_->EnvironmentDestroyed(env); diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index ce214162033ca..ba8856254a7f3 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.cc +++ b/atom/renderer/atom_sandboxed_renderer_client.cc @@ -90,7 +90,8 @@ base::FilePath::StringType GetExecPath() { } void InitializeBindings(v8::Local binding, - v8::Local context) { + v8::Local context, + bool is_main_frame) { auto* isolate = context->GetIsolate(); mate::Dictionary b(isolate, binding); b.SetMethod("get", GetBinding); @@ -101,6 +102,7 @@ void InitializeBindings(v8::Local binding, b.SetMethod("getHeapStatistics", &AtomBindings::GetHeapStatistics); b.SetMethod("getProcessMemoryInfo", &AtomBindings::GetProcessMemoryInfo); b.SetMethod("getSystemMemoryInfo", &AtomBindings::GetSystemMemoryInfo); + b.SetReadOnly("isMainFrame", is_main_frame); // Pass in CLI flags needed to setup the renderer base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); @@ -166,7 +168,15 @@ void AtomSandboxedRendererClient::DidCreateScriptContext( // Only allow preload for the main frame or // For devtools we still want to run the preload_bundle script - if (!render_frame->IsMainFrame() && !IsDevTools(render_frame)) + // Or when nodeSupport is explicitly enabled in sub frames + bool is_main_frame = render_frame->IsMainFrame(); + bool is_devtools = IsDevTools(render_frame); + bool allow_node_in_sub_frames = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + bool should_load_preload = + is_main_frame || is_devtools || allow_node_in_sub_frames; + if (!should_load_preload) return; auto* isolate = context->GetIsolate(); @@ -185,7 +195,7 @@ void AtomSandboxedRendererClient::DidCreateScriptContext( v8::Handle::Cast(script->Run(context).ToLocalChecked()); // Create and initialize the binding object auto binding = v8::Object::New(isolate); - InitializeBindings(binding, context); + InitializeBindings(binding, context, render_frame->IsMainFrame()); AddRenderBindings(isolate, binding); v8::Local args[] = {binding}; // Execute the function with proper arguments @@ -197,7 +207,10 @@ void AtomSandboxedRendererClient::WillReleaseScriptContext( v8::Handle context, content::RenderFrame* render_frame) { // Only allow preload for the main frame - if (!render_frame->IsMainFrame()) + // Or for sub frames when explicitly enabled + if (!render_frame->IsMainFrame() && + !base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames)) return; auto* isolate = context->GetIsolate(); diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index d7897d4201e88..aa1aaa8d7c8d3 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -256,6 +256,10 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. * `nodeIntegrationInWorker` Boolean (optional) - Whether node integration is enabled in web workers. Default is `false`. More about this can be found in [Multithreading](../tutorial/multithreading.md). + * `nodeIntegrationInSubFrames` Boolean (optional) - Experimental option for + enabling NodeJS support in sub-frames such as iframes. All your preloads will load for + every iframe, you can use `process.isMainFrame` to determine if you are + in the main frame or not. * `preload` String (optional) - Specifies a script that will be loaded before other scripts run in the page. This script will always have access to node APIs no matter whether node integration is turned on or off. The value should diff --git a/docs/api/ipc-main.md b/docs/api/ipc-main.md index 1dc51f618cc7b..098e19104535f 100644 --- a/docs/api/ipc-main.md +++ b/docs/api/ipc-main.md @@ -18,7 +18,9 @@ process, see [webContents.send][web-contents-send] for more information. * When sending a message, the event name is the `channel`. * To reply to a synchronous message, you need to set `event.returnValue`. * To send an asynchronous message back to the sender, you can use - `event.sender.send(...)`. + `event.reply(...)`. This helper method will automatically handle messages + coming from frames that aren't the main frame (e.g. iframes) whereas + `event.sender.send(...)` will always send to the main frame. An example of sending and handling messages between the render and main processes: @@ -28,7 +30,7 @@ processes: const {ipcMain} = require('electron') ipcMain.on('asynchronous-message', (event, arg) => { console.log(arg) // prints "ping" - event.sender.send('asynchronous-reply', 'pong') + event.reply('asynchronous-reply', 'pong') }) ipcMain.on('synchronous-message', (event, arg) => { @@ -86,6 +88,10 @@ Removes listeners of the specified `channel`. The `event` object passed to the `callback` has the following methods: +### `event.frameId` + +An `Integer` representing the ID of the renderer frame that sent this message. + ### `event.returnValue` Set this to the value to be returned in a synchronous message. @@ -97,3 +103,10 @@ Returns the `webContents` that sent the message, you can call [webContents.send][web-contents-send] for more information. [web-contents-send]: web-contents.md#contentssendchannel-arg1-arg2- + +### `event.reply` + +A function that will send an IPC message to the renderer frane that sent +the original message that you are currently handling. You should use this +method to "reply" to the sent message in order to guaruntee the reply will go +to the correct process and frame. diff --git a/docs/api/process.md b/docs/api/process.md index e6b34290b92c0..7f554d6c9fb0a 100644 --- a/docs/api/process.md +++ b/docs/api/process.md @@ -48,6 +48,11 @@ process.once('loaded', () => { A `Boolean`. When app is started by being passed as parameter to the default app, this property is `true` in the main process, otherwise it is `undefined`. +### `process.isMainFrame` + +A `Boolean`, `true` when the current renderer context is the "main" renderer +frame. If you want the ID of the current frame you should use `webFrame.routingId`. + ### `process.mas` A `Boolean`. For Mac App Store build, this property is `true`, for other builds it is diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index e506a2b183467..8466a532876d7 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -1253,6 +1253,36 @@ app.on('ready', () => { ``` +#### `contents.sendToFrame(frameId, channel[, arg1][, arg2][, ...])` + +* `frameId` Integer +* `channel` String +* `...args` any[] + +Send an asynchronous message to a specific frame in a renderer process via +`channel`. Arguments will be serialized +as JSON internally and as such no functions or prototype chains will be included. + +The renderer process can handle the message by listening to `channel` with the +[`ipcRenderer`](ipc-renderer.md) module. + +If you want to get the `frameId` of a given renderer context you should use +the `webFrame.routingId` value. E.g. + +```js +// In a renderer process +console.log('My frameId is:', require('electron').webFrame.routingId) +``` + +You can also read `frameId` from all incoming IPC messages in the main process. + +```js +// In the main process +ipcMain.on('ping', (event) => { + console.info('Message came from frameId:', event.frameId) +}) +``` + #### `contents.enableDeviceEmulation(parameters)` * `parameters` Object diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 00aac8cee9f56..dff7b149461d8 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -103,6 +103,18 @@ WebContents.prototype.sendToAll = function (channel, ...args) { if (channel == null) throw new Error('Missing required channel argument') return this._send(true, channel, args) } +WebContents.prototype.sendToFrame = function (frameId, channel, ...args) { + if (typeof channel !== 'string') { + throw new Error('Missing required channel argument') + } else if (typeof frameId !== 'number') { + throw new Error('Missing required frameId argument') + } + + const internal = false + const sendToAll = false + + return this._sendToFrame(internal, sendToAll, frameId, channel, args) +} WebContents.prototype._sendToFrameInternal = function (frameId, channel, ...args) { if (typeof channel !== 'string') { throw new Error('Missing required channel argument') @@ -282,6 +294,12 @@ WebContents.prototype.setSize = function () { 'https://github.com/electron/electron/issues/14120 for more.') } +const addReplyToEvent = (event) => { + event.reply = (...args) => { + event.sender.sendToFrame(event.frameId, ...args) + } +} + // Add JavaScript wrappers for WebContents class. WebContents.prototype._init = function () { // The navigation controller. @@ -293,6 +311,7 @@ WebContents.prototype._init = function () { // Dispatch IPC messages to the ipc module. this.on('ipc-message', function (event, [channel, ...args]) { + addReplyToEvent(event) ipcMain.emit(channel, event, ...args) }) this.on('ipc-message-sync', function (event, [channel, ...args]) { @@ -302,6 +321,7 @@ WebContents.prototype._init = function () { }, get: function () {} }) + addReplyToEvent(event) ipcMain.emit(channel, event, ...args) }) diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index 8bdac799f7086..4623af24e927e 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -177,7 +177,7 @@ ipcMain.on('CHROME_RUNTIME_SENDMESSAGE', function (event, extensionId, message, page.webContents.sendToAll(`CHROME_RUNTIME_ONMESSAGE_${extensionId}`, event.sender.id, message, resultID) ipcMain.once(`CHROME_RUNTIME_ONMESSAGE_RESULT_${resultID}`, (event, result) => { - event.sender.send(`CHROME_RUNTIME_SENDMESSAGE_RESULT_${originResultID}`, result) + event.reply(`CHROME_RUNTIME_SENDMESSAGE_RESULT_${originResultID}`, result) }) resultID++ }) @@ -193,7 +193,7 @@ ipcMain.on('CHROME_TABS_SEND_MESSAGE', function (event, tabId, extensionId, isBa contents.sendToAll(`CHROME_RUNTIME_ONMESSAGE_${extensionId}`, senderTabId, message, resultID) ipcMain.once(`CHROME_RUNTIME_ONMESSAGE_RESULT_${resultID}`, (event, result) => { - event.sender.send(`CHROME_TABS_SEND_MESSAGE_RESULT_${originResultID}`, result) + event.reply(`CHROME_TABS_SEND_MESSAGE_RESULT_${originResultID}`, result) }) resultID++ }) diff --git a/lib/browser/desktop-capturer.js b/lib/browser/desktop-capturer.js index c7fe1abf67c0a..772e6d3e20a29 100644 --- a/lib/browser/desktop-capturer.js +++ b/lib/browser/desktop-capturer.js @@ -19,7 +19,7 @@ ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize, captureScreen, thumbnailSize }, - webContents: event.sender + event } requestsQueue.push(request) if (requestsQueue.length === 1) { @@ -29,14 +29,13 @@ ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize, // If the WebContents is destroyed before receiving result, just remove the // reference from requestsQueue to make the module not send the result to it. event.sender.once('destroyed', () => { - request.webContents = null + request.event = null }) }) desktopCapturer.emit = (event, name, sources) => { // Receiving sources result from main process, now send them back to renderer. const handledRequest = requestsQueue.shift() - const handledWebContents = handledRequest.webContents const unhandledRequestsQueue = [] const result = sources.map(source => { @@ -48,15 +47,17 @@ desktopCapturer.emit = (event, name, sources) => { } }) - if (handledWebContents) { - handledWebContents.send(capturerResult(handledRequest.id), result) + if (handledRequest.event) { + handledRequest.event.reply(capturerResult(handledRequest.id), result) } // Check the queue to see whether there is another identical request & handle requestsQueue.forEach(request => { - const webContents = request.webContents + const event = request.event if (deepEqual(handledRequest.options, request.options)) { - if (webContents) webContents.send(capturerResult(request.id), result) + if (event) { + event.reply(capturerResult(request.id), result) + } } else { unhandledRequestsQueue.push(request) } diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index c44d033a85f9a..324877fe88fdc 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -242,7 +242,8 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn ['javascript', false], ['nativeWindowOpen', true], ['nodeIntegration', false], - ['sandbox', true] + ['sandbox', true], + ['nodeIntegrationInSubFrames', false] ]) // Inherit certain option values from embedder @@ -325,7 +326,7 @@ const watchEmbedder = function (embedder) { } ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, params, requestId) { - event.sender.send(`ELECTRON_RESPONSE_${requestId}`, createGuest(event.sender, params)) + event.reply(`ELECTRON_RESPONSE_${requestId}`, createGuest(event.sender, params)) }) ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event, params) { diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 7985dec4b9ef1..a5aef99fa7969 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -14,7 +14,8 @@ const inheritedWebPreferences = new Map([ ['nativeWindowOpen', true], ['nodeIntegration', false], ['sandbox', true], - ['webviewTag', false] + ['webviewTag', false], + ['nodeIntegrationInSubFrames', false] ]) // Copy attribute of |parent| to |child| if it is not defined in |child|. diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index abc49a26afb6b..20f4b95dc410e 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -429,7 +429,7 @@ ipcMain.on('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', function (event, context let guest = guestViewManager.getGuest(guestInstanceId) if (requestId) { const responseCallback = function (result) { - event.sender.send(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, result) + event.reply(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, result) } args.push(responseCallback) } diff --git a/lib/renderer/init.js b/lib/renderer/init.js index 7ea87363cbe9c..7907bcae37408 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -91,10 +91,12 @@ if (window.location.protocol === 'chrome-devtools:') { require('./override') // Inject content scripts. - require('./content-scripts-injector') + if (process.isMainFrame) { + require('./content-scripts-injector') + } // Load webview tag implementation. - if (webviewTag === 'true' && process.guestInstanceId == null) { + if (webviewTag === 'true' && process.guestInstanceId == null && process.isMainFrame) { require('./web-view/web-view') require('./web-view/web-view-attributes') } diff --git a/lib/renderer/security-warnings.js b/lib/renderer/security-warnings.js index 41557164ce716..ec47d8d73c225 100644 --- a/lib/renderer/security-warnings.js +++ b/lib/renderer/security-warnings.js @@ -9,6 +9,7 @@ let shouldLog = null * @returns {boolean} - Should we log? */ const shouldLogSecurityWarnings = function () { + if (!process.isMainFrame) return false if (shouldLog !== null) { return shouldLog } diff --git a/lib/renderer/window-setup.js b/lib/renderer/window-setup.js index ac329b4307013..77005a7b1477b 100644 --- a/lib/renderer/window-setup.js +++ b/lib/renderer/window-setup.js @@ -26,7 +26,7 @@ const {defineProperty} = Object // Helper function to resolve relative url. -const a = window.top.document.createElement('a') +const a = window.document.createElement('a') const resolveURL = function (url) { a.href = url return a.href diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js index 2acb7db386363..62a2fded658b3 100644 --- a/lib/sandboxed_renderer/init.js +++ b/lib/sandboxed_renderer/init.js @@ -61,6 +61,7 @@ preloadProcess.argv = process.argv = binding.getArgv() preloadProcess.execPath = process.execPath = binding.getExecPath() preloadProcess.platform = process.platform = platform preloadProcess.env = process.env = env +preloadProcess.isMainFrame = process.isMainFrame = binding.isMainFrame process.on('exit', () => preloadProcess.emit('exit')) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index da978ceefb1f3..88e578c006cd0 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -328,7 +328,7 @@ describe('app module', () => { subject: { commonName: 'Client Cert' } }) - event.sender.send('client-certificate-response', list[0]) + event.reply('client-certificate-response', list[0]) }) app.importCertificate(options, result => { diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 3230e1d2287ca..1d0081b54e31a 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1406,12 +1406,12 @@ describe('BrowserWindow module', () => { }) ipcMain.once('parent-ready', function (event) { assert.equal(w.webContents, event.sender) - event.sender.send('verified') + event.reply('verified') }) ipcMain.once('child-ready', function (event) { assert(childWc) assert.equal(childWc, event.sender) - event.sender.send('verified') + event.reply('verified') }) waitForEvents(ipcMain, [ 'parent-answer', diff --git a/spec/api-net-spec.js b/spec/api-net-spec.js index 2beacb67c25cc..12c1724266a8f 100644 --- a/spec/api-net-spec.js +++ b/spec/api-net-spec.js @@ -1458,7 +1458,7 @@ describe('net module', () => { nodeResponse.on('data', (chunk) => { }) nodeResponse.on('end', (chunk) => { - event.sender.send('api-net-spec-done') + event.reply('api-net-spec-done') }) }) netResponse.pipe(nodeRequest) @@ -1536,7 +1536,7 @@ describe('net module', () => { process.nextTick(() => { const v8Util = process.atomBinding('v8_util') v8Util.requestGarbageCollectionForTesting() - event.sender.send('api-net-spec-done') + event.reply('api-net-spec-done') }) `) }) @@ -1570,13 +1570,13 @@ describe('net module', () => { response.on('data', () => { }) response.on('end', () => { - event.sender.send('api-net-spec-done') + event.reply('api-net-spec-done') }) process.nextTick(() => { // Trigger a garbage collection. const v8Util = process.atomBinding('v8_util') v8Util.requestGarbageCollectionForTesting() - event.sender.send('api-net-spec-resume') + event.reply('api-net-spec-resume') }) }) urlRequest.end() @@ -1612,7 +1612,7 @@ describe('net module', () => { process.nextTick(() => { const v8Util = process.atomBinding('v8_util') v8Util.requestGarbageCollectionForTesting() - event.sender.send('api-net-spec-done') + event.reply('api-net-spec-done') }) }) urlRequest.end() diff --git a/spec/api-subframe-spec.js b/spec/api-subframe-spec.js new file mode 100644 index 0000000000000..dc31a370b3da5 --- /dev/null +++ b/spec/api-subframe-spec.js @@ -0,0 +1,88 @@ +const { expect } = require('chai') +const { remote } = require('electron') +const path = require('path') + +const { emittedNTimes, emittedOnce } = require('./events-helpers') +const { closeWindow } = require('./window-helpers') + +const { BrowserWindow } = remote + +describe('renderer nodeIntegrationInSubFrames', () => { + const generateTests = (sandboxEnabled) => { + describe(`with sandbox ${sandboxEnabled ? 'enabled' : 'disabled'}`, () => { + let w + + beforeEach(async () => { + await closeWindow(w) + w = new BrowserWindow({ + show: false, + width: 400, + height: 400, + webPreferences: { + sandbox: sandboxEnabled, + preload: path.resolve(__dirname, 'fixtures/sub-frames/preload.js'), + nodeIntegrationInSubFrames: true + } + }) + }) + + afterEach(() => { + return closeWindow(w).then(() => { w = null }) + }) + + it('should load preload scripts in top level iframes', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html')) + const [event1, event2] = await detailsPromise + expect(event1[0].frameId).to.not.equal(event2[0].frameId) + expect(event1[0].frameId).to.equal(event1[2]) + expect(event2[0].frameId).to.equal(event2[2]) + }) + + it('should load preload scripts in nested iframes', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 3) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-with-frame-container.html')) + const [event1, event2, event3] = await detailsPromise + expect(event1[0].frameId).to.not.equal(event2[0].frameId) + expect(event1[0].frameId).to.not.equal(event3[0].frameId) + expect(event2[0].frameId).to.not.equal(event3[0].frameId) + expect(event1[0].frameId).to.equal(event1[2]) + expect(event2[0].frameId).to.equal(event2[2]) + expect(event3[0].frameId).to.equal(event3[2]) + }) + + it('should correctly reply to the main frame with using event.reply', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html')) + const [event1] = await detailsPromise + const pongPromise = emittedOnce(remote.ipcMain, 'preload-pong') + event1[0].reply('preload-ping') + const details = await pongPromise + expect(details[1]).to.equal(event1[0].frameId) + }) + + it('should correctly reply to the sub-frames with using event.reply', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html')) + const [, event2] = await detailsPromise + const pongPromise = emittedOnce(remote.ipcMain, 'preload-pong') + event2[0].reply('preload-ping') + const details = await pongPromise + expect(details[1]).to.equal(event2[0].frameId) + }) + + it('should correctly reply to the nested sub-frames with using event.reply', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 3) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-with-frame-container.html')) + const [,, event3] = await detailsPromise + const pongPromise = emittedOnce(remote.ipcMain, 'preload-pong') + event3[0].reply('preload-ping') + const details = await pongPromise + expect(details[1]).to.equal(event3[0].frameId) + }) + }) + } + + generateTests(false) + generateTests(true) +}) diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index dc887948b236f..f48d7e6889ad9 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -467,7 +467,7 @@ describe('webContents module', () => { ipcMain.on('set-zoom', (e, host) => { const zoomLevel = hostZoomMap[host] if (!finalNavigation) w.webContents.setZoomLevel(zoomLevel) - e.sender.send(`${host}-zoom-set`) + e.reply(`${host}-zoom-set`) }) ipcMain.on('host1-zoom-level', (e, zoomLevel) => { const expectedZoomLevel = hostZoomMap.host1 diff --git a/spec/events-helpers.js b/spec/events-helpers.js index 9bbd3da899ffc..2a226d0e3271f 100644 --- a/spec/events-helpers.js +++ b/spec/events-helpers.js @@ -20,10 +20,23 @@ const waitForEvent = (target, eventName) => { * @return {!Promise} With Event as the first item. */ const emittedOnce = (emitter, eventName) => { + return emittedNTimes(emitter, eventName, 1).then(([result]) => result) +} + +const emittedNTimes = (emitter, eventName, times) => { + const events = [] return new Promise(resolve => { - emitter.once(eventName, (...args) => resolve(args)) + const handler = (...args) => { + events.push(args) + if (events.length === times) { + emitter.removeListener(eventName, handler) + resolve(events) + } + } + emitter.on(eventName, handler) }) } exports.emittedOnce = emittedOnce +exports.emittedNTimes = emittedNTimes exports.waitForEvent = waitForEvent diff --git a/spec/fixtures/sub-frames/frame-container.html b/spec/fixtures/sub-frames/frame-container.html new file mode 100644 index 0000000000000..f731555a5ddaf --- /dev/null +++ b/spec/fixtures/sub-frames/frame-container.html @@ -0,0 +1,13 @@ + + + + + + + Document + + + This is the root page + + + \ No newline at end of file diff --git a/spec/fixtures/sub-frames/frame-with-frame-container.html b/spec/fixtures/sub-frames/frame-with-frame-container.html new file mode 100644 index 0000000000000..823fb1aafe9e7 --- /dev/null +++ b/spec/fixtures/sub-frames/frame-with-frame-container.html @@ -0,0 +1,13 @@ + + + + + + + Document + + + This is the root page + + + \ No newline at end of file diff --git a/spec/fixtures/sub-frames/frame-with-frame.html b/spec/fixtures/sub-frames/frame-with-frame.html new file mode 100644 index 0000000000000..9d99fef71b332 --- /dev/null +++ b/spec/fixtures/sub-frames/frame-with-frame.html @@ -0,0 +1,13 @@ + + + + + + + Document + + + This is a frame, is has one child + + + \ No newline at end of file diff --git a/spec/fixtures/sub-frames/frame.html b/spec/fixtures/sub-frames/frame.html new file mode 100644 index 0000000000000..4340b8d4efce5 --- /dev/null +++ b/spec/fixtures/sub-frames/frame.html @@ -0,0 +1,12 @@ + + + + + + + Document + + + This is a frame, it has no children + + \ No newline at end of file diff --git a/spec/fixtures/sub-frames/preload.js b/spec/fixtures/sub-frames/preload.js new file mode 100644 index 0000000000000..3b6c461759b25 --- /dev/null +++ b/spec/fixtures/sub-frames/preload.js @@ -0,0 +1,7 @@ +const { ipcRenderer, webFrame } = require('electron') + +ipcRenderer.send('preload-ran', window.location.href, webFrame.routingId) + +ipcRenderer.on('preload-ping', () => { + ipcRenderer.send('preload-pong', webFrame.routingId) +}) diff --git a/spec/static/main.js b/spec/static/main.js index 9abfb11c0a3a8..ab4a5191cb2a7 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -46,7 +46,7 @@ process.env.sandboxmain = '' console ipcMain.on('message', function (event, ...args) { - event.sender.send('message', ...args) + event.reply('message', ...args) }) // Set productName so getUploadedReports() uses the right directory in specs @@ -368,7 +368,7 @@ ipcMain.on('test-webcontents-navigation-observer', (event, options) => { contents.once(options.name, () => destroy()) contents.once('destroyed', () => { - event.sender.send(options.responseEvent) + event.reply(options.responseEvent) }) contents.loadURL(options.url) @@ -391,7 +391,7 @@ ipcMain.on('test-browserwindow-destroy', (event, testOptions) => { windows.forEach(win => win.focus()) windows.forEach(win => win.destroy()) app.removeListener('browser-window-focus', focusListener) - event.sender.send(testOptions.responseEvent) + event.reply(testOptions.responseEvent) }) // Suspend listeners until the next event and then restore them