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

chore: remove deprecated worldSafeExecuteJavaScript option #28456

Merged
merged 1 commit into from Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
5 changes: 0 additions & 5 deletions lib/renderer/web-frame-init.ts
Expand Up @@ -13,11 +13,6 @@ export const webFrameInit = () => {
ipcRendererUtils.handle(IPC_MESSAGES.RENDERER_WEB_FRAME_METHOD, (
event, method: keyof WebFrameMethod, ...args: any[]
) => {
// TODO(MarshallOfSound): Remove once the world-safe-execute-javascript deprecation warning is removed
if (method.startsWith('executeJavaScript')) {
return (webFrame as any)[`_${method}`](...args);
}

// The TypeScript compiler cannot handle the sheer number of
// call signatures here and simply gives up. Incorrect invocations
// will be caught by "keyof WebFrameMethod" though.
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