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 <samuel.r.attard@gmail.com>

* 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

@@ -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,

atom/browser/api/event_emitter.cc

@@ -56,9 +56,10 @@ v8::Local<v8::Object> 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;
 }
 

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

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";

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[];

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);

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);

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -90,7 +90,8 @@ base::FilePath::StringType GetExecPath() {
 }
 
 void InitializeBindings(v8::Local<v8::Object> binding,
-                        v8::Local<v8::Context> context) {
+                        v8::Local<v8::Context> 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<v8::Object> 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<v8::Function>::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<v8::Value> args[] = {binding};
   // Execute the function with proper arguments
@@ -197,7 +207,10 @@ void AtomSandboxedRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> 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();

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

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.

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

docs/api/web-contents.md

@@ -1253,6 +1253,36 @@ app.on('ready', () => {
 </html>
 ```
 
+#### `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

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)
   })
 

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++
 })