mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: correctly track receiver for methods called via ctx bridge (#39978)

Browse source 
* fix: correctly track receiver for methods called via ctx bridge

* spec: test for correct contextBridge passage

---------

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
Samuel Attard 2023-10-18 07:21:42 -07:00 committed by GitHub
parent 5b105f911f
commit fd2861117e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 90 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
shell/renderer/api/electron_api_context_bridge.cc
View file

@ -44,6 +44,8 @@ namespace api {
namespace context_bridge { namespace context_bridge {
const char kProxyFunctionPrivateKey[] = "electron_contextBridge_proxy_fn"; const char kProxyFunctionPrivateKey[] = "electron_contextBridge_proxy_fn";
const char kProxyFunctionReceiverPrivateKey[] =
"electron_contextBridge_proxy_fn_receiver";
const char kSupportsDynamicPropertiesPrivateKey[] = const char kSupportsDynamicPropertiesPrivateKey[] =
"electron_contextBridge_supportsDynamicProperties"; "electron_contextBridge_supportsDynamicProperties";
const char kOriginalFunctionPrivateKey[] = "electron_contextBridge_original_fn"; const char kOriginalFunctionPrivateKey[] = "electron_contextBridge_original_fn";
@ -138,6 +140,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
v8::Local<v8::Context> source_context, v8::Local<v8::Context> source_context,
v8::Local<v8::Context> destination_context, v8::Local<v8::Context> destination_context,
v8::Local<v8::Value> value, v8::Local<v8::Value> value,
v8::Local<v8::Value> parent_value,
context_bridge::ObjectCache* object_cache, context_bridge::ObjectCache* object_cache,
bool support_dynamic_properties, bool support_dynamic_properties,
int recursion_depth, int recursion_depth,
@ -199,6 +202,9 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
v8::Object::New(destination_context->GetIsolate()); v8::Object::New(destination_context->GetIsolate());
SetPrivate(destination_context, state, SetPrivate(destination_context, state,
context_bridge::kProxyFunctionPrivateKey, func); context_bridge::kProxyFunctionPrivateKey, func);
SetPrivate(destination_context, state,
context_bridge::kProxyFunctionReceiverPrivateKey,
parent_value);
SetPrivate(destination_context, state, SetPrivate(destination_context, state,
context_bridge::kSupportsDynamicPropertiesPrivateKey, context_bridge::kSupportsDynamicPropertiesPrivateKey,
gin::ConvertToV8(destination_context->GetIsolate(), gin::ConvertToV8(destination_context->GetIsolate(),
@ -246,10 +252,12 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
v8::MaybeLocal<v8::Value> val; v8::MaybeLocal<v8::Value> val;
{ {
v8::TryCatch try_catch(isolate); v8::TryCatch try_catch(isolate);
v8::Local<v8::Context> source_context =
global_source_context.Get(isolate);
val = PassValueToOtherContext( val = PassValueToOtherContext(
global_source_context.Get(isolate), source_context, global_destination_context.Get(isolate), result,
global_destination_context.Get(isolate), result, &object_cache, source_context->Global(), &object_cache, false, 0,
false, 0, BridgeErrorTarget::kDestination); BridgeErrorTarget::kDestination);
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
if (try_catch.Message().IsEmpty()) { if (try_catch.Message().IsEmpty()) {
proxied_promise->RejectWithErrorMessage( proxied_promise->RejectWithErrorMessage(
@ -292,10 +300,12 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
v8::MaybeLocal<v8::Value> val; v8::MaybeLocal<v8::Value> val;
{ {
v8::TryCatch try_catch(isolate); v8::TryCatch try_catch(isolate);
v8::Local<v8::Context> source_context =
global_source_context.Get(isolate);
val = PassValueToOtherContext( val = PassValueToOtherContext(
global_source_context.Get(isolate), source_context, global_destination_context.Get(isolate), result,
global_destination_context.Get(isolate), result, &object_cache, source_context->Global(), &object_cache, false, 0,
false, 0, BridgeErrorTarget::kDestination); BridgeErrorTarget::kDestination);
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
if (try_catch.Message().IsEmpty()) { if (try_catch.Message().IsEmpty()) {
proxied_promise->RejectWithErrorMessage( proxied_promise->RejectWithErrorMessage(
@ -362,7 +372,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
for (size_t i = 0; i < length; i++) { for (size_t i = 0; i < length; i++) {
auto value_for_array = PassValueToOtherContext( auto value_for_array = PassValueToOtherContext(
source_context, destination_context, source_context, destination_context,
arr->Get(source_context, i).ToLocalChecked(), object_cache, arr->Get(source_context, i).ToLocalChecked(), value, object_cache,
support_dynamic_properties, recursion_depth + 1, error_target); support_dynamic_properties, recursion_depth + 1, error_target);
if (value_for_array.IsEmpty()) if (value_for_array.IsEmpty())
return v8::MaybeLocal<v8::Value>(); return v8::MaybeLocal<v8::Value>();
@ -442,8 +452,10 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
context_bridge::kSupportsDynamicPropertiesPrivateKey); context_bridge::kSupportsDynamicPropertiesPrivateKey);
v8::MaybeLocal<v8::Value> maybe_func = GetPrivate( v8::MaybeLocal<v8::Value> maybe_func = GetPrivate(
calling_context, data, context_bridge::kProxyFunctionPrivateKey); calling_context, data, context_bridge::kProxyFunctionPrivateKey);
v8::MaybeLocal<v8::Value> maybe_recv = GetPrivate(
calling_context, data, context_bridge::kProxyFunctionReceiverPrivateKey);
v8::Local<v8::Value> func_value; v8::Local<v8::Value> func_value;
if (sdp_value.IsEmpty() || maybe_func.IsEmpty() || if (sdp_value.IsEmpty() || maybe_func.IsEmpty() || maybe_recv.IsEmpty() ||
!gin::ConvertFromV8(args.isolate(), sdp_value.ToLocalChecked(), !gin::ConvertFromV8(args.isolate(), sdp_value.ToLocalChecked(),
&support_dynamic_properties) || &support_dynamic_properties) ||
!maybe_func.ToLocal(&func_value)) !maybe_func.ToLocal(&func_value))
@ -463,8 +475,9 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
for (auto value : original_args) { for (auto value : original_args) {
auto arg = PassValueToOtherContext( auto arg = PassValueToOtherContext(
calling_context, func_owning_context, value, &object_cache, calling_context, func_owning_context, value,
support_dynamic_properties, 0, BridgeErrorTarget::kSource); calling_context->Global(), &object_cache, support_dynamic_properties,
0, BridgeErrorTarget::kSource);
if (arg.IsEmpty()) if (arg.IsEmpty())
return; return;
proxied_args.push_back(arg.ToLocalChecked()); proxied_args.push_back(arg.ToLocalChecked());
@ -475,7 +488,8 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
v8::Local<v8::Value> error_message; v8::Local<v8::Value> error_message;
{ {
v8::TryCatch try_catch(args.isolate()); v8::TryCatch try_catch(args.isolate());
maybe_return_value = func->Call(func_owning_context, func, maybe_return_value =
func->Call(func_owning_context, maybe_recv.ToLocalChecked(),
proxied_args.size(), proxied_args.data()); proxied_args.size(), proxied_args.data());
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
did_error = true; did_error = true;
@ -531,6 +545,7 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
v8::TryCatch try_catch(args.isolate()); v8::TryCatch try_catch(args.isolate());
ret = PassValueToOtherContext(func_owning_context, calling_context, ret = PassValueToOtherContext(func_owning_context, calling_context,
maybe_return_value.ToLocalChecked(), maybe_return_value.ToLocalChecked(),
func_owning_context->Global(),
&object_cache, support_dynamic_properties, &object_cache, support_dynamic_properties,
0, BridgeErrorTarget::kDestination); 0, BridgeErrorTarget::kDestination);
if (try_catch.HasCaught()) { if (try_catch.HasCaught()) {
@ -607,18 +622,18 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
v8::Local<v8::Value> getter_proxy; v8::Local<v8::Value> getter_proxy;
v8::Local<v8::Value> setter_proxy; v8::Local<v8::Value> setter_proxy;
if (!getter.IsEmpty()) { if (!getter.IsEmpty()) {
if (!PassValueToOtherContext(source_context, destination_context, if (!PassValueToOtherContext(
getter, object_cache, source_context, destination_context, getter,
support_dynamic_properties, 1, api.GetHandle(), object_cache,
error_target) support_dynamic_properties, 1, error_target)
.ToLocal(&getter_proxy)) .ToLocal(&getter_proxy))
continue; continue;
} }
if (!setter.IsEmpty()) { if (!setter.IsEmpty()) {
if (!PassValueToOtherContext(source_context, destination_context, if (!PassValueToOtherContext(
setter, object_cache, source_context, destination_context, setter,
support_dynamic_properties, 1, api.GetHandle(), object_cache,
error_target) support_dynamic_properties, 1, error_target)
.ToLocal(&setter_proxy)) .ToLocal(&setter_proxy))
continue; continue;
} }
@ -635,8 +650,9 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
continue; continue;
auto passed_value = PassValueToOtherContext( auto passed_value = PassValueToOtherContext(
source_context, destination_context, value, object_cache, source_context, destination_context, value, api.GetHandle(),
support_dynamic_properties, recursion_depth + 1, error_target); object_cache, support_dynamic_properties, recursion_depth + 1,
error_target);
if (passed_value.IsEmpty()) if (passed_value.IsEmpty())
return v8::MaybeLocal<v8::Object>(); return v8::MaybeLocal<v8::Object>();
proxy.Set(key, passed_value.ToLocalChecked()); proxy.Set(key, passed_value.ToLocalChecked());
@ -683,7 +699,8 @@ void ExposeAPIInWorld(v8::Isolate* isolate,
v8::Context::Scope target_context_scope(target_context); v8::Context::Scope target_context_scope(target_context);
v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext( v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
electron_isolated_context, target_context, api, &object_cache, false, 0, electron_isolated_context, target_context, api,
electron_isolated_context->Global(), &object_cache, false, 0,
BridgeErrorTarget::kSource); BridgeErrorTarget::kSource);
if (maybe_proxy.IsEmpty()) if (maybe_proxy.IsEmpty())
return; return;
@ -732,9 +749,11 @@ void OverrideGlobalValueFromIsolatedWorld(
{ {
v8::Context::Scope main_context_scope(main_context); v8::Context::Scope main_context_scope(main_context);
context_bridge::ObjectCache object_cache; context_bridge::ObjectCache object_cache;
v8::Local<v8::Context> source_context = value->GetCreationContextChecked();
v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext( v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
value->GetCreationContextChecked(), main_context, value, &object_cache, source_context, main_context, value, source_context->Global(),
support_dynamic_properties, 1, BridgeErrorTarget::kSource); &object_cache, support_dynamic_properties, 1,
BridgeErrorTarget::kSource);
DCHECK(!maybe_proxy.IsEmpty()); DCHECK(!maybe_proxy.IsEmpty());
auto proxy = maybe_proxy.ToLocalChecked(); auto proxy = maybe_proxy.ToLocalChecked();
@ -768,15 +787,19 @@ bool OverrideGlobalPropertyFromIsolatedWorld(
v8::Local<v8::Value> getter_proxy; v8::Local<v8::Value> getter_proxy;
v8::Local<v8::Value> setter_proxy; v8::Local<v8::Value> setter_proxy;
if (!getter->IsNullOrUndefined()) { if (!getter->IsNullOrUndefined()) {
v8::Local<v8::Context> source_context =
getter->GetCreationContextChecked();
v8::MaybeLocal<v8::Value> maybe_getter_proxy = PassValueToOtherContext( v8::MaybeLocal<v8::Value> maybe_getter_proxy = PassValueToOtherContext(
getter->GetCreationContextChecked(), main_context, getter, source_context, main_context, getter, source_context->Global(),
&object_cache, false, 1, BridgeErrorTarget::kSource); &object_cache, false, 1, BridgeErrorTarget::kSource);
DCHECK(!maybe_getter_proxy.IsEmpty()); DCHECK(!maybe_getter_proxy.IsEmpty());
getter_proxy = maybe_getter_proxy.ToLocalChecked(); getter_proxy = maybe_getter_proxy.ToLocalChecked();
} }
if (!setter->IsNullOrUndefined() && setter->IsObject()) { if (!setter->IsNullOrUndefined() && setter->IsObject()) {
v8::Local<v8::Context> source_context =
getter->GetCreationContextChecked();
v8::MaybeLocal<v8::Value> maybe_setter_proxy = PassValueToOtherContext( v8::MaybeLocal<v8::Value> maybe_setter_proxy = PassValueToOtherContext(
getter->GetCreationContextChecked(), main_context, setter, source_context, main_context, setter, source_context->Global(),
&object_cache, false, 1, BridgeErrorTarget::kSource); &object_cache, false, 1, BridgeErrorTarget::kSource);
DCHECK(!maybe_setter_proxy.IsEmpty()); DCHECK(!maybe_setter_proxy.IsEmpty());
setter_proxy = maybe_setter_proxy.ToLocalChecked(); setter_proxy = maybe_setter_proxy.ToLocalChecked();

8
shell/renderer/api/electron_api_context_bridge.h
View file

@ -36,6 +36,14 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
v8::Local<v8::Context> source_context, v8::Local<v8::Context> source_context,
v8::Local<v8::Context> destination_context, v8::Local<v8::Context> destination_context,
v8::Local<v8::Value> value, v8::Local<v8::Value> value,
/**
* Used to automatically bind a function across
* worlds to its appropriate default "this" value.
*
* If this value is the root of a tree going over
* the bridge set this to the "context" of the value.
*/
v8::Local<v8::Value> parent_value,
context_bridge::ObjectCache* object_cache, context_bridge::ObjectCache* object_cache,
bool support_dynamic_properties, bool support_dynamic_properties,
int recursion_depth, int recursion_depth,

9
shell/renderer/api/electron_api_web_frame.cc
View file

@ -140,9 +140,12 @@ class ScriptExecutionCallback {
{ {
v8::TryCatch try_catch(isolate); v8::TryCatch try_catch(isolate);
context_bridge::ObjectCache object_cache; context_bridge::ObjectCache object_cache;
maybe_result = PassValueToOtherContext( v8::Local<v8::Context> source_context =
result->GetCreationContextChecked(), promise_.GetContext(), result, result->GetCreationContextChecked();
&object_cache, false, 0, BridgeErrorTarget::kSource); maybe_result =
PassValueToOtherContext(source_context, promise_.GetContext(), result,
source_context->Global(), &object_cache,
false, 0, BridgeErrorTarget::kSource);
if (maybe_result.IsEmpty() || try_catch.HasCaught()) { if (maybe_result.IsEmpty() || try_catch.HasCaught()) {
success = false; success = false;
} }

4
shell/renderer/renderer_client_base.cc
View file

@ -608,8 +608,8 @@ void RendererClientBase::SetupMainWorldOverrides(
if (global.GetHidden("guestViewInternal", &guest_view_internal)) { if (global.GetHidden("guestViewInternal", &guest_view_internal)) {
api::context_bridge::ObjectCache object_cache; api::context_bridge::ObjectCache object_cache;
auto result = api::PassValueToOtherContext( auto result = api::PassValueToOtherContext(
source_context, context, guest_view_internal, &object_cache, false, 0, source_context, context, guest_view_internal, source_context->Global(),
api::BridgeErrorTarget::kSource); &object_cache, false, 0, api::BridgeErrorTarget::kSource);
if (!result.IsEmpty()) { if (!result.IsEmpty()) {
isolated_api.Set("guestViewInternal", result.ToLocalChecked()); isolated_api.Set("guestViewInternal", result.ToLocalChecked());
} }

31
spec/webview-spec.ts
View file

@ -2020,25 +2020,34 @@ describe('<webview> tag', function () {
// TODO(miniak): figure out why this is failing on windows // TODO(miniak): figure out why this is failing on windows
ifdescribe(process.platform !== 'win32')('<webview>.capturePage()', () => { ifdescribe(process.platform !== 'win32')('<webview>.capturePage()', () => {
it('returns a Promise with a NativeImage', async () => { it('returns a Promise with a NativeImage', async function () {
this.retries(5);
const src = 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E'; const src = 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E';
await loadWebViewAndWaitForEvent(w, { src }, 'did-stop-loading'); await loadWebViewAndWaitForEvent(w, { src }, 'did-stop-loading');
// Retry a few times due to flake.
for (let i = 0; i < 5; i++) {
try {
const image = await w.executeJavaScript('webview.capturePage()'); const image = await w.executeJavaScript('webview.capturePage()');
const imgBuffer = image.toPNG(); expect(image.isEmpty()).to.be.false();
// Check the 25th byte in the PNG. // Check the 25th byte in the PNG.
// Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha // Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha
const imgBuffer = image.toPNG();
expect(imgBuffer[25]).to.equal(6); expect(imgBuffer[25]).to.equal(6);
return; });
} catch {
/* drop the error */ it('returns a Promise with a NativeImage in the renderer', async function () {
} this.retries(5);
}
expect(false).to.be.true('could not successfully capture the page'); const src = 'data:text/html,%3Ch1%3EHello%2C%20World!%3C%2Fh1%3E';
await loadWebViewAndWaitForEvent(w, { src }, 'did-stop-loading');
const byte = await w.executeJavaScript(`new Promise(resolve => {
webview.capturePage().then(image => {
resolve(image.toPNG()[25])
});
})`);
expect(byte).to.equal(6);
}); });
}); });