Browse Source

refactor: clean up webFrame implementation to use gin wrappers (#28497)

* refactor: clean up webFrame implementation to use gin wrappers

The previous implementation of webFrame in the renderer process leaked
sub-frame contexts and global objects across the context boundaries thus
making it possible for apps to either maliciously or accidentally
violate the contextIsolation boundary.

This re-implementation binds all methods in native code directly to
content::RenderFrame instances instead of relying on JS to provide a
"window" with every method request.  This is much more consistent with
the rest of the Electron codebase and is substantially safer.

* chore: un-re-order for ease of review

* chore: pass isolate around instead of ErrorThrower

* chore: fix rebase typo

* chore: remove unused variables
Samuel Attard 4 years ago
parent
commit
6df2680cb6

+ 2 - 2
lib/renderer/api/context-bridge.ts

@@ -1,7 +1,7 @@
-const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');
+const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
 const binding = process._linkedBinding('electron_renderer_context_bridge');
 
-const contextIsolationEnabled = getWebPreference(window, 'contextIsolation');
+const contextIsolationEnabled = mainFrame.getWebPreference('contextIsolation');
 
 const checkContextIsolationEnabled = () => {
   if (!contextIsolationEnabled) throw new Error('contextBridge API can only be used when contextIsolation is enabled');

+ 2 - 68
lib/renderer/api/web-frame.ts

@@ -1,69 +1,3 @@
-import { EventEmitter } from 'events';
+const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
 
-const binding = process._linkedBinding('electron_renderer_web_frame');
-
-class WebFrame extends EventEmitter {
-  constructor (public context: Window) {
-    super();
-
-    // Lots of webview would subscribe to webFrame's events.
-    this.setMaxListeners(0);
-  }
-
-  findFrameByRoutingId (routingId: number) {
-    return getWebFrame(binding._findFrameByRoutingId(this.context, routingId));
-  }
-
-  getFrameForSelector (selector: string) {
-    return getWebFrame(binding._getFrameForSelector(this.context, selector));
-  }
-
-  findFrameByName (name: string) {
-    return getWebFrame(binding._findFrameByName(this.context, name));
-  }
-
-  get opener () {
-    return getWebFrame(binding._getOpener(this.context));
-  }
-
-  get parent () {
-    return getWebFrame(binding._getParent(this.context));
-  }
-
-  get top () {
-    return getWebFrame(binding._getTop(this.context));
-  }
-
-  get firstChild () {
-    return getWebFrame(binding._getFirstChild(this.context));
-  }
-
-  get nextSibling () {
-    return getWebFrame(binding._getNextSibling(this.context));
-  }
-
-  get routingId () {
-    return binding._getRoutingId(this.context);
-  }
-}
-
-// 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>) {
-      return (binding as any)[name](this.context, ...args);
-    };
-  }
-}
-
-// Helper to return WebFrame or null depending on context.
-// TODO(zcbenz): Consider returning same WebFrame for the same frame.
-function getWebFrame (context: Window) {
-  return context ? new WebFrame(context) : null;
-}
-
-const _webFrame = new WebFrame(window);
-
-export default _webFrame;
+export default mainFrame;

+ 12 - 12
lib/renderer/init.ts

@@ -63,18 +63,18 @@ webFrameInit();
 
 // Process command line arguments.
 const { hasSwitch, getSwitchValue } = process._linkedBinding('electron_common_command_line');
-const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');
-
-const contextIsolation = getWebPreference(window, 'contextIsolation');
-const nodeIntegration = getWebPreference(window, 'nodeIntegration');
-const webviewTag = getWebPreference(window, 'webviewTag');
-const isHiddenPage = getWebPreference(window, 'hiddenPage');
-const usesNativeWindowOpen = getWebPreference(window, 'nativeWindowOpen');
-const rendererProcessReuseEnabled = getWebPreference(window, 'disableElectronSiteInstanceOverrides');
-const preloadScript = getWebPreference(window, 'preload');
-const preloadScripts = getWebPreference(window, 'preloadScripts');
-const guestInstanceId = getWebPreference(window, 'guestInstanceId') || null;
-const openerId = getWebPreference(window, 'openerId') || null;
+const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
+
+const contextIsolation = mainFrame.getWebPreference('contextIsolation');
+const nodeIntegration = mainFrame.getWebPreference('nodeIntegration');
+const webviewTag = mainFrame.getWebPreference('webviewTag');
+const isHiddenPage = mainFrame.getWebPreference('hiddenPage');
+const usesNativeWindowOpen = mainFrame.getWebPreference('nativeWindowOpen');
+const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides');
+const preloadScript = mainFrame.getWebPreference('preload');
+const preloadScripts = mainFrame.getWebPreference('preloadScripts');
+const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null;
+const openerId = mainFrame.getWebPreference('openerId') || null;
 const appPath = hasSwitch('app-path') ? getSwitchValue('app-path') : null;
 
 // The webContents preload script is loaded after the session preload scripts.

+ 7 - 7
lib/sandboxed_renderer/init.ts

@@ -113,7 +113,7 @@ function preloadRequire (module: string) {
 
 // Process command line arguments.
 const { hasSwitch } = process._linkedBinding('electron_common_command_line');
-const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');
+const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
 
 // Similar to nodes --expose-internals flag, this exposes _linkedBinding so
 // that tests can call it to get access to some test only bindings
@@ -121,13 +121,13 @@ if (hasSwitch('unsafely-expose-electron-internals-for-testing')) {
   preloadProcess._linkedBinding = process._linkedBinding;
 }
 
-const contextIsolation = getWebPreference(window, 'contextIsolation');
-const webviewTag = getWebPreference(window, 'webviewTag');
-const isHiddenPage = getWebPreference(window, 'hiddenPage');
-const rendererProcessReuseEnabled = getWebPreference(window, 'disableElectronSiteInstanceOverrides');
+const contextIsolation = mainFrame.getWebPreference('contextIsolation');
+const webviewTag = mainFrame.getWebPreference('webviewTag');
+const isHiddenPage = mainFrame.getWebPreference('hiddenPage');
+const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides');
 const usesNativeWindowOpen = true;
-const guestInstanceId = getWebPreference(window, 'guestInstanceId') || null;
-const openerId = getWebPreference(window, 'openerId') || null;
+const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null;
+const openerId = mainFrame.getWebPreference('openerId') || null;
 
 switch (window.location.protocol) {
   case 'devtools:': {

+ 6 - 6
shell/common/gin_helper/error_thrower.cc

@@ -17,27 +17,27 @@ ErrorThrower::ErrorThrower() : isolate_(v8::Isolate::GetCurrent()) {}
 
 ErrorThrower::~ErrorThrower() = default;
 
-void ErrorThrower::ThrowError(base::StringPiece err_msg) {
+void ErrorThrower::ThrowError(base::StringPiece err_msg) const {
   Throw(v8::Exception::Error, err_msg);
 }
 
-void ErrorThrower::ThrowTypeError(base::StringPiece err_msg) {
+void ErrorThrower::ThrowTypeError(base::StringPiece err_msg) const {
   Throw(v8::Exception::TypeError, err_msg);
 }
 
-void ErrorThrower::ThrowRangeError(base::StringPiece err_msg) {
+void ErrorThrower::ThrowRangeError(base::StringPiece err_msg) const {
   Throw(v8::Exception::RangeError, err_msg);
 }
 
-void ErrorThrower::ThrowReferenceError(base::StringPiece err_msg) {
+void ErrorThrower::ThrowReferenceError(base::StringPiece err_msg) const {
   Throw(v8::Exception::ReferenceError, err_msg);
 }
 
-void ErrorThrower::ThrowSyntaxError(base::StringPiece err_msg) {
+void ErrorThrower::ThrowSyntaxError(base::StringPiece err_msg) const {
   Throw(v8::Exception::SyntaxError, err_msg);
 }
 
-void ErrorThrower::Throw(ErrorGenerator gen, base::StringPiece err_msg) {
+void ErrorThrower::Throw(ErrorGenerator gen, base::StringPiece err_msg) const {
   v8::Local<v8::Value> exception = gen(gin::StringToV8(isolate_, err_msg));
   if (!isolate_->IsExecutionTerminating())
     isolate_->ThrowException(exception);

+ 6 - 6
shell/common/gin_helper/error_thrower.h

@@ -17,18 +17,18 @@ class ErrorThrower {
 
   ~ErrorThrower();
 
-  void ThrowError(base::StringPiece err_msg);
-  void ThrowTypeError(base::StringPiece err_msg);
-  void ThrowRangeError(base::StringPiece err_msg);
-  void ThrowReferenceError(base::StringPiece err_msg);
-  void ThrowSyntaxError(base::StringPiece err_msg);
+  void ThrowError(base::StringPiece err_msg) const;
+  void ThrowTypeError(base::StringPiece err_msg) const;
+  void ThrowRangeError(base::StringPiece err_msg) const;
+  void ThrowReferenceError(base::StringPiece err_msg) const;
+  void ThrowSyntaxError(base::StringPiece err_msg) const;
 
   v8::Isolate* isolate() const { return isolate_; }
 
  private:
   using ErrorGenerator =
       v8::Local<v8::Value> (*)(v8::Local<v8::String> err_msg);
-  void Throw(ErrorGenerator gen, base::StringPiece err_msg);
+  void Throw(ErrorGenerator gen, base::StringPiece err_msg) const;
 
   v8::Isolate* isolate_;
 };

File diff suppressed because it is too large
+ 553 - 479
shell/renderer/api/electron_api_web_frame.cc


+ 5 - 10
typings/internal-ambient.d.ts

@@ -114,17 +114,12 @@ declare namespace NodeJS {
     webviewTag: boolean;
   }
 
+  interface InternalWebFrame extends Electron.WebFrame {
+    getWebPreference<K extends keyof InternalWebPreferences>(name: K): InternalWebPreferences[K];
+  }
+
   interface WebFrameBinding {
-    _findFrameByRoutingId(window: Window, routingId: number): Window;
-    _getFrameForSelector(window: Window, selector: string): Window;
-    _findFrameByName(window: Window, name: string): Window;
-    _getOpener(window: Window): Window;
-    _getParent(window: Window): Window;
-    _getTop(window: Window): Window;
-    _getFirstChild(window: Window): Window;
-    _getNextSibling(window: Window): Window;
-    _getRoutingId(window: Window): number;
-    getWebPreference<K extends keyof InternalWebPreferences>(window: Window, name: K): InternalWebPreferences[K];
+    mainFrame: InternalWebFrame;
   }
 
   type DataPipe = {

Some files were not shown because too many files changed in this diff