fix: crash on invalid zoomFactor (#22673)

electron · Mar 16, 2020 · 0c1fa0b · 0c1fa0b
1 parent bcb1d52
commit 0c1fa0b
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 6 deletions.
diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md
@@ -1068,11 +1068,13 @@ Returns `Boolean` - Whether audio is currently playing.
 
 #### `contents.setZoomFactor(factor)`
 
-* `factor` Number - Zoom factor.
+* `factor` Double - Zoom factor; default is 1.0.
 
 Changes the zoom factor to the specified factor. Zoom factor is
 zoom percent divided by 100, so 300% = 3.0.
 
+The factor must be greater than 0.0.
+
 **[Deprecated](modernization/property-updates.md)**
 
 #### `contents.getZoomFactor()`

diff --git a/docs/api/web-frame.md b/docs/api/web-frame.md
@@ -22,11 +22,13 @@ The `WebFrame` class has the following instance methods:
 
 ### `webFrame.setZoomFactor(factor)`
 
-* `factor` Number - Zoom factor.
+* `factor` Double - Zoom factor; default is 1.0.
 
 Changes the zoom factor to the specified factor. Zoom factor is
 zoom percent divided by 100, so 300% = 3.0.
 
+The factor must be greater than 0.0.
+
 ### `webFrame.getZoomFactor()`
 
 Returns `Number` - The current zoom factor.

diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
@@ -4,6 +4,7 @@
 
 #include "shell/browser/api/electron_api_web_contents.h"
 
+#include <limits>
 #include <memory>
 #include <set>
 #include <string>
@@ -2422,7 +2423,13 @@ double WebContents::GetZoomLevel() const {
   return zoom_controller_->GetZoomLevel();
 }
 
-void WebContents::SetZoomFactor(double factor) {
+void WebContents::SetZoomFactor(gin_helper::ErrorThrower thrower,
+                                double factor) {
+  if (factor < std::numeric_limits<double>::epsilon()) {
+    thrower.ThrowError("'zoomFactor' must be a double greater than 0.0");
+    return;
+  }
+
   auto level = blink::PageZoomFactorToZoomLevel(factor);
   SetZoomLevel(level);
 }

diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h
@@ -290,7 +290,7 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
   // Methods for zoom handling.
   void SetZoomLevel(double level);
   double GetZoomLevel() const;
-  void SetZoomFactor(double factor);
+  void SetZoomFactor(gin_helper::ErrorThrower thrower, double factor);
   double GetZoomFactor() const;
 
   // Callback triggered on permission response.

diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
+#include <limits>
 #include <memory>
 #include <string>
 #include <utility>
@@ -275,7 +276,14 @@ double GetZoomLevel(v8::Local<v8::Value> window) {
   return result;
 }
 
-void SetZoomFactor(v8::Local<v8::Value> window, double factor) {
+void SetZoomFactor(gin_helper::ErrorThrower thrower,
+                   v8::Local<v8::Value> window,
+                   double factor) {
+  if (factor < std::numeric_limits<double>::epsilon()) {
+    thrower.ThrowError("'zoomFactor' must be a double greater than 0.0");
+    return;
+  }
+
   SetZoomLevel(window, blink::PageZoomFactorToZoomLevel(factor));
 }
 

diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts
@@ -800,7 +800,19 @@ describe('webContents module', () => {
 
     afterEach(closeAllWindows)
 
-    // TODO(codebytere): remove in Electron v8.0.0
+    it('throws on an invalid zoomFactor', async () => {
+      const w = new BrowserWindow({ show: false })
+      await w.loadURL('about:blank')
+
+      expect(() => {
+        w.webContents.setZoomFactor(0.0)
+      }).to.throw(/'zoomFactor' must be a double greater than 0.0/)
+
+      expect(() => {
+        w.webContents.setZoomFactor(-2.0)
+      }).to.throw(/'zoomFactor' must be a double greater than 0.0/)
+    })
+
     it('can set the correct zoom level (functions)', async () => {
       const w = new BrowserWindow({ show: false })
       try {