Merge pull request #6676 from thc202/client/pscan-tree-href

client: use Sites tree for history references

Author: psiinon

addOns/client/CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Changed
 - Updated Chrome and Firefox extensions to v0.1.5.
+- Reduce warnings when passive scanning.
 
 ### Fixed
 - Error logs to always include stack trace.

addOns/client/src/main/java/org/zaproxy/addon/client/pscan/ClientPassiveScanHelper.java

@@ -22,20 +22,22 @@
 import java.awt.event.KeyEvent;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.URIException;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.parosproxy.paros.core.scanner.Alert;
 import org.parosproxy.paros.extension.history.ExtensionHistory;
 import org.parosproxy.paros.model.HistoryReference;
+import org.parosproxy.paros.model.SiteNode;
 import org.zaproxy.addon.client.ClientUtils;
 import org.zaproxy.zap.extension.alert.ExtensionAlert;
 import org.zaproxy.zap.utils.Stats;
 
 public class ClientPassiveScanHelper {
 
     private static final Logger LOGGER = LogManager.getLogger(ClientPassiveScanHelper.class);
-    private static final int MAX_HREFS_TO_CHECK = 1000;
     private ExtensionAlert extAlert;
     private ExtensionHistory extHistory;
 
@@ -46,21 +48,20 @@ public ClientPassiveScanHelper(ExtensionAlert extAlert, ExtensionHistory extHist
 
     public HistoryReference findHistoryRef(String url) {
         url = ClientUtils.stripUrlFragment(url);
-        int lastId = extHistory.getLastHistoryId();
 
-        // We don't expect to have to go too far back..
-        int limit = Math.max(lastId - MAX_HREFS_TO_CHECK, 0);
-        LOGGER.debug("Searching for history reference for {}", url);
-        for (int i = lastId; i >= limit; i--) {
-            HistoryReference hr = extHistory.getHistoryReference(i);
-            if (hr != null && url.equals(hr.getURI().toString())) {
-                LOGGER.debug("Found history reference {} for {}", hr.getHistoryId(), url);
+        try {
+            SiteNode node =
+                    extHistory.getModel().getSession().getSiteTree().findNode(new URI(url, true));
+            if (node != null) {
+                HistoryReference hr = node.getHistoryReference();
                 Stats.incCounter("stats.client.pscan.href.found");
+                LOGGER.debug("Found history reference {} for {}", hr.getHistoryId(), url);
                 return hr;
             }
+        } catch (URIException e) {
+            LOGGER.warn("Failed to create URI from: {} Cause: {}", url, e.getMessage());
         }
-        // Include the limit in case we change it in the future
-        Stats.incCounter("stats.client.pscan.href.missing." + MAX_HREFS_TO_CHECK);
+        Stats.incCounter("stats.client.pscan.href.missing");
         return null;
     }
 

addOns/client/src/test/java/org/zaproxy/addon/client/pscan/ClientPassiveScanHelperUnitTest.java

@@ -23,7 +23,6 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
-import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.withSettings;
@@ -40,6 +39,10 @@
 import org.mockito.quality.Strictness;
 import org.parosproxy.paros.extension.history.ExtensionHistory;
 import org.parosproxy.paros.model.HistoryReference;
+import org.parosproxy.paros.model.Model;
+import org.parosproxy.paros.model.Session;
+import org.parosproxy.paros.model.SiteMap;
+import org.parosproxy.paros.model.SiteNode;
 import org.zaproxy.addon.client.ExtensionClientIntegration;
 import org.zaproxy.zap.extension.alert.ExtensionAlert;
 import org.zaproxy.zap.testutils.TestUtils;
@@ -66,16 +69,38 @@ void setup() {
     @Test
     void shouldFindHistoryRef() throws Exception {
         // Given
-        given(extHistory.getLastHistoryId()).willReturn(3);
+        Model model = mock();
+        given(extHistory.getModel()).willReturn(model);
+        Session session = mock();
+        given(model.getSession()).willReturn(session);
+        SiteMap siteTree = mock();
+        given(session.getSiteTree()).willReturn(siteTree);
+        SiteNode siteNode = mock();
         String url = "http://example.com/";
-        HistoryReference href1 = mockHistoryReference(url);
-        HistoryReference href2Deleted = null;
-        HistoryReference href3 = mockHistoryReference("http://not.example.com/");
-        given(extHistory.getHistoryReference(anyInt())).willReturn(href3, href2Deleted, href1);
+        given(siteTree.findNode(new URI(url, true))).willReturn(siteNode);
+        HistoryReference href = mockHistoryReference(url);
+        given(siteNode.getHistoryReference()).willReturn(href);
         // When
         HistoryReference foundHref = helper.findHistoryRef(url);
         // Then
-        assertThat(foundHref, is(equalTo(href1)));
+        assertThat(foundHref, is(equalTo(href)));
+    }
+
+    @Test
+    void shouldNotFindHistoryRefIfNotPresent() throws Exception {
+        // Given
+        Model model = mock();
+        given(extHistory.getModel()).willReturn(model);
+        Session session = mock();
+        given(model.getSession()).willReturn(session);
+        SiteMap siteTree = mock();
+        given(session.getSiteTree()).willReturn(siteTree);
+        String url = "http://example.com/";
+        given(siteTree.findNode(new URI(url, true))).willReturn(null);
+        // When
+        HistoryReference foundHref = helper.findHistoryRef(url);
+        // Then
+        assertThat(foundHref, is(nullValue()));
     }
 
     private static HistoryReference mockHistoryReference(String url) throws URIException {