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

feat: add a new contextBridge module #20307

Merged
merged 43 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
5c4c9d5
feat: add a new contextBridge module
MarshallOfSound Sep 20, 2019
64fa6f3
chore: fix docs linting
MarshallOfSound Sep 20, 2019
4b79d73
feat: add support for function arguments being proxied
MarshallOfSound Sep 20, 2019
50c3dfd
chore: ensure that contextBridge can only be used when contextIsolati…
MarshallOfSound Sep 20, 2019
4fe3eb6
docs: getReverseBinding can be null
MarshallOfSound Sep 20, 2019
1104711
docs: fix broken links in md file
MarshallOfSound Sep 20, 2019
f308827
feat: add support for promises in function parameters
MarshallOfSound Sep 20, 2019
291e71e
fix: linting failure for explicit constructor
MarshallOfSound Sep 21, 2019
dc170d4
Update atom_api_context_bridge.cc
MarshallOfSound Sep 21, 2019
e897664
chore: update docs and API design as per feedback
MarshallOfSound Sep 24, 2019
7d652be
refactor: remove reverse bindings and handle GC'able functions across…
MarshallOfSound Sep 26, 2019
07a87bc
chore: only expose debugGC in testing builds
MarshallOfSound Sep 26, 2019
fe23ca3
fix: do not proxy promises as objects
MarshallOfSound Sep 26, 2019
1f9312a
spec: add complete spec coverage for contextBridge
MarshallOfSound Sep 26, 2019
cce347c
spec: add tests for null/undefined and the anti-overwrite logic
MarshallOfSound Sep 26, 2019
c1d1ae0
chore: fix linting
MarshallOfSound Sep 26, 2019
8ad78c0
spec: add complex nested back-and-forth function calling
MarshallOfSound Sep 27, 2019
403cb0e
fix: expose contextBridge in sandboxed renderers
MarshallOfSound Sep 27, 2019
de67801
refactor: improve security of default_app using the new contextBridge…
MarshallOfSound Sep 27, 2019
d568dac
s/bindAPIInMainWorld/exposeInMainWorld
MarshallOfSound Sep 27, 2019
a433905
chore: sorry for this commit, its a big one, I fixed like everything …
MarshallOfSound Oct 5, 2019
1fb7e4d
chore: remove PassedValueCache as it is unused now
MarshallOfSound Oct 5, 2019
42c31cb
chore: move to anonymous namespace
MarshallOfSound Oct 5, 2019
2b9cbf2
refactor: remove PassValueToOtherContextWithCache
MarshallOfSound Oct 5, 2019
8573d0f
chore: remove commented unused code blocks
MarshallOfSound Oct 5, 2019
8868b76
chore: remove .only
MarshallOfSound Oct 5, 2019
ed8a8c2
chore: remote commented code
MarshallOfSound Oct 5, 2019
c57c8ae
refactor: extract RenderFramePersistenceStore
MarshallOfSound Oct 5, 2019
7306cfe
spec: ensure it works with numbered keys
MarshallOfSound Oct 5, 2019
03c1cd9
fix: handle number keys correctly
MarshallOfSound Oct 5, 2019
3c77df0
fix: sort out the linter
MarshallOfSound Oct 5, 2019
bb41a56
spec: update default_app asar spec for removed file
MarshallOfSound Oct 5, 2019
785bc2f
refactor: change signatures to return v8 objects directly rather than…
MarshallOfSound Oct 6, 2019
33b598d
refactor: use the v8 serializer to support cloneable buffers and othe…
MarshallOfSound Oct 9, 2019
6b05ca0
chore: fix linting
MarshallOfSound Oct 10, 2019
b7b23ae
fix: handle hash collisions with a linked list in the map
MarshallOfSound Oct 11, 2019
7aa9d80
fix: enforce a recursion limit on the context bridge
MarshallOfSound Oct 11, 2019
5a5548e
chore: fix linting
MarshallOfSound Oct 11, 2019
55bbdac
chore: remove TODO
MarshallOfSound Oct 11, 2019
84598b8
chore: adapt for PR feedback
MarshallOfSound Oct 15, 2019
7913f52
chore: remove .only
MarshallOfSound Oct 15, 2019
a4a5af1
chore: clean up docs and clean up the proxy map when objects are rele…
MarshallOfSound Oct 16, 2019
4475a88
chore: ensure we cache object values that are cloned through the V8 s…
MarshallOfSound Oct 17, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions default_app/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

<head>
<title>Electron</title>
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; connect-src 'self'" />
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'sha256-6PH54BfkNq/EMMhUY7nhHf3c+AxloOwfy7hWyT01CM8='; style-src 'self'; img-src 'self'; connect-src 'self'" />
<link href="./styles.css" type="text/css" rel="stylesheet" />
<link href="./octicon/build.css" type="text/css" rel="stylesheet" />
<script defer src="./index.js"></script>
</head>

<body>
Expand Down Expand Up @@ -84,6 +83,9 @@ <h4>Forge</h4>
</div>
</div>
</nav>
<script>
window.electronDefaultApp.initialize()
</script>
</body>

</html>
30 changes: 0 additions & 30 deletions default_app/index.ts

This file was deleted.

37 changes: 35 additions & 2 deletions default_app/preload.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,31 @@
import { ipcRenderer } from 'electron'
import { ipcRenderer, contextBridge } from 'electron'

async function getOcticonSvg (name: string) {
try {
const response = await fetch(`octicon/${name}.svg`)
const div = document.createElement('div')
div.innerHTML = await response.text()
return div
} catch {
return null
}
}

async function loadSVG (element: HTMLSpanElement) {
for (const cssClass of element.classList) {
if (cssClass.startsWith('octicon-')) {
const icon = await getOcticonSvg(cssClass.substr(8))
if (icon) {
for (const elemClass of element.classList) {
icon.classList.add(elemClass)
}
element.before(icon)
element.remove()
break
}
}
}
}
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

async function initialize () {
const electronPath = await ipcRenderer.invoke('bootstrap')
Expand All @@ -15,6 +42,12 @@ async function initialize () {
replaceText('.node-version', `Node v${process.versions.node}`)
replaceText('.v8-version', `v8 v${process.versions.v8}`)
replaceText('.command-example', `${electronPath} path-to-app`)

for (const element of document.querySelectorAll<HTMLSpanElement>('.octicon')) {
loadSVG(element)
}
}

document.addEventListener('DOMContentLoaded', initialize)
contextBridge.exposeInMainWorld('electronDefaultApp', {
initialize
})
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
111 changes: 111 additions & 0 deletions docs/api/context-bridge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# contextBridge

> Create a safe, bi-directional, synchronous bridge across isolated contexts
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

Process: [Renderer](../glossary.md#renderer-process)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a short paragraph explaining why the reader might want to use this. What problems does this solve?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't get addressed...?

An example of exposing an API to a renderer from an isolated preload script is given below:

```javascript
// Preload (Isolated World)
const { contextBridge, ipcRenderer } = require('electron')

contextBridge.exposeInMainWorld(
'electron',
{
doThing: () => ipcRenderer.send('do-a-thing')
}
)
```

```javascript
// Renderer (Main World)
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

window.electron.doThing()
```

## Glossary

### Main World

The "Main World" is the javascript context that your main renderer code runs in. By default the page you load in your renderer
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
executes code in this world.

### Isolated World

When `contextIsolation` is enabled in your `webPreferences` your `preload` scripts run in an "Isolated World". You can read more about
context isolation and what it affects in the [BrowserWindow](browser-window.md) docs.
Copy link
Member

@codebytere codebytere Sep 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since contextIsolation is mentioned in a BW constructor option and isn't super obvious from just a link to browser-window.md that it would be worthwhile to lower that barrier and re-copy it here or move it into a standalone document we could link to


## Methods

The `contextBridge` module has the following methods:

### `contextBridge.exposeInMainWorld(apiKey, api)`
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

* `apiKey` String - The key to inject the API onto `window` with. The API will be accessible on `window[apiKey]`.
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
* `api` Record<String, any> - Your API object, more information on what this API can be and how it works is available below.

## Usage

### API Objects

The `api` object provided to [`exposeInMainWorld`](#contextbridgeexposeinmainworldapikey-api) must be an object
whose keys are strings and values are a `Function`, `String`, `Number`, `Array`, `Boolean` or another nested object that meets the same conditions.
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

`Function` values are proxied to the other context and all other values are **copied** and **frozen**. I.e. Any data / primitives sent in
the API object become immutable and updates on either side of the bridge do not result in an update on the other side.
nornagon marked this conversation as resolved.
Show resolved Hide resolved

An example of a complex API object is shown below.

```javascript
const { contextBridge } = require('electron')

contextBridge.exposeInMainWorld(
'electron',
{
doThing: () => ipcRenderer.send('do-a-thing'),
myPromises: [Promise.resolve(), Promise.reject(new Error('whoops'))],
anAsyncFunction: async () => 123,
data: {
myFlags: ['a', 'b', 'c'],
bootTime: 1234
},
nestedAPI: {
evenDeeper: {
youCanDoThisAsMuchAsYouWant: {
fn: () => ({
returnData: 123
})
}
}
}
}
)
```

### API Functions

`Function` values that you bind through the `contextBridge` are proxied through Electron to ensure that contexts remain isolated. This
results in some key limitations that we've outlined below.

#### Parameter / Error / Return Type support

Because parameters, errors and return values are **copied** when they are sent over the bridge there are only certain types that can be used.
At a high level if the type you want to use can be serialized and un-serialized into the same object it will work. A table of type support
has been included below for completeness.

| Type | Complexity | Parameter Support | Return Value Support | Limitations |
| ---- | ---------- | ----------------- | -------------------- | ----------- |
| `String` | Simple | ✅ | ✅ | N/A |
Copy link
Contributor

@miniak miniak Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about Date, RegExp, ArrayBuffer, ArrayBufferView? those are now natively supported by the IPC payload serializer. It would be nice if they were supported here as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are already supported, just need to put them or structured clone into this table

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update this to point to SCA probably.

| `Number` | Simple | ✅ | ✅ | N/A |
| `Boolean` | Simple | ✅ | ✅ | N/A |
| `Object` | Complex | ✅ | ✅ | Keys must be supported "Simple" types in this table. Values must be supported in this table. Prototype modifications are dropped. Sending custom classes will copy values but not the prototype. |
| `Array` | Complex | ✅ | ✅ | Same limitations as the `Object` type |
| `Error` | Complex | ✅ | ✅ | Errors that are thrown are also copied, this can result in the message and stack trace of the error changing slightly due to being thrown in a different context |
| `Promise` | Complex | ✅ | ✅ | Promises are only proxied if they are a the return value or exact parameter. Promises nested in arrays or obejcts will be dropped. |
| `Function` | Complex | ✅ | ✅ | Prototype modifications are dropped. Sending classes or constructors will not work. |
| [Cloneable Types](https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm) | Simple | ✅ | ✅ | See the linked document on cloneable types |
| `Symbol` | N/A | ❌ | ❌ | Symbols cannot be copied across contexts so they are dropped |


If the type you care about is not in the above table it is probably not supported.
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions filenames.auto.gni
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ auto_filenames = {
"docs/api/command-line-switches.md",
"docs/api/command-line.md",
"docs/api/content-tracing.md",
"docs/api/context-bridge.md",
"docs/api/cookies.md",
"docs/api/crash-reporter.md",
"docs/api/debugger.md",
Expand Down Expand Up @@ -142,6 +143,7 @@ auto_filenames = {
"lib/common/remote/buffer-utils.ts",
"lib/common/remote/is-promise.ts",
"lib/common/web-view-methods.ts",
"lib/renderer/api/context-bridge.ts",
"lib/renderer/api/crash-reporter.js",
"lib/renderer/api/desktop-capturer.ts",
"lib/renderer/api/ipc-renderer.ts",
Expand Down Expand Up @@ -303,6 +305,7 @@ auto_filenames = {
"lib/common/remote/is-promise.ts",
"lib/common/reset-search-paths.ts",
"lib/common/web-view-methods.ts",
"lib/renderer/api/context-bridge.ts",
"lib/renderer/api/crash-reporter.js",
"lib/renderer/api/desktop-capturer.ts",
"lib/renderer/api/exports/electron.ts",
Expand Down Expand Up @@ -352,6 +355,7 @@ auto_filenames = {
"lib/common/remote/buffer-utils.ts",
"lib/common/remote/is-promise.ts",
"lib/common/reset-search-paths.ts",
"lib/renderer/api/context-bridge.ts",
"lib/renderer/api/crash-reporter.js",
"lib/renderer/api/desktop-capturer.ts",
"lib/renderer/api/exports/electron.ts",
Expand Down
5 changes: 4 additions & 1 deletion filenames.gni
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
filenames = {
default_app_ts_sources = [
"default_app/default_app.ts",
"default_app/index.ts",
"default_app/main.ts",
"default_app/preload.ts",
]
Expand Down Expand Up @@ -554,6 +553,10 @@ filenames = {
"shell/common/promise_util.cc",
"shell/common/skia_util.h",
"shell/common/skia_util.cc",
"shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc",
"shell/renderer/api/context_bridge/render_frame_context_bridge_store.h",
"shell/renderer/api/atom_api_context_bridge.cc",
"shell/renderer/api/atom_api_context_bridge.h",
"shell/renderer/api/atom_api_renderer_ipc.cc",
"shell/renderer/api/atom_api_spell_check_client.cc",
"shell/renderer/api/atom_api_spell_check_client.h",
Expand Down
20 changes: 20 additions & 0 deletions lib/renderer/api/context-bridge.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const { hasSwitch } = process.electronBinding('command_line')
const binding = process.electronBinding('context_bridge')

const contextIsolationEnabled = hasSwitch('context-isolation')
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

const checkContextIsolationEnabled = () => {
if (!contextIsolationEnabled) throw new Error('contextBridge API can only be used when contextIsolation is enabled')
}

const contextBridge = {
exposeInMainWorld: (key: string, api: Record<string, any>) => {
checkContextIsolationEnabled()
return binding.exposeAPIInMainWorld(key, api)
},
debugGC: () => binding._debugGCMaps({})
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
}

if (!binding._debugGCMaps) delete contextBridge.debugGC
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved

export default contextBridge
1 change: 1 addition & 0 deletions lib/renderer/api/module-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const enableRemoteModule = v8Util.getHiddenValue<boolean>(global, 'enableRemoteM

// Renderer side modules, please sort alphabetically.
export const rendererModuleList: ElectronInternal.ModuleEntry[] = [
{ name: 'contextBridge', loader: () => require('./context-bridge') },
{ name: 'crashReporter', loader: () => require('./crash-reporter') },
{ name: 'ipcRenderer', loader: () => require('./ipc-renderer') },
{ name: 'webFrame', loader: () => require('./web-frame') }
Expand Down
4 changes: 4 additions & 0 deletions lib/sandboxed_renderer/api/module-list.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
const features = process.electronBinding('features')

export const moduleList: ElectronInternal.ModuleEntry[] = [
{
name: 'contextBridge',
loader: () => require('@electron/internal/renderer/api/context-bridge')
},
{
name: 'crashReporter',
loader: () => require('@electron/internal/renderer/api/crash-reporter')
Expand Down
4 changes: 2 additions & 2 deletions native_mate/native_mate/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class Arguments {

template <typename T>
bool GetHolder(T* out) {
return ConvertFromV8(isolate_, info_->Holder(), out);
return mate::ConvertFromV8(isolate_, info_->Holder(), out);
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
}

template <typename T>
Expand All @@ -57,7 +57,7 @@ class Arguments {
return false;
}
v8::Local<v8::Value> val = (*info_)[next_];
bool success = ConvertFromV8(isolate_, val, out);
bool success = mate::ConvertFromV8(isolate_, val, out);
if (success)
next_++;
return success;
Expand Down
17 changes: 17 additions & 0 deletions native_mate/native_mate/dictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ class Dictionary {

static Dictionary CreateEmpty(v8::Isolate* isolate);

bool Has(base::StringPiece key) const {
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
v8::Local<v8::Context> context = isolate_->GetCurrentContext();
v8::Local<v8::String> v8_key = StringToV8(isolate_, key);
return internal::IsTrue(GetHandle()->Has(context, v8_key));
}

template <typename T>
bool Get(base::StringPiece key, T* out) const {
// Check for existence before getting, otherwise this method will always
Expand Down Expand Up @@ -76,6 +82,17 @@ class Dictionary {
return !result.IsNothing() && result.FromJust();
}

template <typename T>
bool SetReadOnlyNonConfigurable(base::StringPiece key, T val) {
v8::Local<v8::Value> v8_value;
if (!TryConvertToV8(isolate_, val, &v8_value))
return false;
v8::Maybe<bool> result = GetHandle()->DefineOwnProperty(
isolate_->GetCurrentContext(), StringToV8(isolate_, key), v8_value,
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete));
return !result.IsNothing() && result.FromJust();
}

template <typename T>
bool SetMethod(base::StringPiece key, const T& callback) {
return GetHandle()
Expand Down
1 change: 1 addition & 0 deletions shell/common/node_bindings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
V(atom_common_screen) \
V(atom_common_shell) \
V(atom_common_v8_util) \
V(atom_renderer_context_bridge) \
V(atom_renderer_ipc) \
V(atom_renderer_web_frame)

Expand Down
10 changes: 10 additions & 0 deletions shell/common/promise_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ class Promise {
return GetInner()->Reject(GetContext(), v8::Undefined(isolate()));
}

v8::Maybe<bool> Reject(v8::Local<v8::Value> exception) {
MarshallOfSound marked this conversation as resolved.
Show resolved Hide resolved
v8::HandleScope handle_scope(isolate());
v8::MicrotasksScope script_scope(isolate(),
v8::MicrotasksScope::kRunMicrotasks);
v8::Context::Scope context_scope(
v8::Local<v8::Context>::New(isolate(), GetContext()));

return GetInner()->Reject(GetContext(), exception);
}

template <typename... ResolveType>
v8::MaybeLocal<v8::Promise> Then(
base::OnceCallback<void(ResolveType...)> cb) {
Expand Down
Loading