Skip to content

Commit

Permalink
fix: ensure that functions are not retained beyond their context bein…
Browse files Browse the repository at this point in the history
…g released
  • Loading branch information
MarshallOfSound committed Apr 21, 2020
1 parent aca2e4f commit ecb6317
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
14 changes: 14 additions & 0 deletions shell/renderer/api/context_bridge/render_frame_function_store.cc
Expand Up @@ -32,6 +32,20 @@ void RenderFrameFunctionStore::OnDestruct() {
delete this;
}

void RenderFrameFunctionStore::WillReleaseScriptContext(
v8::Local<v8::Context> context,
int32_t world_id) {
std::vector<size_t> to_remove;
for (auto const& pair : functions_) {
v8::Local<v8::Context> func_owning_context =
std::get<1>(pair.second).Get(context->GetIsolate());
if (func_owning_context == context)
to_remove.push_back(pair.first);
}
for (auto remove : to_remove)
functions_.erase(remove);
}

} // namespace context_bridge

} // namespace api
Expand Down
Expand Up @@ -29,6 +29,8 @@ class RenderFrameFunctionStore final : public content::RenderFrameObserver {

// RenderFrameObserver implementation.
void OnDestruct() override;
void WillReleaseScriptContext(v8::Local<v8::Context> context,
int32_t world_id) override;

size_t take_func_id() { return next_func_id_++; }

Expand Down
39 changes: 38 additions & 1 deletion spec-main/api-context-bridge-spec.ts
Expand Up @@ -2,17 +2,33 @@ import { BrowserWindow, ipcMain } from 'electron/main';
import { contextBridge } from 'electron/renderer';
import { expect } from 'chai';
import * as fs from 'fs-extra';
import * as http from 'http';
import * as os from 'os';
import * as path from 'path';

import { closeWindow } from './window-helpers';
import { emittedOnce } from './events-helpers';
import { AddressInfo } from 'net';

const fixturesPath = path.resolve(__dirname, 'fixtures', 'api', 'context-bridge');

describe('contextBridge', () => {
let w: BrowserWindow;
let dir: string;
let server: http.Server;

before(async () => {
server = http.createServer((req, res) => {
res.setHeader('Content-Type', 'text/html');
res.end(fs.readFileSync(path.resolve(fixturesPath, 'empty.html')));
});
await new Promise(resolve => server.listen(0, resolve));
});

after(async () => {
if (server) await new Promise(resolve => server.close(resolve));
server = null as any;
});

afterEach(async () => {
await closeWindow(w);
Expand Down Expand Up @@ -65,7 +81,7 @@ describe('contextBridge', () => {
preload: path.resolve(tmpDir, 'preload.js')
}
});
await w.loadFile(path.resolve(fixturesPath, 'empty.html'));
await w.loadURL(`http://127.0.0.1:${(server.address() as AddressInfo).port}`);
};

const callWithBindings = (fn: Function) =>
Expand Down Expand Up @@ -343,6 +359,27 @@ describe('contextBridge', () => {
});
}

if (useSandbox) {
it('should not leak the global hold on methods sent across contexts when reloading a sandboxed renderer', async () => {
await makeBindingWindow(() => {
require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()));
contextBridge.exposeInMainWorld('example', {
getFunction: () => () => 123
});
require('electron').ipcRenderer.send('window-ready-for-tasking');
});
const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking');
expect((await getGCInfo()).functionCount).to.equal(1);
await callWithBindings((root: any) => {
root.location.reload();
});
await loadPromise;
// If this is ever "2" it means we leaked the exposed function and
// therefore the entire context after a reload
expect((await getGCInfo()).functionCount).to.equal(1);
});
}

it('it should not let you overwrite existing exposed things', async () => {
await makeBindingWindow(() => {
let threw = false;
Expand Down

0 comments on commit ecb6317

Please sign in to comment.