Merge pull request #3594 from thc202/webdrivers-quit

Quit all browsers when ZAP shuts down
zaproxy · Feb 17, 2022 · 4471601 · 4471601
2 parents 7569db3 + b502729
commit 4471601
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 35 deletions.
diff --git a/addOns/domxss/CHANGELOG.md b/addOns/domxss/CHANGELOG.md
@@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Update minimum ZAP version to 2.11.1.
 - Use Network add-on to proxy browser requests.
 
+### Fixed
+- Stop the proxy when ZAP shuts down.
+
 ## [12] - 2021-12-06
 ### Changed
 - Dependency updates.

diff --git a/addOns/domxss/src/main/java/org/zaproxy/zap/extension/domxss/ExtensionDomXSS.java b/addOns/domxss/src/main/java/org/zaproxy/zap/extension/domxss/ExtensionDomXSS.java
@@ -79,11 +79,7 @@ public void hook(ExtensionHook extensionHook) {
     }
 
     @Override
-    public void unload() {
-        super.unload();
-
-        PluginFactory.unloadedPlugin(scanner);
-
+    public void stop() {
         Server proxy = DomXssScanRule.proxy;
         if (proxy != null) {
             try {
@@ -94,6 +90,13 @@ public void unload() {
         }
     }
 
+    @Override
+    public void unload() {
+        super.unload();
+
+        PluginFactory.unloadedPlugin(scanner);
+    }
+
     @Override
     public String getName() {
         return "ExtensionDomXSS";

diff --git a/addOns/selenium/CHANGELOG.md b/addOns/selenium/CHANGELOG.md
@@ -5,7 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
-
+### Fixed
+- Quit all browsers when ZAP shuts down ([Issue #6643](https://github.com/zaproxy/zaproxy/issues/6643)).
 
 ## [15.6.0] - 2021-12-13
 ### Changed

diff --git a/addOns/selenium/src/main/java/org/zaproxy/zap/extension/selenium/ExtensionSelenium.java b/addOns/selenium/src/main/java/org/zaproxy/zap/extension/selenium/ExtensionSelenium.java
@@ -112,10 +112,10 @@ public class ExtensionSelenium extends ExtensionAdaptor {
     private List<ProvidedBrowserUI> providedBrowserUIList;
 
     /**
-     * A list containing all of the proxied WebDrivers opened, so that they can be closed when ZAP
-     * is closed.
+     * A list containing all of the WebDrivers opened, so that they can be closed when ZAP is
+     * closed.
      */
-    private Map<String, List<WebDriver>> proxiedWebDrivers = new HashMap<>();
+    private static List<WebDriver> webDrivers = new ArrayList<>();
 
     private List<WeakReference<ProvidedBrowsersComboBoxModel>> providedBrowserComboBoxModels =
             new ArrayList<>();
@@ -237,24 +237,15 @@ public void unload() {
     }
 
     @Override
-    public void stop() {
-        super.stop();
-        this.proxiedWebDrivers.values().forEach(ExtensionSelenium::quitWebDrivers);
-        this.proxiedWebDrivers.clear();
-    }
-
-    private static void quitWebDrivers(List<WebDriver> drivers) {
-        if (drivers == null || drivers.isEmpty()) {
-            return;
-        }
-
-        for (WebDriver wd : drivers) {
+    public void destroy() {
+        for (WebDriver wd : webDrivers) {
             try {
                 wd.quit();
             } catch (Exception ex) {
                 // Ignore - the user might well have already closed the browser
             }
         }
+        webDrivers.clear();
     }
 
     /**
@@ -336,7 +327,6 @@ private static void validateWebDriverProvider(SingleWebDriverProvider webDriverP
     public void removeWebDriverProvider(SingleWebDriverProvider webDriverProvider) {
         validateWebDriverProvider(webDriverProvider);
 
-        quitWebDrivers(proxiedWebDrivers.remove(webDriverProvider.getId()));
         webDriverProviders.remove(webDriverProvider.getId());
         providedBrowsers.remove(webDriverProvider.getProvidedBrowser().getId());
         buildProvidedBrowserUIList();
@@ -714,13 +704,8 @@ public WebDriver getProxiedBrowser(
                         Model.getSingleton().getOptionsParam().getProxyParam().getProxyPort(),
                         enableExtensions);
 
-        if (webDriver != null) {
-            proxiedWebDrivers
-                    .computeIfAbsent(providedBrowserId, k -> new ArrayList<>())
-                    .add(webDriver);
-            if (url != null) {
-                webDriver.get(url);
-            }
+        if (webDriver != null && url != null) {
+            webDriver.get(url);
         }
         return webDriver;
     }
@@ -865,8 +850,11 @@ public static WebDriver getWebDriver(
             boolean enableExtensions) {
         validateProxyAddressPort(proxyAddress, proxyPort);
 
-        return getWebDriverImpl(
-                requester, browser, proxyAddress, proxyPort, c -> {}, enableExtensions);
+        WebDriver wd =
+                getWebDriverImpl(
+                        requester, browser, proxyAddress, proxyPort, c -> {}, enableExtensions);
+        webDrivers.add(wd);
+        return wd;
     }
 
     public static WebDriver getWebDriver(
@@ -887,7 +875,10 @@ public static WebDriver getWebDriver(
             boolean enableExtensions) {
         validateProxyAddressPort(proxyAddress, proxyPort);
 
-        return getWebDriverImpl(-1, browser, proxyAddress, proxyPort, consumer, enableExtensions);
+        WebDriver wd =
+                getWebDriverImpl(-1, browser, proxyAddress, proxyPort, consumer, enableExtensions);
+        webDrivers.add(wd);
+        return wd;
     }
 
     private static void setCommonOptions(

diff --git a/addOns/spiderAjax/CHANGELOG.md b/addOns/spiderAjax/CHANGELOG.md
@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Use Network add-on to proxy Crawljax/browser requests.
 - Maintenance changes.
 
+### Fixed
+- Stop the spider scans when ZAP shuts down ([Issue #6643](https://github.com/zaproxy/zaproxy/issues/6643)).
+
 ## [23.7.0] - 2021-11-02
 ### Added
 - Automation authentication support

diff --git a/addOns/spiderAjax/src/main/java/org/zaproxy/zap/extension/spiderAjax/AjaxSpiderAPI.java b/addOns/spiderAjax/src/main/java/org/zaproxy/zap/extension/spiderAjax/AjaxSpiderAPI.java
@@ -468,7 +468,7 @@ private boolean isSpiderRunning() {
         return (extension.isSpiderRunning() && spiderThread != null);
     }
 
-    private void stopSpider() {
+    void stopSpider() {
         if (isSpiderRunning()) {
             spiderThread.stopSpider();
             spiderThread = null;

diff --git a/addOns/spiderAjax/src/main/java/org/zaproxy/zap/extension/spiderAjax/ExtensionAjax.java b/addOns/spiderAjax/src/main/java/org/zaproxy/zap/extension/spiderAjax/ExtensionAjax.java
@@ -129,8 +129,7 @@ public boolean canUnload() {
 
     @Override
     public void unload() {
-        if (getView() != null) {
-            getSpiderPanel().stopScan();
+        if (hasView()) {
             getSpiderPanel().unload();
 
             SpiderEventPublisher.unregisterPublisher();
@@ -145,6 +144,14 @@ public void unload() {
         super.unload();
     }
 
+    @Override
+    public void stop() {
+        if (hasView()) {
+            stopScan();
+        }
+        ajaxSpiderApi.stopSpider();
+    }
+
     @Override
     public List<String> getActiveActions() {
         if (isSpiderRunning()) {