fix: ensure that functions are not retained beyond their context being released (#23207)

This commit is contained in:
Samuel Attard 2020-04-21 18:05:01 -07:00 committed by GitHub
parent aca2e4f968
commit 0cbcee6740
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 50 additions and 1 deletions

View file

@ -32,6 +32,16 @@ void RenderFrameFunctionStore::OnDestruct() {
delete this;
}
void RenderFrameFunctionStore::WillReleaseScriptContext(
v8::Local<v8::Context> context,
int32_t world_id) {
base::EraseIf(functions_, [context](auto const& pair) {
v8::Local<v8::Context> func_owning_context =
std::get<1>(pair.second).Get(context->GetIsolate());
return func_owning_context == context;
});
}
} // namespace context_bridge
} // namespace api

View file

@ -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_++; }

View file

@ -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('');
});
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);
@ -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) =>
@ -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;