Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: <webview> not working with contextIsolation + sandbox #16469

Merged
merged 1 commit into from Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions atom/renderer/atom_sandboxed_renderer_client.cc
Expand Up @@ -202,6 +202,30 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
&preload_bundle_params, &preload_bundle_args, nullptr);
}

void AtomSandboxedRendererClient::SetupMainWorldOverrides(
v8::Handle<v8::Context> context,
content::RenderFrame* render_frame) {
// Setup window overrides in the main world context
// Wrap the bundle into a function that receives the isolatedWorld as
// an argument.
auto* isolate = context->GetIsolate();

mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
process.SetMethod("binding", GetBinding);

std::vector<v8::Local<v8::String>> isolated_bundle_params = {
node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
node::FIXED_ONE_BYTE_STRING(isolate, "isolatedWorld")};

std::vector<v8::Local<v8::Value>> isolated_bundle_args = {
process.GetHandle(),
GetContext(render_frame->GetWebFrame(), isolate)->Global()};

node::per_process::native_module_loader.CompileAndCall(
context, "electron/js2c/isolated_bundle", &isolated_bundle_params,
&isolated_bundle_args, nullptr);
}

void AtomSandboxedRendererClient::WillReleaseScriptContext(
v8::Handle<v8::Context> context,
content::RenderFrame* render_frame) {
Expand Down
2 changes: 1 addition & 1 deletion atom/renderer/atom_sandboxed_renderer_client.h
Expand Up @@ -29,7 +29,7 @@ class AtomSandboxedRendererClient : public RendererClientBase {
void WillReleaseScriptContext(v8::Handle<v8::Context> context,
content::RenderFrame* render_frame) override;
void SetupMainWorldOverrides(v8::Handle<v8::Context> context,
content::RenderFrame* render_frame) override {}
content::RenderFrame* render_frame) override;
// content::ContentRendererClient:
void RenderFrameCreated(content::RenderFrame*) override;
void RenderViewCreated(content::RenderView*) override;
Expand Down
1 change: 1 addition & 0 deletions filenames.gni
Expand Up @@ -74,6 +74,7 @@ filenames = {
"lib/renderer/web-view/web-view-constants.js",
"lib/renderer/web-view/web-view-element.js",
"lib/renderer/web-view/web-view-impl.js",
"lib/renderer/web-view/web-view-init.js",
"lib/renderer/api/exports/electron.js",
"lib/renderer/api/crash-reporter.js",
"lib/renderer/api/ipc-renderer.js",
Expand Down
15 changes: 10 additions & 5 deletions lib/isolated_renderer/init.js
Expand Up @@ -2,16 +2,21 @@

/* global nodeProcess, isolatedWorld */

// Note: Don't use "process", as it will be replaced by browserify's one.
const v8Util = nodeProcess.atomBinding('v8_util')
const atomBinding = require('@electron/internal/common/atom-binding-setup')(nodeProcess.binding, 'renderer')

const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')
const { webViewImpl, ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
const v8Util = atomBinding('v8_util')

const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl')

if (webViewImpl) {
// Must setup the WebView element in main world.
const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
setupWebView(v8Util, webViewImpl)
}

require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')

if (isolatedWorldArgs) {
const { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
}
54 changes: 19 additions & 35 deletions lib/renderer/init.js
Expand Up @@ -58,33 +58,29 @@ if (preloadScript) {
preloadScripts.push(preloadScript)
}

if (window.location.protocol === 'chrome-devtools:') {
// Override some inspector APIs.
require('@electron/internal/renderer/inspector')
} else if (window.location.protocol === 'chrome-extension:') {
// Add implementations of chrome API.
require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
} else if (window.location.protocol === 'chrome:') {
// Disable node integration for chrome UI scheme.
} else {
// Override default web functions.
require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)

// Inject content scripts.
require('@electron/internal/renderer/content-scripts-injector')
switch (window.location.protocol) {
case 'chrome-devtools:': {
// Override some inspector APIs.
require('@electron/internal/renderer/inspector')
break
}
case 'chrome-extension:': {
// Inject the chrome.* APIs that chrome extensions require
require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
break
}
default: {
// Override default web functions.
require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)

// Load webview tag implementation.
if (webviewTag && guestInstanceId == null) {
const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
if (contextIsolation) {
Object.assign(isolatedWorldArgs, { webViewImpl })
} else {
const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
setupWebView(v8Util, webViewImpl)
}
// Inject content scripts.
require('@electron/internal/renderer/content-scripts-injector')
}
}

// Load webview tag implementation.
require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, webviewTag, guestInstanceId)

// Pass the arguments to isolatedWorld.
if (contextIsolation) {
v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
Expand Down Expand Up @@ -163,15 +159,3 @@ for (const preloadScript of preloadScripts) {

// Warn about security issues
require('@electron/internal/renderer/security-warnings')(nodeIntegration)

// Report focus/blur events of webview to browser.
// Note that while Chromium content APIs have observer for focus/blur, they
// unfortunately do not work for webview.
if (guestInstanceId) {
window.addEventListener('focus', () => {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId)
})
window.addEventListener('blur', () => {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId)
})
}
35 changes: 35 additions & 0 deletions lib/renderer/web-view/web-view-init.js
@@ -0,0 +1,35 @@
'use strict'

const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
const v8Util = process.atomBinding('v8_util')

function handleFocusBlur (guestInstanceId) {
// Note that while Chromium content APIs have observer for focus/blur, they
// unfortunately do not work for webview.

window.addEventListener('focus', () => {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId)
})

window.addEventListener('blur', () => {
ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId)
})
}

module.exports = function (contextIsolation, webviewTag, guestInstanceId) {
// Load webview tag implementation.
if (webviewTag && guestInstanceId == null) {
const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
if (contextIsolation) {
v8Util.setHiddenValue(window, 'web-view-impl', webViewImpl)
} else {
const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
setupWebView(v8Util, webViewImpl)
}
}

if (guestInstanceId) {
// Report focus/blur events of webview to browser.
handleFocusBlur(guestInstanceId)
}
}
17 changes: 7 additions & 10 deletions lib/sandboxed_renderer/init.js
Expand Up @@ -105,8 +105,12 @@ function preloadRequire (module) {
throw new Error('module not found')
}

// Process command line arguments.
const { hasSwitch } = process.atomBinding('command_line')

const isBackgroundPage = hasSwitch('background-page')
const contextIsolation = hasSwitch('context-isolation')

switch (window.location.protocol) {
case 'chrome-devtools:': {
// Override some inspector APIs.
Expand All @@ -115,22 +119,15 @@ switch (window.location.protocol) {
}
case 'chrome-extension:': {
// Inject the chrome.* APIs that chrome extensions require
const isBackgroundPage = hasSwitch('background-page')
require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
break
}
}

if (binding.guestInstanceId) {
process.guestInstanceId = parseInt(binding.guestInstanceId)
}
const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId)

// Don't allow recursive `<webview>`.
if (!process.guestInstanceId && isWebViewTagEnabled) {
const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
setupWebView(v8Util, webViewImpl)
}
// Load webview tag implementation.
require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, isWebViewTagEnabled, guestInstanceId)

const errorUtils = require('@electron/internal/common/error-utils')

Expand Down
98 changes: 67 additions & 31 deletions spec/webview-spec.js
Expand Up @@ -76,6 +76,19 @@ describe('<webview> tag', function () {
await emittedOnce(ipcMain, 'pong')
})

it('works with sandbox', async () => {
const w = await openTheWindow({
show: false,
webPreferences: {
webviewTag: true,
nodeIntegration: true,
sandbox: true
}
})
w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
await emittedOnce(ipcMain, 'pong')
})

it('works with contextIsolation', async () => {
const w = await openTheWindow({
show: false,
Expand All @@ -89,6 +102,20 @@ describe('<webview> tag', function () {
await emittedOnce(ipcMain, 'pong')
})

it('works with contextIsolation + sandbox', async () => {
const w = await openTheWindow({
show: false,
webPreferences: {
webviewTag: true,
nodeIntegration: true,
contextIsolation: true,
sandbox: true
}
})
w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
await emittedOnce(ipcMain, 'pong')
})

it('is disabled by default', async () => {
const w = await openTheWindow({
show: false,
Expand Down Expand Up @@ -1349,47 +1376,56 @@ describe('<webview> tag', function () {
if (div != null) div.remove()
})

it('emits resize events', async () => {
const firstResizeSignal = waitForEvent(webview, 'resize')
const domReadySignal = waitForEvent(webview, 'dom-ready')
const generateSpecs = (description, sandbox) => {
describe(description, () => {
it('emits resize events', async () => {
const firstResizeSignal = waitForEvent(webview, 'resize')
const domReadySignal = waitForEvent(webview, 'dom-ready')

webview.src = `file://${fixtures}/pages/a.html`
div.appendChild(webview)
document.body.appendChild(div)
webview.src = `file://${fixtures}/pages/a.html`
webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}`
div.appendChild(webview)
document.body.appendChild(div)

const firstResizeEvent = await firstResizeSignal
expect(firstResizeEvent.target).to.equal(webview)
expect(firstResizeEvent.newWidth).to.equal(100)
expect(firstResizeEvent.newHeight).to.equal(10)
const firstResizeEvent = await firstResizeSignal
expect(firstResizeEvent.target).to.equal(webview)
expect(firstResizeEvent.newWidth).to.equal(100)
expect(firstResizeEvent.newHeight).to.equal(10)

await domReadySignal
await domReadySignal

const secondResizeSignal = waitForEvent(webview, 'resize')
const secondResizeSignal = waitForEvent(webview, 'resize')

const newWidth = 1234
const newHeight = 789
div.style.width = `${newWidth}px`
div.style.height = `${newHeight}px`
const newWidth = 1234
const newHeight = 789
div.style.width = `${newWidth}px`
div.style.height = `${newHeight}px`

const secondResizeEvent = await secondResizeSignal
expect(secondResizeEvent.target).to.equal(webview)
expect(secondResizeEvent.newWidth).to.equal(newWidth)
expect(secondResizeEvent.newHeight).to.equal(newHeight)
})
const secondResizeEvent = await secondResizeSignal
expect(secondResizeEvent.target).to.equal(webview)
expect(secondResizeEvent.newWidth).to.equal(newWidth)
expect(secondResizeEvent.newHeight).to.equal(newHeight)
})

it('emits focus event', async () => {
const domReadySignal = waitForEvent(webview, 'dom-ready')
webview.src = `file://${fixtures}/pages/a.html`
document.body.appendChild(webview)
it('emits focus event', async () => {
const domReadySignal = waitForEvent(webview, 'dom-ready')
webview.src = `file://${fixtures}/pages/a.html`
webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}`
document.body.appendChild(webview)

await domReadySignal
await domReadySignal

// If this test fails, check if webview.focus() still works.
const focusSignal = waitForEvent(webview, 'focus')
webview.focus()
// If this test fails, check if webview.focus() still works.
const focusSignal = waitForEvent(webview, 'focus')
webview.focus()

await focusSignal
})
await focusSignal
})
})
}

generateSpecs('without sandbox', false)
generateSpecs('with sandbox', true)
})

describe('zoom behavior', () => {
Expand Down