Browse Source

chore: remove deprecated worldSafeExecuteJavaScript option (#28456)

Milan Burda 4 years ago
parent
commit
da8c35e3b2

+ 0 - 2
docs/api/browser-window.md

@@ -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

+ 0 - 15
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');
 
@@ -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);
-      };
-    }
   }
 }
 

+ 1 - 2
lib/renderer/security-warnings.ts

@@ -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 {

+ 0 - 5
lib/renderer/web-frame-init.ts

@@ -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.

+ 10 - 18
patches/chromium/allow_in_process_windows_to_have_different_web_prefs.patch

@@ -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),
@@ -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),
@@ -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,
@@ -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();
@@ -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();
@@ -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 @@
@@ -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;
  
@@ -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;
@@ -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 @@
@@ -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;
    }
  
@@ -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;
 +  }
@@ -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";
@@ -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;
  
@@ -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;

+ 0 - 4
shell/browser/web_contents_preferences.cc

@@ -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);
@@ -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;

+ 0 - 3
shell/common/options_switches.cc

@@ -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";
 

+ 0 - 1
shell/common/options_switches.h

@@ -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[];

+ 1 - 14
shell/renderer/api/electron_api_web_frame.cc

@@ -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;
 
@@ -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()) &&
@@ -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);
@@ -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);
@@ -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;
@@ -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;

+ 14 - 18
spec-main/api-web-frame-spec.ts

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

+ 0 - 1
typings/internal-ambient.d.ts

@@ -112,7 +112,6 @@ declare namespace NodeJS {
     preload: string
     preloadScripts: string[];
     webviewTag: boolean;
-    worldSafeExecuteJavaScript: boolean;
   }
 
   interface WebFrameBinding {

+ 0 - 1
typings/internal-electron.d.ts

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