From f5da0bbba85da1dfcb6367152f7c372bb53ed641 Mon Sep 17 00:00:00 2001 From: Alex Garbutt Date: Tue, 8 Jan 2019 13:26:39 -0800 Subject: [PATCH 1/2] fix: emit IPC event in correct context if isolation and sandbox enabled IPC events were not being delivered to renderer processes when both `contextIsolation` and `sandbox` were enabled. This is because the `AtomSandboxedRenderFrameObserver` class was incorrectly using the `MainWorldScriptContext`, rather than conditionally selecting the context based on if isolation was enabled. Fixes #11922 --- .../atom_sandboxed_renderer_client.cc | 4 +- spec/api-ipc-renderer-spec.js | 43 ++++++++++++++++++- spec/fixtures/module/preload-ipc-ping-pong.js | 4 ++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/module/preload-ipc-ping-pong.js diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index 7c492714ace2f..d8a2ec732324c 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.cc +++ b/atom/renderer/atom_sandboxed_renderer_client.cc @@ -110,8 +110,10 @@ class AtomSandboxedRenderFrameObserver : public AtomRenderFrameObserver { auto* isolate = blink::MainThreadIsolate(); v8::HandleScope handle_scope(isolate); - auto context = frame->MainWorldScriptContext(); + + auto context = renderer_client_->GetContext(frame, isolate); v8::Context::Scope context_scope(context); + v8::Local argv[] = {mate::ConvertToV8(isolate, channel), mate::ConvertToV8(isolate, args), mate::ConvertToV8(isolate, sender_id)}; diff --git a/spec/api-ipc-renderer-spec.js b/spec/api-ipc-renderer-spec.js index 343df56cca7f4..b538cc959cd1f 100644 --- a/spec/api-ipc-renderer-spec.js +++ b/spec/api-ipc-renderer-spec.js @@ -157,7 +157,7 @@ describe('ipc renderer module', () => { contents.loadFile(path.join(fixtures, 'pages', 'ping-pong.html')) }) - it('sends message to WebContents (sanboxed renderer)', done => { + it('sends message to WebContents (sandboxed renderer)', done => { contents = webContents.create({ preload: path.join(fixtures, 'module', 'preload-inject-ipc.js'), sandbox: true @@ -177,6 +177,47 @@ describe('ipc renderer module', () => { contents.loadFile(path.join(fixtures, 'pages', 'ping-pong.html')) }) + it('sends message to WebContents (context isolation)', done => { + contents = webContents.create({ + preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), + contextIsolation: true + }) + + const payload = 'Hello World!' + + ipcRenderer.once('pong', (event, data) => { + expect(payload).to.equal(data) + done() + }) + + contents.once('did-finish-load', () => { + ipcRenderer.sendTo(contents.id, 'ping', payload) + }) + + contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) + }) + + it('sends message to WebContents (context isolation and sandboxed renderer)', done => { + contents = webContents.create({ + preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), + contextIsolation: true, + sandbox: true + }) + + const payload = 'Hello World!' + + ipcRenderer.once('pong', (event, data) => { + expect(payload).to.equal(data) + done() + }) + + contents.once('did-finish-load', () => { + ipcRenderer.sendTo(contents.id, 'ping', payload) + }) + + contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) + }) + it('sends message to WebContents (channel has special chars)', done => { contents = webContents.create({ preload: path.join(fixtures, 'module', 'preload-inject-ipc.js') diff --git a/spec/fixtures/module/preload-ipc-ping-pong.js b/spec/fixtures/module/preload-ipc-ping-pong.js new file mode 100644 index 0000000000000..8dd8e7a99609b --- /dev/null +++ b/spec/fixtures/module/preload-ipc-ping-pong.js @@ -0,0 +1,4 @@ +const { ipcRenderer } = require('electron') +ipcRenderer.on('ping', function (event, payload) { + ipcRenderer.sendTo(event.senderId, 'pong', payload) +}) From 271ed85613de333975f282d3c776223be6e916b6 Mon Sep 17 00:00:00 2001 From: Alex Garbutt Date: Thu, 10 Jan 2019 10:59:43 -0800 Subject: [PATCH 2/2] Update tests --- spec/api-ipc-renderer-spec.js | 124 ++++++------------ spec/fixtures/module/preload-inject-ipc.js | 2 - spec/fixtures/module/preload-ipc-ping-pong.js | 5 + spec/fixtures/pages/ping-pong.html | 12 -- 4 files changed, 42 insertions(+), 101 deletions(-) delete mode 100644 spec/fixtures/module/preload-inject-ipc.js delete mode 100644 spec/fixtures/pages/ping-pong.html diff --git a/spec/api-ipc-renderer-spec.js b/spec/api-ipc-renderer-spec.js index b538cc959cd1f..475a0ac0cb265 100644 --- a/spec/api-ipc-renderer-spec.js +++ b/spec/api-ipc-renderer-spec.js @@ -138,104 +138,54 @@ describe('ipc renderer module', () => { contents = null }) - it('sends message to WebContents', done => { - contents = webContents.create({ - preload: path.join(fixtures, 'module', 'preload-inject-ipc.js') - }) - - const payload = 'Hello World!' - - ipcRenderer.once('pong', (event, data) => { - expect(payload).to.equal(data) - done() - }) - - contents.once('did-finish-load', () => { - ipcRenderer.sendTo(contents.id, 'ping', payload) - }) - - contents.loadFile(path.join(fixtures, 'pages', 'ping-pong.html')) - }) - - it('sends message to WebContents (sandboxed renderer)', done => { - contents = webContents.create({ - preload: path.join(fixtures, 'module', 'preload-inject-ipc.js'), - sandbox: true - }) - - const payload = 'Hello World!' - - ipcRenderer.once('pong', (event, data) => { - expect(payload).to.equal(data) - done() - }) - - contents.once('did-finish-load', () => { - ipcRenderer.sendTo(contents.id, 'ping', payload) - }) - - contents.loadFile(path.join(fixtures, 'pages', 'ping-pong.html')) - }) - - it('sends message to WebContents (context isolation)', done => { - contents = webContents.create({ - preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), - contextIsolation: true - }) - - const payload = 'Hello World!' - - ipcRenderer.once('pong', (event, data) => { - expect(payload).to.equal(data) - done() - }) - - contents.once('did-finish-load', () => { - ipcRenderer.sendTo(contents.id, 'ping', payload) - }) - - contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) - }) + const generateSpecs = (description, webPreferences) => { + describe(description, () => { + it('sends message to WebContents', done => { + contents = webContents.create({ + preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), + ...webPreferences + }) - it('sends message to WebContents (context isolation and sandboxed renderer)', done => { - contents = webContents.create({ - preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), - contextIsolation: true, - sandbox: true - }) + const payload = 'Hello World!' - const payload = 'Hello World!' + ipcRenderer.once('pong', (event, data) => { + expect(payload).to.equal(data) + done() + }) - ipcRenderer.once('pong', (event, data) => { - expect(payload).to.equal(data) - done() - }) + contents.once('did-finish-load', () => { + ipcRenderer.sendTo(contents.id, 'ping', payload) + }) - contents.once('did-finish-load', () => { - ipcRenderer.sendTo(contents.id, 'ping', payload) - }) + contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) + }) - contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) - }) + it('sends message to WebContents (channel has special chars)', done => { + contents = webContents.create({ + preload: path.join(fixtures, 'module', 'preload-ipc-ping-pong.js'), + ...webPreferences + }) - it('sends message to WebContents (channel has special chars)', done => { - contents = webContents.create({ - preload: path.join(fixtures, 'module', 'preload-inject-ipc.js') - }) + const payload = 'Hello World!' - const payload = 'Hello World!' + ipcRenderer.once('pong-æøåü', (event, data) => { + expect(payload).to.equal(data) + done() + }) - ipcRenderer.once('pong-æøåü', (event, data) => { - expect(payload).to.equal(data) - done() - }) + contents.once('did-finish-load', () => { + ipcRenderer.sendTo(contents.id, 'ping-æøåü', payload) + }) - contents.once('did-finish-load', () => { - ipcRenderer.sendTo(contents.id, 'ping-æøåü', payload) + contents.loadFile(path.join(fixtures, 'pages', 'base-page.html')) + }) }) + } - contents.loadFile(path.join(fixtures, 'pages', 'ping-pong.html')) - }) + generateSpecs('without sandbox', {}) + generateSpecs('with sandbox', { sandbox: true }) + generateSpecs('with contextIsolation', { contextIsolation: true }) + generateSpecs('with contextIsolation + sandbox', { contextIsolation: true, sandbox: true }) }) describe('remote listeners', () => { diff --git a/spec/fixtures/module/preload-inject-ipc.js b/spec/fixtures/module/preload-inject-ipc.js deleted file mode 100644 index a1a45b097067c..0000000000000 --- a/spec/fixtures/module/preload-inject-ipc.js +++ /dev/null @@ -1,2 +0,0 @@ -const { ipcRenderer } = require('electron') -window.ipcRenderer = ipcRenderer diff --git a/spec/fixtures/module/preload-ipc-ping-pong.js b/spec/fixtures/module/preload-ipc-ping-pong.js index 8dd8e7a99609b..6ea0b32fdad95 100644 --- a/spec/fixtures/module/preload-ipc-ping-pong.js +++ b/spec/fixtures/module/preload-ipc-ping-pong.js @@ -1,4 +1,9 @@ const { ipcRenderer } = require('electron') + ipcRenderer.on('ping', function (event, payload) { ipcRenderer.sendTo(event.senderId, 'pong', payload) }) + +ipcRenderer.on('ping-æøåü', function (event, payload) { + ipcRenderer.sendTo(event.senderId, 'pong-æøåü', payload) +}) diff --git a/spec/fixtures/pages/ping-pong.html b/spec/fixtures/pages/ping-pong.html deleted file mode 100644 index 743131e0d1a90..0000000000000 --- a/spec/fixtures/pages/ping-pong.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - -