Skip to content

Commit

Permalink
chore: remove deprecated worldSafeExecuteJavaScript option
Browse files Browse the repository at this point in the history
  • Loading branch information
miniak committed Apr 3, 2021
1 parent 55c66e3 commit b7d2a73
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 79 deletions.
2 changes: 0 additions & 2 deletions docs/api/browser-window.md
Expand Up @@ -348,8 +348,6 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
[Chrome Content Scripts][chrome-content-scripts]. You can access this
context in the dev tools by selecting the 'Electron Isolated Context'
entry in the combo box at the top of the Console tab.
* `worldSafeExecuteJavaScript` Boolean (optional) - If true, values returned from `webFrame.executeJavaScript` will be sanitized to ensure JS values
can't unsafely cross between worlds when using `contextIsolation`. Defaults to `true`. _Deprecated_
* `nativeWindowOpen` Boolean (optional) - Whether to use native
`window.open()`. Defaults to `false`. Child windows will always have node
integration disabled unless `nodeIntegrationInSubFrames` is true. **Note:** This option is currently
Expand Down
15 changes: 0 additions & 15 deletions lib/renderer/api/web-frame.ts
@@ -1,5 +1,4 @@
import { EventEmitter } from 'events';
import deprecate from '@electron/internal/common/api/deprecate';

const binding = process._linkedBinding('electron_renderer_web_frame');

Expand Down Expand Up @@ -48,28 +47,14 @@ class WebFrame extends EventEmitter {
}
}

const contextIsolation = binding.getWebPreference(window, 'contextIsolation');
const worldSafeExecuteJavaScript = binding.getWebPreference(window, 'worldSafeExecuteJavaScript');

const worldSafeJS = worldSafeExecuteJavaScript || !contextIsolation;

// Populate the methods.
for (const name in binding) {
if (!name.startsWith('_')) { // some methods are manually populated above
// TODO(felixrieseberg): Once we can type web_frame natives, we could
// use a neat `keyof` here
(WebFrame as any).prototype[name] = function (...args: Array<any>) {
if (!worldSafeJS && name.startsWith('executeJavaScript')) {
deprecate.log(`Security Warning: webFrame.${name} was called without worldSafeExecuteJavaScript enabled. This is considered unsafe. worldSafeExecuteJavaScript will be enabled by default in Electron 12.`);
}
return (binding as any)[name](this.context, ...args);
};
// TODO(MarshallOfSound): Remove once the above deprecation is removed
if (name.startsWith('executeJavaScript')) {
(WebFrame as any).prototype[`_${name}`] = function (...args: Array<any>) {
return (binding as any)[name](this.context, ...args);
};
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions lib/renderer/security-warnings.ts
Expand Up @@ -78,8 +78,7 @@ const isLocalhost = function () {
* @returns {boolean} Is a CSP with `unsafe-eval` set?
*/
const isUnsafeEvalEnabled: () => Promise<boolean> = function () {
// Call _executeJavaScript to bypass the world-safe deprecation warning
return webFrame._executeJavaScript(`(${(() => {
return webFrame.executeJavaScript(`(${(() => {
try {
eval(window.trustedTypes.emptyScript); // eslint-disable-line no-eval
} catch {
Expand Down
Expand Up @@ -8,10 +8,10 @@ WebPreferences of in-process child windows, rather than relying on
process-level command line switches, as before.

diff --git a/third_party/blink/common/web_preferences/web_preferences.cc b/third_party/blink/common/web_preferences/web_preferences.cc
index 758b0b1616ecf86b7dd090adce94395851d9baf2..43eed39329d5d4337471a2ae8512714d6c6cb841 100644
index 758b0b1616ecf86b7dd090adce94395851d9baf2..cb5625e4a3363be85bbe83686f3aa1b07306f5a0 100644
--- a/third_party/blink/common/web_preferences/web_preferences.cc
+++ b/third_party/blink/common/web_preferences/web_preferences.cc
@@ -146,6 +146,28 @@ WebPreferences::WebPreferences()
@@ -146,6 +146,27 @@ WebPreferences::WebPreferences()
navigate_on_drag_drop(true),
v8_cache_options(blink::mojom::V8CacheOptions::kDefault),
record_whole_document(false),
Expand All @@ -21,7 +21,6 @@ index 758b0b1616ecf86b7dd090adce94395851d9baf2..43eed39329d5d4337471a2ae8512714d
+ background_color(base::EmptyString()),
+ opener_id(0),
+ context_isolation(false),
+ world_safe_execute_javascript(false),
+ guest_instance_id(0),
+ hidden_page(false),
+ offscreen(false),
Expand All @@ -41,7 +40,7 @@ index 758b0b1616ecf86b7dd090adce94395851d9baf2..43eed39329d5d4337471a2ae8512714d
accelerated_video_decode_enabled(false),
animation_policy(
diff --git a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc
index ba1ba323ec45296c33b5931652a001d6bd24dbe0..178cae9c389e48733fde982f4906d9748004dbe3 100644
index ba1ba323ec45296c33b5931652a001d6bd24dbe0..7d644150a1733bd0bca1c6bb63c759641ba091e8 100644
--- a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc
+++ b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc
@@ -24,6 +24,11 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
Expand All @@ -56,7 +55,7 @@ index ba1ba323ec45296c33b5931652a001d6bd24dbe0..178cae9c389e48733fde982f4906d974
!data.ReadLazyFrameLoadingDistanceThresholdsPx(
&out->lazy_frame_loading_distance_thresholds_px) ||
!data.ReadLazyImageLoadingDistanceThresholdsPx(
@@ -152,6 +157,26 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
@@ -152,6 +157,25 @@ bool StructTraits<blink::mojom::WebPreferencesDataView,
out->navigate_on_drag_drop = data.navigate_on_drag_drop();
out->v8_cache_options = data.v8_cache_options();
out->record_whole_document = data.record_whole_document();
Expand All @@ -65,7 +64,6 @@ index ba1ba323ec45296c33b5931652a001d6bd24dbe0..178cae9c389e48733fde982f4906d974
+ out->disable_electron_site_instance_overrides = data.disable_electron_site_instance_overrides();
+ out->opener_id = data.opener_id();
+ out->context_isolation = data.context_isolation();
+ out->world_safe_execute_javascript = data.world_safe_execute_javascript();
+ out->guest_instance_id = data.guest_instance_id();
+ out->hidden_page = data.hidden_page();
+ out->offscreen = data.offscreen();
Expand All @@ -84,7 +82,7 @@ index ba1ba323ec45296c33b5931652a001d6bd24dbe0..178cae9c389e48733fde982f4906d974
out->accelerated_video_decode_enabled =
data.accelerated_video_decode_enabled();
diff --git a/third_party/blink/public/common/web_preferences/web_preferences.h b/third_party/blink/public/common/web_preferences/web_preferences.h
index 74ed4b91cdd4c0cc0244491dfbbdf8e69e54e6f5..6795a5307ff49bbe366041e28c54dd2d9976c7f8 100644
index 74ed4b91cdd4c0cc0244491dfbbdf8e69e54e6f5..de7c24ff8c780b05f7af613f6303705ec6cb2bf6 100644
--- a/third_party/blink/public/common/web_preferences/web_preferences.h
+++ b/third_party/blink/public/common/web_preferences/web_preferences.h
@@ -9,6 +9,7 @@
Expand All @@ -95,7 +93,7 @@ index 74ed4b91cdd4c0cc0244491dfbbdf8e69e54e6f5..6795a5307ff49bbe366041e28c54dd2d
#include "base/time/time.h"
#include "build/build_config.h"
#include "net/nqe/effective_connection_type.h"
@@ -160,6 +161,28 @@ struct BLINK_COMMON_EXPORT WebPreferences {
@@ -160,6 +161,27 @@ struct BLINK_COMMON_EXPORT WebPreferences {
blink::mojom::V8CacheOptions v8_cache_options;
bool record_whole_document;

Expand All @@ -105,7 +103,6 @@ index 74ed4b91cdd4c0cc0244491dfbbdf8e69e54e6f5..6795a5307ff49bbe366041e28c54dd2d
+ std::string background_color;
+ int opener_id;
+ bool context_isolation;
+ bool world_safe_execute_javascript;
+ int guest_instance_id;
+ bool hidden_page;
+ bool offscreen;
Expand All @@ -125,7 +122,7 @@ index 74ed4b91cdd4c0cc0244491dfbbdf8e69e54e6f5..6795a5307ff49bbe366041e28c54dd2d
// only controls whether or not the "document.cookie" field is properly
// connected to the backing store, for instance if you wanted to be able to
diff --git a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h
index ae180b30284c17c7319925531440161f66b873c7..6ba055814a8385052d7798be56de53691dbe3343 100644
index ae180b30284c17c7319925531440161f66b873c7..18c55d24e40e2fee59ac3b4111d0c5ebb2661cad 100644
--- a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h
+++ b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h
@@ -6,6 +6,7 @@
Expand All @@ -136,7 +133,7 @@ index ae180b30284c17c7319925531440161f66b873c7..6ba055814a8385052d7798be56de5369
#include "mojo/public/cpp/bindings/struct_traits.h"
#include "net/nqe/effective_connection_type.h"
#include "third_party/blink/public/common/common_export.h"
@@ -441,6 +442,84 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::WebPreferencesDataView,
@@ -441,6 +442,80 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::WebPreferencesDataView,
return r.record_whole_document;
}

Expand All @@ -161,10 +158,6 @@ index ae180b30284c17c7319925531440161f66b873c7..6ba055814a8385052d7798be56de5369
+ return r.context_isolation;
+ }
+
+ static bool world_safe_execute_javascript(const blink::web_pref::WebPreferences& r) {
+ return r.world_safe_execute_javascript;
+ }
+
+ static int guest_instance_id(const blink::web_pref::WebPreferences& r) {
+ return r.guest_instance_id;
+ }
Expand Down Expand Up @@ -222,7 +215,7 @@ index ae180b30284c17c7319925531440161f66b873c7..6ba055814a8385052d7798be56de5369
return r.cookie_enabled;
}
diff --git a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
index 5428fa6e79ed60774fcd6e87dcd6a602143158b7..3f86e539fb4c70c690286f9eecf8d60bd23939af 100644
index 5428fa6e79ed60774fcd6e87dcd6a602143158b7..7decbc41450c7e8b4536eb8c3c087676a38f912c 100644
--- a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
+++ b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom
@@ -9,6 +9,7 @@ import "third_party/blink/public/mojom/css/preferred_contrast.mojom";
Expand All @@ -233,7 +226,7 @@ index 5428fa6e79ed60774fcd6e87dcd6a602143158b7..3f86e539fb4c70c690286f9eecf8d60b

enum PointerType {
kPointerNone = 1, // 1 << 0
@@ -211,6 +212,28 @@ struct WebPreferences {
@@ -211,6 +212,27 @@ struct WebPreferences {
V8CacheOptions v8_cache_options;
bool record_whole_document;

Expand All @@ -243,7 +236,6 @@ index 5428fa6e79ed60774fcd6e87dcd6a602143158b7..3f86e539fb4c70c690286f9eecf8d60b
+ string background_color;
+ int32 opener_id;
+ bool context_isolation;
+ bool world_safe_execute_javascript;
+ int32 guest_instance_id;
+ bool hidden_page;
+ bool offscreen;
Expand Down
4 changes: 0 additions & 4 deletions shell/browser/web_contents_preferences.cc
Expand Up @@ -143,7 +143,6 @@ WebContentsPreferences::WebContentsPreferences(
SetDefaultBoolIfUndefined(options::kSandbox, false);
SetDefaultBoolIfUndefined(options::kNativeWindowOpen, false);
SetDefaultBoolIfUndefined(options::kContextIsolation, true);
SetDefaultBoolIfUndefined(options::kWorldSafeExecuteJavaScript, true);
SetDefaultBoolIfUndefined(options::kJavaScript, true);
SetDefaultBoolIfUndefined(options::kImages, true);
SetDefaultBoolIfUndefined(options::kTextAreasAreResizable, true);
Expand Down Expand Up @@ -443,9 +442,6 @@ void WebContentsPreferences::OverrideWebkitPrefs(
// Run Electron APIs and preload script in isolated world
prefs->context_isolation = IsEnabled(options::kContextIsolation, true);

prefs->world_safe_execute_javascript =
IsEnabled(options::kWorldSafeExecuteJavaScript, true);

int guest_instance_id = 0;
if (GetAsInteger(&preference_, options::kGuestInstanceID, &guest_instance_id))
prefs->guest_instance_id = guest_instance_id;
Expand Down
3 changes: 0 additions & 3 deletions shell/common/options_switches.cc
Expand Up @@ -121,9 +121,6 @@ const char kNodeIntegration[] = "nodeIntegration";
// Enable context isolation of Electron APIs and preload script
const char kContextIsolation[] = "contextIsolation";

// Enable world safe passing of values when using "executeJavaScript"
const char kWorldSafeExecuteJavaScript[] = "worldSafeExecuteJavaScript";

// Instance ID of guest WebContents.
const char kGuestInstanceID[] = "guestInstanceId";

Expand Down
1 change: 0 additions & 1 deletion shell/common/options_switches.h
Expand Up @@ -65,7 +65,6 @@ extern const char kPreloadScripts[];
extern const char kPreloadURL[];
extern const char kNodeIntegration[];
extern const char kContextIsolation[];
extern const char kWorldSafeExecuteJavaScript[];
extern const char kGuestInstanceID[];
extern const char kExperimentalFeatures[];
extern const char kOpenerID[];
Expand Down
15 changes: 1 addition & 14 deletions shell/renderer/api/electron_api_web_frame.cc
Expand Up @@ -152,11 +152,8 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {

explicit ScriptExecutionCallback(
gin_helper::Promise<v8::Local<v8::Value>> promise,
bool world_safe_result,
CompletionCallback callback)
: promise_(std::move(promise)),
world_safe_result_(world_safe_result),
callback_(std::move(callback)) {}
: promise_(std::move(promise)), callback_(std::move(callback)) {}

~ScriptExecutionCallback() override = default;

Expand Down Expand Up @@ -213,7 +210,6 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
// the same world as the caller or the result is not an object and
// therefore does not have a prototype chain to protect
bool should_clone_value =
world_safe_result_ &&
!(value->IsObject() &&
promise_.GetContext() ==
value.As<v8::Object>()->CreationContext()) &&
Expand Down Expand Up @@ -261,7 +257,6 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {

private:
gin_helper::Promise<v8::Local<v8::Value>> promise_;
bool world_safe_result_;
CompletionCallback callback_;

DISALLOW_COPY_AND_ASSIGN(ScriptExecutionCallback);
Expand Down Expand Up @@ -427,8 +422,6 @@ v8::Local<v8::Value> GetWebPreference(v8::Isolate* isolate,
return gin::ConvertToV8(isolate, prefs.opener_id);
} else if (pref_name == options::kContextIsolation) {
return gin::ConvertToV8(isolate, prefs.context_isolation);
} else if (pref_name == options::kWorldSafeExecuteJavaScript) {
return gin::ConvertToV8(isolate, prefs.world_safe_execute_javascript);
} else if (pref_name == options::kGuestInstanceID) {
// NOTE: guestInstanceId is internal-only.
return gin::ConvertToV8(isolate, prefs.guest_instance_id);
Expand Down Expand Up @@ -643,13 +636,10 @@ v8::Local<v8::Promise> ExecuteJavaScript(gin_helper::Arguments* args,
ScriptExecutionCallback::CompletionCallback completion_callback;
args->GetNext(&completion_callback);

auto& prefs = render_frame->GetBlinkPreferences();

render_frame->GetWebFrame()->RequestExecuteScriptAndReturnValue(
blink::WebScriptSource(blink::WebString::FromUTF16(code)),
has_user_gesture,
new ScriptExecutionCallback(std::move(promise),
prefs.world_safe_execute_javascript,
std::move(completion_callback)));

return handle;
Expand Down Expand Up @@ -709,13 +699,10 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
blink::WebURL(GURL(url)), start_line));
}

auto& prefs = render_frame->GetBlinkPreferences();

render_frame->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
world_id, &sources.front(), sources.size(), has_user_gesture,
scriptExecutionType,
new ScriptExecutionCallback(std::move(promise),
prefs.world_safe_execute_javascript,
std::move(completion_callback)));

return handle;
Expand Down
32 changes: 14 additions & 18 deletions spec-main/api-web-frame-spec.ts
Expand Up @@ -9,31 +9,27 @@ describe('webFrame module', () => {

afterEach(closeAllWindows);

for (const worldSafe of [true, false]) {
it(`can use executeJavaScript with world safe mode ${worldSafe ? 'enabled' : 'disabled'}`, async () => {
const w = new BrowserWindow({
show: true,
webPreferences: {
nodeIntegration: true,
contextIsolation: true,
worldSafeExecuteJavaScript: worldSafe,
preload: path.join(fixtures, 'pages', 'world-safe-preload.js')
}
});
const isSafe = emittedOnce(ipcMain, 'executejs-safe');
w.loadURL('about:blank');
const [, wasSafe] = await isSafe;
expect(wasSafe).to.equal(worldSafe);
it('can use executeJavaScript', async () => {
const w = new BrowserWindow({
show: true,
webPreferences: {
nodeIntegration: true,
contextIsolation: true,
preload: path.join(fixtures, 'pages', 'world-safe-preload.js')
}
});
}
const isSafe = emittedOnce(ipcMain, 'executejs-safe');
w.loadURL('about:blank');
const [, wasSafe] = await isSafe;
expect(wasSafe).to.equal(true);
});

it('can use executeJavaScript with world safe mode enabled and catch conversion errors', async () => {
it('can use executeJavaScript and catch conversion errors', async () => {
const w = new BrowserWindow({
show: true,
webPreferences: {
nodeIntegration: true,
contextIsolation: true,
worldSafeExecuteJavaScript: true,
preload: path.join(fixtures, 'pages', 'world-safe-preload-error.js')
}
});
Expand Down
1 change: 0 additions & 1 deletion typings/internal-ambient.d.ts
Expand Up @@ -112,7 +112,6 @@ declare namespace NodeJS {
preload: string
preloadScripts: string[];
webviewTag: boolean;
worldSafeExecuteJavaScript: boolean;
}

interface WebFrameBinding {
Expand Down
1 change: 0 additions & 1 deletion typings/internal-electron.d.ts
Expand Up @@ -92,7 +92,6 @@ declare namespace Electron {
}

interface WebFrame {
_executeJavaScript(code: string, userGesture?: boolean): Promise<any>;
getWebFrameId(window: Window): number;
allowGuestViewElementDefinition(window: Window, context: any): void;
}
Expand Down

0 comments on commit b7d2a73

Please sign in to comment.