From 2b27abc2b8c9b64ff18def250f3afbcf14ca76d9 Mon Sep 17 00:00:00 2001 From: thc202 Date: Fri, 26 Jun 2026 17:49:29 +0100 Subject: [PATCH] client: consume element locator Use the locator provided by the browser extension when crawling. Signed-off-by: thc202 --- addOns/client/CHANGELOG.md | 3 +- .../addon/client/internal/ClientMap.java | 8 +- .../client/internal/ClientMapListener.java | 8 +- .../client/internal/ClientSideComponent.java | 38 +++- .../addon/client/internal/ElementLocator.java | 29 +++ .../client/spider/ActionWaitStrategy.java | 5 +- .../addon/client/spider/ClientSpider.java | 68 +++---- .../spider/actions/BaseElementAction.java | 10 +- .../client/spider/actions/ClickElement.java | 50 +---- .../client/spider/actions/FollowGraph.java | 12 +- .../client/spider/actions/SubmitForm.java | 21 +-- .../client/internal/ClientMapUnitTest.java | 114 +++--------- .../internal/ClientSideComponentUnitTest.java | 81 +++++++++ .../client/spider/ClientSpiderUnitTest.java | 169 +++++------------ .../actions/BaseElementActionUnitTest.java | 9 +- .../spider/actions/ClickElementUnitTest.java | 172 ++++++++---------- .../spider/actions/FollowGraphUnitTest.java | 17 +- .../spider/actions/SubmitFormUnitTest.java | 56 ++++-- 18 files changed, 419 insertions(+), 451 deletions(-) create mode 100644 addOns/client/src/main/java/org/zaproxy/addon/client/internal/ElementLocator.java diff --git a/addOns/client/CHANGELOG.md b/addOns/client/CHANGELOG.md index c7184943c72..7380f8108c9 100644 --- a/addOns/client/CHANGELOG.md +++ b/addOns/client/CHANGELOG.md @@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased - +### Changed +- Use new functionality from the browser extension for crawling. ## [0.28.0] - 2026-06-26 ### Added diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMap.java b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMap.java index 9a86d9717f1..189f98961dd 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMap.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMap.java @@ -236,13 +236,15 @@ private boolean addComponentToNode(ClientNode node, ClientSideComponent componen if (!wasVisited || componentAdded) { details.setVisited(true); + int depth = node.getLevel(); + int siblings = node.getChildCount(); Map map = new HashMap<>(component.getData()); - map.put(DEPTH_KEY, Integer.toString(node.getLevel())); - map.put(SIBLINGS_KEY, Integer.toString(node.getChildCount())); + map.put(DEPTH_KEY, Integer.toString(depth)); + map.put(SIBLINGS_KEY, Integer.toString(siblings)); ZAP.getEventBus() .publishSyncEvent( this, new Event(this, MAP_COMPONENT_ADDED_EVENT, new Target(), map)); - listeners.forEach(l -> l.componentAdded(map, source)); + listeners.forEach(l -> l.componentAdded(component, depth, siblings, source)); notifyNodeChanged(node); } return componentAdded; diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapListener.java b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapListener.java index f0960935619..71b8288cf8c 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapListener.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapListener.java @@ -19,8 +19,6 @@ */ package org.zaproxy.addon.client.internal; -import java.util.Map; - /** Listener notified when nodes or components are added to a {@link ClientMap}. */ public interface ClientMapListener { @@ -38,11 +36,13 @@ public interface ClientMapListener { /** * Called when a component is added to a node in the map. * - * @param parameters the component data parameters. + * @param component the component that was added. + * @param depth the depth of the node in the map. + * @param siblings the sibling count of the node (including itself) after insertion. * @param source an identifier for the source that triggered the addition, or {@code 0} if the * source is unknown. */ - void componentAdded(Map parameters, int source); + void componentAdded(ClientSideComponent component, int depth, int siblings, int source); /** * Called when a page-load event is reported for a URL. diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java index b3ec94ab7b0..069ac897fee 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java @@ -22,16 +22,15 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; -import lombok.AllArgsConstructor; +import lombok.AccessLevel; import lombok.Getter; -import lombok.NonNull; import lombok.Setter; import net.sf.json.JSONObject; +import org.openqa.selenium.By; import org.parosproxy.paros.Constant; import org.zaproxy.addon.client.ExtensionClientIntegration; @Getter -@AllArgsConstructor public class ClientSideComponent implements Comparable { public static final String REDIRECT = "Redirect"; @@ -148,10 +147,14 @@ public static Type getTypeForKey(String key) { private String parentUrl; private String href; private String text; - @NonNull private Type type; + private Type type; private String tagType; private int formId; @Setter private InteractableState interactable; + @Setter private ElementLocator elementLocator; + + @Getter(AccessLevel.NONE) + private By cachedBy; public ClientSideComponent( Map data, @@ -163,7 +166,29 @@ public ClientSideComponent( Type type, String tagType, int formId) { - this(data, tagName, id, parentUrl, href, text, type, tagType, formId, null); + this.data = data; + this.tagName = tagName; + this.id = id; + this.parentUrl = parentUrl; + this.href = href; + this.text = text; + this.type = Objects.requireNonNull(type); + this.tagType = tagType; + this.formId = formId; + } + + public By getBy() { + if (cachedBy == null && elementLocator != null) { + cachedBy = + switch (elementLocator.type()) { + case "id" -> By.id(elementLocator.element()); + case "className" -> By.className(elementLocator.element()); + case "cssSelector" -> By.cssSelector(elementLocator.element()); + case "xpath" -> By.xpath(elementLocator.element()); + default -> null; + }; + } + return cachedBy; } public ClientSideComponent(JSONObject json) { @@ -196,6 +221,9 @@ public ClientSideComponent(JSONObject json) { s.optBoolean("enabled", false), s.optBoolean("pointer", false)); } + if (json.containsKey("elementLocator") && !json.get("elementLocator").equals("null")) { + this.elementLocator = ElementLocator.fromJson(json.getJSONObject("elementLocator")); + } } public Map getData() { diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ElementLocator.java b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ElementLocator.java new file mode 100644 index 00000000000..0913d9c82ad --- /dev/null +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/internal/ElementLocator.java @@ -0,0 +1,29 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2026 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.addon.client.internal; + +import net.sf.json.JSONObject; + +public record ElementLocator(String type, String element) { + + public static ElementLocator fromJson(JSONObject json) { + return new ElementLocator(json.getString("type"), json.getString("element")); + } +} diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ActionWaitStrategy.java b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ActionWaitStrategy.java index 17073f5d598..d99578637ae 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ActionWaitStrategy.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ActionWaitStrategy.java @@ -19,8 +19,8 @@ */ package org.zaproxy.addon.client.spider; -import java.util.Map; import org.zaproxy.addon.client.internal.ClientMapListener; +import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.spider.ClientSpider.WebDriverProcess; public interface ActionWaitStrategy extends ClientMapListener { @@ -39,5 +39,6 @@ default void onRequestCompleted(String url) {} default void nodeAdded(String url, int depth, int siblings, int source) {} @Override - default void componentAdded(Map parameters, int source) {} + default void componentAdded( + ClientSideComponent component, int depth, int siblings, int source) {} } diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpider.java b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpider.java index da209786c49..6e323d4201d 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpider.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/ClientSpider.java @@ -29,7 +29,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; @@ -40,7 +39,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.IntSupplier; import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Stream; @@ -371,9 +369,8 @@ private void addExistingTasks(ClientNode node) { && isUrlInScope(nodeUrl)) { addFollowGraphTask(nodeUrl); for (ClientSideComponent component : details.getComponents()) { - Map data = component.getData(); - if (SubmitForm.isSupported(data)) { - addSubmitTask(nodeUrl, data); + if (SubmitForm.isSupported(component)) { + addSubmitTask(nodeUrl, component); } } } @@ -408,12 +405,13 @@ public void returnWebDriverProcess(WebDriverProcess wdp) { } } - private void addSubmitTask(String nodeUrl, Map data) { + private void addSubmitTask(String nodeUrl, ClientSideComponent component) { addTask( nodeUrl, - followGraphAction(nodeUrl, new SubmitForm(valueProvider, createUri(nodeUrl), data)), + followGraphAction( + nodeUrl, new SubmitForm(valueProvider, createUri(nodeUrl), component)), Constant.messages.getString("client.spider.panel.table.action.submit"), - paramsToString(data)); + paramsToString(component)); } private ClientSpiderTask addTask( @@ -490,8 +488,7 @@ private class ClientMapListenerImpl implements ClientMapListener { private static final Pattern SCHEME_PATTERN = Pattern.compile("^https?://", Pattern.CASE_INSENSITIVE); - private boolean shouldIgnore( - String url, int source, IntSupplier depthSupplier, IntSupplier childrenSupplier) { + private boolean shouldIgnore(String url, int source, int depth, int siblings) { if (stopping.get() || stopped || !proxyPorts.contains(source)) { return true; } @@ -505,7 +502,6 @@ private boolean shouldIgnore( } if (options.getMaxDepth() > 0) { - int depth = depthSupplier.getAsInt(); if (depth > options.getMaxDepth()) { LOGGER.debug( "Ignoring URL - too deep {} > {} : {}", @@ -518,7 +514,6 @@ private boolean shouldIgnore( } if (options.getMaxChildren() > 0) { - int siblings = childrenSupplier.getAsInt(); if (siblings > options.getMaxChildren()) { LOGGER.debug( "Ignoring URL - too wide {} > {} : {}", @@ -545,7 +540,7 @@ public void nodeAdded(String url, int depth, int siblings, int source) { if (scanOptions.isExistingOnly()) { return; } - if (shouldIgnore(url, source, () -> depth, () -> siblings)) { + if (shouldIgnore(url, source, depth, siblings)) { return; } @@ -553,13 +548,13 @@ public void nodeAdded(String url, int depth, int siblings, int source) { addDiscoveredUrl(url); } - private boolean isHrefAlreadyHandled(Map parameters) { - String href = parameters.get("href"); + private boolean isHrefAlreadyHandled(ClientSideComponent component) { + String href = component.getHref(); if (href == null || !SCHEME_PATTERN.matcher(href).find()) { return false; } - String sourceUrl = parameters.get(ClientMap.URL_KEY); + String sourceUrl = component.getParentUrl(); if (sourceUrl.equals(href)) { return true; } @@ -579,34 +574,31 @@ private boolean isHrefAlreadyHandled(Map parameters) { } @Override - public void componentAdded(Map parameters, int source) { + public void componentAdded( + ClientSideComponent component, int depth, int siblings, int source) { if (scanOptions.isExistingOnly()) { return; } - String url = parameters.get(ClientMap.URL_KEY); - if (shouldIgnore( - url, - source, - () -> Integer.parseInt(parameters.get(ClientMap.DEPTH_KEY)), - () -> Integer.parseInt(parameters.get(ClientMap.SIBLINGS_KEY)))) { + String url = component.getParentUrl(); + if (shouldIgnore(url, source, depth, siblings)) { return; } Stats.incCounter("stats.client.spider.event.component"); - if (ClickElement.isSupported(ClientSpider.this::isUrlInScope, parameters) - && !(options.isLogoutAvoidance() && isLogoutElement(parameters)) - && !isHrefAlreadyHandled(parameters)) { + if (ClickElement.isSupported(ClientSpider.this::isUrlInScope, component) + && !(options.isLogoutAvoidance() && isLogoutElement(component)) + && !isHrefAlreadyHandled(component)) { Stats.incCounter("stats.client.spider.event.component.click"); addTask( url, followGraphAction( url, - new ClickElement(valueProvider, createUri(url), parameters, false)), + new ClickElement(valueProvider, createUri(url), component, false)), Constant.messages.getString("client.spider.panel.table.action.click"), - paramsToString(parameters)); - } else if (SubmitForm.isSupported(parameters)) { + paramsToString(component)); + } else if (SubmitForm.isSupported(component)) { Stats.incCounter("stats.client.spider.event.component.form"); - addSubmitTask(url, parameters); + addSubmitTask(url, component); } } } @@ -666,8 +658,8 @@ protected ResourceState checkResourceState(URI uri, String hostName, boolean all return state; } - private static boolean isLogoutElement(Map parameters) { - String text = parameters.get("text"); + private static boolean isLogoutElement(ClientSideComponent component) { + String text = component.getText(); if (text == null || text.isBlank()) { return false; } @@ -675,21 +667,21 @@ private static boolean isLogoutElement(Map parameters) { return AuthConstants.getLogoutIndicators().stream().anyMatch(normalized::contains); } - private static String paramsToString(Map parameters) { - String tag = parameters.get("tagName"); + private static String paramsToString(ClientSideComponent component) { + String tag = component.getTagName(); if (tag != null) { switch (tag) { case "A": return Constant.messages.getString( "client.spider.panel.table.details.link", - parameters.get("href"), - parameters.get("text")); + component.getHref(), + component.getText()); case "BUTTON": return Constant.messages.getString( - "client.spider.panel.table.details.button", parameters.get("text")); + "client.spider.panel.table.details.button", component.getText()); } } - return parameters.toString(); + return component.getData().toString(); } private static URI createUri(String value) { diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/BaseElementAction.java b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/BaseElementAction.java index 7cb51edf826..2529cb06585 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/BaseElementAction.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/BaseElementAction.java @@ -28,6 +28,7 @@ import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.client.spider.SpiderAction; import org.zaproxy.addon.commonlib.ValueProvider; @@ -39,10 +40,13 @@ abstract class BaseElementAction implements SpiderAction { private final ValueProvider valueProvider; private final URI uri; + protected final ClientSideComponent component; - protected BaseElementAction(ValueProvider valueProvider, URI uri) { + protected BaseElementAction( + ValueProvider valueProvider, URI uri, ClientSideComponent component) { this.valueProvider = Objects.requireNonNull(valueProvider); this.uri = Objects.requireNonNull(uri); + this.component = Objects.requireNonNull(component); } protected URI getUri() { @@ -54,7 +58,7 @@ public final boolean run(ActionWaitStrategy waitStrategy, WebDriver wd) { String statsPrefix = getStatsPrefix(); Stats.incCounter(statsPrefix); - By by = getElementBy(); + By by = component.getBy(); if (by == null) { Stats.incCounter(statsPrefix + ".noby"); return false; @@ -78,8 +82,6 @@ public final boolean run(ActionWaitStrategy waitStrategy, WebDriver wd) { protected abstract String getStatsPrefix(); - protected abstract By getElementBy(); - protected abstract boolean run( ActionWaitStrategy waitStrategy, WebDriver wd, WebElement element, String statsPrefix); diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/ClickElement.java b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/ClickElement.java index 542327e7963..58b4d69edcc 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/ClickElement.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/ClickElement.java @@ -19,16 +19,14 @@ */ package org.zaproxy.addon.client.spider.actions; -import java.util.Map; -import java.util.Objects; import java.util.function.Predicate; import org.apache.commons.httpclient.URI; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; import org.zaproxy.zap.utils.Stats; @@ -39,20 +37,13 @@ public class ClickElement extends BaseElementAction { private static final String STATS_PREFIX = "stats.client.spider.action.click"; - private final Map elementData; private final boolean passive; - private final String tagName; public ClickElement( - ValueProvider valueProvider, - URI uri, - Map elementData, - boolean passive) { - super(valueProvider, uri); + ValueProvider valueProvider, URI uri, ClientSideComponent component, boolean passive) { + super(valueProvider, uri, component); - this.elementData = Objects.requireNonNull(elementData); this.passive = passive; - tagName = getTagName(elementData); } @Override @@ -73,42 +64,19 @@ public boolean run( return false; } - @Override - protected By getElementBy() { - return getBy(elementData); - } - @Override protected String getStatsPrefix() { - return STATS_PREFIX + ".tag." + tagName; - } - - private static By getBy(Map data) { - String id = data.get("id"); - if (StringUtils.isNotBlank(id)) { - return By.id(id); - } - - String tag = getTagName(data); - String text = data.get("text"); - if ("INPUT".equalsIgnoreCase(tag)) { - return By.xpath("//" + tag + "[@value='" + text + "']"); - } - - if (StringUtils.isNotBlank(text)) { - return By.xpath("//" + tag + "[contains(text(), '" + text + "')]"); - } - - return By.tagName(tag); + return STATS_PREFIX + ".tag." + component.getTagName(); } - public static boolean isSupported(Predicate scopeChecker, Map data) { - String tag = getTagName(data); + public static boolean isSupported( + Predicate scopeChecker, ClientSideComponent component) { + String tag = component.getTagName(); if (tag == null) { return false; } - String href = data.get("href"); + String href = component.getHref(); if (href != null && !scopeChecker.test(href)) { return false; } @@ -118,7 +86,7 @@ public static boolean isSupported(Predicate scopeChecker, Map elementData) { - super(valueProvider, uri, elementData, true); - tagName = getTagName(elementData); + ValueProvider valueProvider, URI uri, ClientSideComponent component) { + super(valueProvider, uri, component, true); } @Override protected String getStatsPrefix() { - return STATS_PREFIX + ".tag." + tagName; + return STATS_PREFIX + ".tag." + component.getTagName(); } } diff --git a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/SubmitForm.java b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/SubmitForm.java index b3dd0ab6418..19aea796e3b 100644 --- a/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/SubmitForm.java +++ b/addOns/client/src/main/java/org/zaproxy/addon/client/spider/actions/SubmitForm.java @@ -19,14 +19,13 @@ */ package org.zaproxy.addon.client.spider.actions; -import java.util.Map; -import java.util.Objects; import org.apache.commons.httpclient.URI; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; import org.zaproxy.zap.utils.Stats; @@ -37,14 +36,11 @@ public class SubmitForm extends BaseElementAction { private static final String STATS_PREFIX = "stats.client.spider.action.form"; - private final String tagName; private final int formIndex; - public SubmitForm(ValueProvider valueProvider, URI uri, Map elementData) { - super(valueProvider, uri); - Objects.requireNonNull(elementData); - tagName = getTagName(elementData); - formIndex = Integer.valueOf(elementData.get("formId")); + public SubmitForm(ValueProvider valueProvider, URI uri, ClientSideComponent component) { + super(valueProvider, uri, component); + this.formIndex = component.getFormId(); } @Override @@ -64,17 +60,12 @@ public boolean run( return false; } - @Override - protected By getElementBy() { - return By.xpath("(//" + tagName + ")[" + (formIndex + 1) + "]"); - } - @Override protected String getStatsPrefix() { return STATS_PREFIX + "." + formIndex; } - public static boolean isSupported(Map data) { - return data.containsKey("formId") && "FORM".equalsIgnoreCase(getTagName(data)); + public static boolean isSupported(ClientSideComponent component) { + return "FORM".equalsIgnoreCase(component.getTagName()); } } diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientMapUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientMapUnitTest.java index 06dc869c906..20d666d56c6 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientMapUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientMapUnitTest.java @@ -20,6 +20,7 @@ package org.zaproxy.addon.client.internal; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -27,6 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.lenient; @@ -39,12 +41,14 @@ import java.util.Map; import java.util.Set; import java.util.function.Consumer; +import net.sf.json.JSONObject; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.parosproxy.paros.model.Session; import org.zaproxy.addon.client.ExtensionClientIntegration; @@ -506,7 +510,7 @@ void shouldAddComponentCreatingNodeIfAbsent() { assertThat(node.getUserObject().isVisited(), is(true)); assertThat(node.getUserObject().getComponents(), is(Set.of(component))); verify(listener).nodeAdded(BBB_URL, 1, 1, 0); - verify(listener).componentAdded(Map.of("siblings", "0", "depth", "1"), 0); + verify(listener).componentAdded(component, 1, 0, 0); } @Test @@ -532,7 +536,7 @@ void shouldAddComponentToExistingNode() { assertThat(map.getNode(BBB_URL, false, false), is(existing)); assertThat(existing.getUserObject().getComponents(), is(Set.of(component))); verify(listener).nodeAdded(BBB_URL, 1, 1, 0); - verify(listener).componentAdded(Map.of("siblings", "0", "depth", "1"), 0); + verify(listener).componentAdded(component, 1, 0, 0); } @Test @@ -562,7 +566,7 @@ void shouldAddStorageComponentToStorageNode() { ClientNode storageNode = map.getNode(storageUrl, false, true); assertThat(storageNode, is(notNullValue())); assertThat(storageNode.getUserObject().getComponents(), is(Set.of(component))); - verify(listener, times(2)).componentAdded(Map.of("siblings", "0", "depth", "1"), 0); + verify(listener, times(2)).componentAdded(component, 1, 0, 0); } @Test @@ -577,19 +581,7 @@ void shouldAddComponentToClientMapOnReportObject() { // Then assertThat(map.getNode(url, false, false), is(notNullValue())); verify(listener).nodeAdded(url, 2, 1, 0); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", "null", - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 0); + verifyNotifiedAddedComponent(component(json), 2, 0, 0); } @Test @@ -604,19 +596,7 @@ void shouldNotifyListenerWithCorrectSourceOnReportObject() { // Then assertThat(map.getNode(url, false, false), is(notNullValue())); verify(listener).nodeAdded(url, 2, 1, 42); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", "null", - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 42); + verifyNotifiedAddedComponent(component(json), 2, 0, 42); } @Test @@ -647,19 +627,7 @@ void shouldAddHrefNodeOnReportObject() { assertThat(map.getNode(href, false, false), is(notNullValue())); verify(listener).nodeAdded(url, 2, 1, 0); verify(listener).nodeAdded(href, 2, 2, 0); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", href, - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 0); + verifyNotifiedAddedComponent(component(json), 2, 0, 0); } @Test @@ -676,19 +644,7 @@ void shouldNotifyListenerWithCorrectSourceOnNodeAddedViaHref() { assertThat(map.getNode(href, false, false), is(notNullValue())); verify(listener).nodeAdded(url, 2, 1, 42); verify(listener).nodeAdded(href, 2, 2, 42); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", href, - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 42); + verifyNotifiedAddedComponent(component(json), 2, 0, 42); } @ParameterizedTest @@ -773,19 +729,7 @@ void shouldCallConsumerOnHandleReportObject() { // Then verify(consumer).accept(any(ReportedElement.class)); verify(listener).nodeAdded(url, 2, 1, 0); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", "null", - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 0); + verify(listener).componentAdded(any(ClientSideComponent.class), eq(2), eq(0), eq(0)); } @Test @@ -799,19 +743,7 @@ void shouldNotifyListenerWithCorrectSourceOnComponentAdded() { // Then verify(listener).nodeAdded(url, 2, 1, 42); - verify(listener) - .componentAdded( - Map.of( - "nodeName", "INPUT", - "siblings", "0", - "depth", "2", - "id", "", - "href", "null", - "tagName", "INPUT", - "type", "input", - "url", url, - "timestamp", "0"), - 42); + verify(listener).componentAdded(any(ClientSideComponent.class), eq(2), eq(0), eq(42)); } @Test @@ -890,7 +822,8 @@ public void nodeAdded(String url, int depth, int siblings, int source) { } @Override - public void componentAdded(Map parameters, int source) { + public void componentAdded( + ClientSideComponent component, int depth, int siblings, int source) { map.addListener(mock(ClientMapListener.class)); } }; @@ -1126,7 +1059,7 @@ void shouldNotUpdateInteractableWhenNoMatchingComponentInNode() { // Then assertThat(existing.getInteractable(), is(nullValue())); - verify(listener).componentAdded(any(), eq(0)); + verify(listener).componentAdded(eq(existing), anyInt(), anyInt(), eq(0)); } @Test @@ -1143,7 +1076,7 @@ void shouldUpdateComponentInteractableOnNodeChanged() { // Then assertThat(existing.getInteractable(), is(new InteractableState(true, true, false))); - verify(listener).componentAdded(any(), eq(0)); + verify(listener).componentAdded(eq(existing), anyInt(), anyInt(), eq(0)); } @Test @@ -1162,6 +1095,17 @@ void shouldNotUpdateWhenInteractableAlreadySameState() { // Then assertThat(existing.getInteractable(), is(state)); - verify(listener).componentAdded(any(), eq(0)); + verify(listener).componentAdded(eq(existing), anyInt(), anyInt(), eq(0)); + } + + private void verifyNotifiedAddedComponent( + ClientSideComponent component, int depth, int siblings, int source) { + ArgumentCaptor captor = ArgumentCaptor.captor(); + verify(listener).componentAdded(captor.capture(), eq(depth), eq(siblings), eq(source)); + assertThat(captor.getValue(), is(equalTo(component))); + } + + private static ClientSideComponent component(String json) { + return new ClientSideComponent(JSONObject.fromObject(json)); } } diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponentUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponentUnitTest.java index ce15ee0de18..07e71cf06df 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponentUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponentUnitTest.java @@ -25,6 +25,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.List; @@ -39,6 +40,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.openqa.selenium.By; import org.zaproxy.addon.client.ExtensionClientIntegration; import org.zaproxy.addon.client.internal.ClientSideComponent.Type; import org.zaproxy.zap.testutils.TestUtils; @@ -506,4 +508,83 @@ void shouldParseInteractableObjectFromJson() { assertThat(state.isEnabled(), is(false)); assertThat(state.isPointer(), is(true)); } + + @Test + void shouldReturnNullByWhenNoElementLocator() { + // Given + ClientSideComponent component = + new ClientSideComponent( + Map.of(), "A", "", EXAMPLE_URL, EXAMPLE_URL, "", Type.LINK, "", -1); + // When / Then + assertThat(component.getBy(), is(nullValue())); + } + + @ParameterizedTest + @MethodSource("provideLocatorTypes") + void shouldReturnCorrectByForLocatorType( + String locatorType, String locatorValue, By expectedBy) { + // Given + ClientSideComponent component = + new ClientSideComponent( + Map.of(), "A", "", EXAMPLE_URL, EXAMPLE_URL, "", Type.LINK, "", -1); + component.setElementLocator(new ElementLocator(locatorType, locatorValue)); + // When + By by = component.getBy(); + // Then + assertThat(by, is(equalTo(expectedBy))); + } + + static Stream provideLocatorTypes() { + return Stream.of( + Arguments.of("id", "my-id", By.id("my-id")), + Arguments.of("className", "my-class", By.className("my-class")), + Arguments.of("cssSelector", "div > a", By.cssSelector("div > a")), + Arguments.of("xpath", "//div/a", By.xpath("//div/a"))); + } + + @Test + void shouldCacheByOnSubsequentCalls() { + // Given + ClientSideComponent component = + new ClientSideComponent( + Map.of(), "A", "", EXAMPLE_URL, EXAMPLE_URL, "", Type.LINK, "", -1); + component.setElementLocator(new ElementLocator("id", "my-id")); + // When + By first = component.getBy(); + By second = component.getBy(); + // Then + assertThat(first, is(sameInstance(second))); + } + + @Test + void shouldDefaultElementLocatorToNullFromJson() { + // Given + JSONObject json = + JSONObject.fromObject( + """ + {"tagName": "A", "id": "", "type": "nodeAdded", "url": "%s", "timestamp": 0}""" + .formatted(EXAMPLE_URL)); + // When + ClientSideComponent component = new ClientSideComponent(json); + // Then + assertThat(component.getElementLocator(), is(nullValue())); + } + + @Test + void shouldParseElementLocatorFromJson() { + // Given + JSONObject json = + JSONObject.fromObject( + """ + {"tagName": "A", "id": "my-link", "type": "nodeAdded", "url": "%s", "timestamp": 0, + "elementLocator": {"type": "id", "element": "my-link"}}""" + .formatted(EXAMPLE_URL)); + // When + ClientSideComponent component = new ClientSideComponent(json); + // Then + ElementLocator locator = component.getElementLocator(); + assertThat(locator, is(notNullValue())); + assertThat(locator.type(), is("id")); + assertThat(locator.element(), is("my-link")); + } } diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/ClientSpiderUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/ClientSpiderUnitTest.java index 3a34efdc3d5..d0c178c0132 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/ClientSpiderUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/ClientSpiderUnitTest.java @@ -92,6 +92,7 @@ import org.zaproxy.addon.client.internal.ClientNode; import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.internal.ClientSideDetails; +import org.zaproxy.addon.client.internal.ElementLocator; import org.zaproxy.addon.commonlib.ValueProvider; import org.zaproxy.addon.commonlib.http.HttpFieldsNames; import org.zaproxy.addon.network.ExtensionNetwork; @@ -573,18 +574,9 @@ void shouldHandleComponentAdded() { waitForProxy(); // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "text", - "Click", - "depth", - "1"), - PROXY_PORT); + ClientSideComponent component = linkComponent(seedUrl, "A", "Click"); + component.setElementLocator(new ElementLocator("xpath", "//A[contains(text(), 'Click')]")); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -598,18 +590,9 @@ void shouldNotHandleComponentAddedIfNotFromProxy() { waitForProxy(); // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "text", - "Click", - "depth", - "1"), - 1234); + ClientSideComponent component = linkComponent(seedUrl, "A", "Click"); + component.setElementLocator(new ElementLocator("xpath", "//A[contains(text(), 'Click')]")); + clientMapListener().componentAdded(component, 1, 0, 1234); sleep(); // Then @@ -627,18 +610,10 @@ void shouldHandleComponentHrefWithoutHostname() { clientMapListener().nodeAdded(url, 0, 1, PROXY_PORT); // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - url, - "tagName", - "area", - "href", - "#", - "depth", - "1"), - PROXY_PORT); + ClientSideComponent component = + new ClientSideComponent( + Map.of(), "area", "", url, "#", "", ClientSideComponent.Type.LINK, "", -1); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -665,18 +640,9 @@ void shouldHandleLogoutElementsBasedOnLogoutAvoidance( waitForProxy(); // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - url, - "tagName", - "A", - "text", - logoutText, - "depth", - "1"), - PROXY_PORT); + ClientSideComponent component = linkComponent(url, "A", logoutText); + component.setElementLocator(new ElementLocator("xpath", "//A[contains(text(), 'logout')]")); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -698,21 +664,10 @@ void shouldNotSkipClickForLinkComponentWithHref(String href) { spider.run(); waitForProxy(); + ClientSideComponent component = linkAComponent(seedUrl, href, "Unknown Link"); + // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "href", - href, - "text", - "Unknown Link", - "depth", - "1"), - PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -724,22 +679,10 @@ void shouldSkipClickForLinkComponentForSamePage() { // Given spider.run(); waitForProxy(); + ClientSideComponent component = linkAComponent(seedUrl, seedUrl, "Same Page Link"); // When - clientMapListener() - .componentAdded( - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "href", - seedUrl, - "text", - "Same Page Link", - "depth", - "1"), - PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -751,26 +694,14 @@ void shouldSkipClickForLinkComponentWithAlreadyHandledHref() { // Given String href = "https://www.example.com/queued-once"; given(map.getNode(href, false, false)).willReturn(null); + ClientSideComponent component = linkAComponent(seedUrl, href, "Same Link"); spider.run(); waitForProxy(); - Map params = - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "href", - href, - "text", - "Some Link", - "depth", - "1"); - // When - clientMapListener().componentAdded(params, PROXY_PORT); - clientMapListener().componentAdded(params, PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -783,23 +714,11 @@ void shouldNotSkipClickForLinkComponentWithNonHttpHref(String href) { // Given spider.run(); waitForProxy(); - - Map params = - Map.of( - ClientMap.URL_KEY, - seedUrl, - "tagName", - "A", - "href", - href, - "text", - "Some Link", - "depth", - "1"); + ClientSideComponent component = linkAComponent(seedUrl, href, "Some Link"); // When - clientMapListener().componentAdded(params, PROXY_PORT); - clientMapListener().componentAdded(params, PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); + clientMapListener().componentAdded(component, 1, 0, PROXY_PORT); sleep(); // Then @@ -925,16 +844,7 @@ void shouldNotProcessComponentsDiscoveredDuringExistingOnlyScan() { // When clientMapListener() .componentAdded( - Map.of( - ClientMap.URL_KEY, - existingUrl, - "tagName", - "A", - "text", - "Some Link", - "depth", - "1"), - PROXY_PORT); + linkAComponent(seedUrl, existingUrl, "Some Link"), 1, 0, PROXY_PORT); sleep(); // Then @@ -960,7 +870,7 @@ void shouldSubmitKnownFormComponentsWhenExistingOnly() { sleep(); // Then - FollowGraph navigates to the page, then SubmitForm finds and submits the form - verify(wd).findElement(By.xpath("(//FORM)[1]")); + verify(wd).findElement(By.xpath("//FORM[1]")); } class SpiderStatus { @@ -987,6 +897,27 @@ public boolean isStopped() { } } + private ClientSideComponent linkAComponent(String parentUrl, String href, String text) { + ClientSideComponent component = + new ClientSideComponent( + Map.of(), + "A", + "", + parentUrl, + href, + text, + ClientSideComponent.Type.LINK, + "", + -1); + component.setElementLocator(new ElementLocator("xpath", "//A[contains(text(), 'logout')]")); + return component; + } + + private static ClientSideComponent linkComponent(String url, String tagName, String text) { + return new ClientSideComponent( + Map.of(), tagName, "", url, null, text, ClientSideComponent.Type.LINK, "", -1); + } + private static ClientNode mockClientNode( String url, boolean storage, boolean visited, boolean contentLoaded) { ClientNode node = mock(withSettings().strictness(Strictness.LENIENT)); @@ -1037,8 +968,8 @@ private static ClientNode mockRootNode(ClientNode... children) { private static ClientSideComponent mockFormComponent(int formId) { ClientSideComponent component = mock(withSettings().strictness(Strictness.LENIENT)); - given(component.getData()) - .willReturn(Map.of("formId", String.valueOf(formId), "tagName", "FORM")); + given(component.getBy()).willReturn(By.xpath("//FORM[" + (formId + 1) + "]")); + given(component.getTagName()).willReturn("FORM"); return component; } diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/BaseElementActionUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/BaseElementActionUnitTest.java index e696ffc887d..27f2265e18c 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/BaseElementActionUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/BaseElementActionUnitTest.java @@ -37,6 +37,7 @@ import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebDriverException; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; @@ -51,9 +52,10 @@ class BaseElementActionUnitTest { void setupEach() throws IOException { valueProvider = mock(ValueProvider.class); uri = new URI("http://localhost:1234/test", true); + ClientSideComponent component = mock(); action = - new BaseElementAction(valueProvider, uri) { + new BaseElementAction(valueProvider, uri, component) { @Override protected boolean run( @@ -68,11 +70,6 @@ protected boolean run( protected String getStatsPrefix() { return "prefix"; } - - @Override - protected By getElementBy() { - return null; - } }; } diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/ClickElementUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/ClickElementUnitTest.java index ef27ff7db09..2a9eb3b6f1c 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/ClickElementUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/ClickElementUnitTest.java @@ -46,11 +46,13 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; +import org.zaproxy.addon.client.internal.ClientSideComponent.Type; +import org.zaproxy.addon.client.internal.ElementLocator; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; import org.zaproxy.zap.extension.stats.InMemoryStats; @@ -79,7 +81,7 @@ void tearDown() { } @Test - void shouldThrowIfElementDataIsNull() { + void shouldThrowIfComponentIsNull() { assertThrows( NullPointerException.class, () -> new ClickElement(valueProvider, uri, null, false)); @@ -88,11 +90,11 @@ void shouldThrowIfElementDataIsNull() { @Test void shouldClickElementOnRun() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); WebElement element = visibleElement(); - given(wd.findElement(any(By.class))).willReturn(element); + given(wd.findElement(By.id("btn"))).willReturn(element); given(wd.findElements(any(By.class))).willReturn(List.of()); // When @@ -108,8 +110,8 @@ void shouldClickElementOnRun() { @Test void shouldFillInputsBeforeClicking() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); WebElement element = visibleElement(); given(wd.findElement(any(By.class))).willReturn(element); @@ -136,8 +138,8 @@ void shouldFillInputsBeforeClicking() { @Test void shouldNotFillInputsBeforeClickingWhenPassive() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, true); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, true); WebDriver wd = mock(WebDriver.class); WebElement element = visibleElement(); given(wd.findElement(any(By.class))).willReturn(element); @@ -161,8 +163,8 @@ void shouldNotFillInputsBeforeClickingWhenPassive() { @Test void shouldHandleClickExceptionGracefully() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); WebElement element = visibleElement(); given(wd.findElement(any(By.class))).willReturn(element); @@ -179,8 +181,8 @@ void shouldHandleClickExceptionGracefully() { @Test void shouldIncrementStatsWhenElementNotFound() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); given(wd.findElement(any(By.class))).willThrow(RuntimeException.class); @@ -196,8 +198,8 @@ void shouldIncrementStatsWhenElementNotFound() { @Test void shouldIncrementStatsWhenElementNotDisplayed() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = componentWithLocator("A", "id", "btn"); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); WebElement element = mock(WebElement.class); given(wd.findElement(any(By.class))).willReturn(element); @@ -213,92 +215,28 @@ void shouldIncrementStatsWhenElementNotDisplayed() { } @Test - void shouldUseByIdWhenIdPresent() { - // Given - String id = "my-button"; - Map elementData = Map.of("tagName", "BUTTON", "id", id); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); - WebDriver wd = mock(WebDriver.class); - WebElement visibleElement = visibleElement(); - ArgumentCaptor byCaptor = ArgumentCaptor.forClass(By.class); - given(wd.findElement(byCaptor.capture())).willReturn(visibleElement); - given(wd.findElements(any(By.class))).willReturn(List.of()); - - // When - boolean result = action.run(waitStrategy, wd); - - // Then - assertThat(result, is(equalTo(true))); - assertThat(byCaptor.getValue(), is(By.id(id))); - } - - @Test - void shouldUseXpathByValueForInputTag() { - // Given - String text = "Submit"; - Map elementData = Map.of("tagName", "INPUT", "text", text); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); - WebDriver wd = mock(WebDriver.class); - WebElement visibleElement = visibleElement(); - ArgumentCaptor byCaptor = ArgumentCaptor.forClass(By.class); - given(wd.findElement(byCaptor.capture())).willReturn(visibleElement); - given(wd.findElements(any(By.class))).willReturn(List.of()); - - // When - boolean result = action.run(waitStrategy, wd); - - // Then - assertThat(result, is(equalTo(true))); - assertThat(byCaptor.getValue(), is(By.xpath("//INPUT[@value='" + text + "']"))); - } - - @Test - void shouldUseXpathContainsTextForOtherTag() { - // Given - String text = "Click me"; - Map elementData = Map.of("tagName", "BUTTON", "text", text); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); - WebDriver wd = mock(WebDriver.class); - WebElement visibleElement = visibleElement(); - ArgumentCaptor byCaptor = ArgumentCaptor.forClass(By.class); - given(wd.findElement(byCaptor.capture())).willReturn(visibleElement); - given(wd.findElements(any(By.class))).willReturn(List.of()); - - // When - boolean result = action.run(waitStrategy, wd); - - // Then - assertThat(result, is(equalTo(true))); - assertThat(byCaptor.getValue(), is(By.xpath("//BUTTON[contains(text(), '" + text + "')]"))); - } - - @Test - void shouldUseByTagNameWhenNoText() { + void shouldYieldNoByWhenNoElementLocator() { // Given - Map elementData = Map.of("tagName", "A"); - ClickElement action = new ClickElement(valueProvider, uri, elementData, false); + ClientSideComponent component = component("A", null); + ClickElement action = new ClickElement(valueProvider, uri, component, false); WebDriver wd = mock(WebDriver.class); - WebElement visibleElement = visibleElement(); - ArgumentCaptor byCaptor = ArgumentCaptor.forClass(By.class); - given(wd.findElement(byCaptor.capture())).willReturn(visibleElement); - given(wd.findElements(any(By.class))).willReturn(List.of()); // When boolean result = action.run(waitStrategy, wd); // Then - assertThat(result, is(equalTo(true))); - assertThat(byCaptor.getValue(), is(By.tagName("A"))); + assertThat(result, is(equalTo(false))); + assertThat(stats.getStat("stats.client.spider.action.click.tag.A.noby"), is(1L)); } @ParameterizedTest @MethodSource("provideIsSupportedData") - void shouldReturnExpectedSupportedValue(Map data, boolean expected) { + void shouldReturnExpectedSupportedValue(ClientSideComponent component, boolean expected) { // Given Predicate scopeChecker = href -> true; // When - boolean supported = ClickElement.isSupported(scopeChecker, data); + boolean supported = ClickElement.isSupported(scopeChecker, component); // Then assertThat(supported, is(expected)); @@ -306,33 +244,73 @@ void shouldReturnExpectedSupportedValue(Map data, boolean expect static Stream provideIsSupportedData() { return Stream.of( - arguments(Map.of(), false), - arguments(Map.of("tagName", "A"), true), - arguments(Map.of("tagName", "BUTTON"), true), - arguments(Map.of("tagName", "INPUT", "tagType", "submit"), true), - arguments(Map.of("tagName", "INPUT", "tagType", "button"), true), - arguments(Map.of("tagName", "INPUT", "tagType", "text"), false), - arguments(Map.of("tagName", "FORM"), false)); + arguments(component(null, ""), false), + arguments(component("A", ""), true), + arguments(component("BUTTON", ""), true), + arguments(component("INPUT", "submit"), true), + arguments(component("INPUT", "button"), true), + arguments(component("INPUT", "text"), false), + arguments(component("FORM", ""), false)); } @Test void shouldNotSupportWhenHrefOutOfScope() { // Given - Map data = Map.of("tagName", "A", "href", "http://other.example.com/"); + ClientSideComponent component = + new ClientSideComponent( + Map.of(), + "A", + "", + "http://example.com", + "http://other.example.com/", + "", + Type.LINK, + "", + -1); Predicate scopeChecker = href -> false; // When / Then - assertThat(ClickElement.isSupported(scopeChecker, data), is(false)); + assertThat(ClickElement.isSupported(scopeChecker, component), is(false)); } @Test void shouldSupportWhenHrefInScope() { // Given - Map data = Map.of("tagName", "A", "href", "http://example.com/page"); + ClientSideComponent component = + new ClientSideComponent( + Map.of(), + "A", + "", + "http://example.com", + "http://example.com/page", + "", + Type.LINK, + "", + -1); Predicate scopeChecker = href -> true; // When / Then - assertThat(ClickElement.isSupported(scopeChecker, data), is(true)); + assertThat(ClickElement.isSupported(scopeChecker, component), is(true)); + } + + private static ClientSideComponent componentWithLocator( + String tagName, String locatorType, String locatorValue) { + ClientSideComponent c = component(tagName, ""); + c.setElementLocator(new ElementLocator(locatorType, locatorValue)); + return c; + } + + private static ClientSideComponent component(String tagName, String tagType) { + return new ClientSideComponent( + Map.of(), + tagName, + "", + "http://example.com", + null, + "", + tagName != null ? Type.LINK : Type.UNKNOWN, + tagType, + -1); } private static WebElement visibleElement() { diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/FollowGraphUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/FollowGraphUnitTest.java index cb644434499..03721baacf5 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/FollowGraphUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/FollowGraphUnitTest.java @@ -47,6 +47,7 @@ import org.openqa.selenium.WebElement; import org.zaproxy.addon.client.ExtensionClientIntegration; import org.zaproxy.addon.client.internal.ClientSideComponent; +import org.zaproxy.addon.client.internal.ElementLocator; import org.zaproxy.addon.client.internal.graph.ClientGraphVertex; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; @@ -264,9 +265,19 @@ private void assertCommonState(WebDriver wd, boolean result) { } private static ClientSideComponent createLinkComponent(String parentUrl, String href) { - Map data = Map.of("tagName", "A", "url", parentUrl, "href", href); - return new ClientSideComponent( - data, "A", null, parentUrl, href, null, ClientSideComponent.Type.LINK, null, -1); + ClientSideComponent component = + new ClientSideComponent( + Map.of(), + "A", + null, + parentUrl, + href, + null, + ClientSideComponent.Type.LINK, + null, + -1); + component.setElementLocator(new ElementLocator("xpath", "//a[@href='" + href + "']")); + return component; } private void addGraphEdge(String sourceUrl, ClientSideComponent component, String targetUrl) { diff --git a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/SubmitFormUnitTest.java b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/SubmitFormUnitTest.java index 998a803850f..1a716bc4da2 100644 --- a/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/SubmitFormUnitTest.java +++ b/addOns/client/src/test/java/org/zaproxy/addon/client/spider/actions/SubmitFormUnitTest.java @@ -49,6 +49,9 @@ import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.zaproxy.addon.client.internal.ClientSideComponent; +import org.zaproxy.addon.client.internal.ClientSideComponent.Type; +import org.zaproxy.addon.client.internal.ElementLocator; import org.zaproxy.addon.client.spider.ActionWaitStrategy; import org.zaproxy.addon.commonlib.ValueProvider; import org.zaproxy.zap.extension.stats.InMemoryStats; @@ -77,15 +80,15 @@ void tearDown() { } @Test - void shouldThrowIfElementDataIsNull() { + void shouldThrowIfComponentIsNull() { assertThrows(NullPointerException.class, () -> new SubmitForm(valueProvider, uri, null)); } @Test void shouldSubmitFormOnRun() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "0"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(0, "xpath", "//FORM"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); WebElement form = visibleElement(); given(wd.findElement(any(By.class))).willReturn(form); @@ -103,8 +106,8 @@ void shouldSubmitFormOnRun() { @Test void shouldFillInputsFromFormBeforeSubmitting() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "0"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(0, "xpath", "//FORM"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); WebElement form = visibleElement(); given(wd.findElement(any(By.class))).willReturn(form); @@ -131,8 +134,8 @@ void shouldFillInputsFromFormBeforeSubmitting() { @Test void shouldHandleSubmitExceptionGracefully() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "0"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(0, "xpath", "//FORM"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); WebElement form = visibleElement(); given(wd.findElement(any(By.class))).willReturn(form); @@ -149,8 +152,8 @@ void shouldHandleSubmitExceptionGracefully() { @Test void shouldIncrementStatsWhenFormNotFound() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "0"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(0, "xpath", "//FORM"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); given(wd.findElement(any(By.class))).willThrow(RuntimeException.class); @@ -166,8 +169,8 @@ void shouldIncrementStatsWhenFormNotFound() { @Test void shouldIncrementStatsWhenFormNotDisplayed() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "0"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(0, "xpath", "//FORM"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); WebElement form = mock(WebElement.class); given(wd.findElement(any(By.class))).willReturn(form); @@ -183,10 +186,10 @@ void shouldIncrementStatsWhenFormNotDisplayed() { } @Test - void shouldUseXpathWithFormIndexAndTagName() { + void shouldUseByFromElementLocator() { // Given - Map elementData = Map.of("tagName", "FORM", "formId", "1"); - SubmitForm action = new SubmitForm(valueProvider, uri, elementData); + ClientSideComponent component = formComponent(1, "xpath", "(//FORM)[2]"); + SubmitForm action = new SubmitForm(valueProvider, uri, component); WebDriver wd = mock(WebDriver.class); WebElement visibleForm = visibleElement(); ArgumentCaptor byCaptor = ArgumentCaptor.forClass(By.class); @@ -199,13 +202,14 @@ void shouldUseXpathWithFormIndexAndTagName() { // Then assertThat(result, is(equalTo(true))); assertThat(byCaptor.getValue(), is(By.xpath("(//FORM)[2]"))); + assertThat(stats.getStat("stats.client.spider.action.form.1"), is(1L)); } @ParameterizedTest @MethodSource("provideIsSupportedData") - void shouldReturnExpectedSupportedValue(Map data, boolean expected) { + void shouldReturnExpectedSupportedValue(ClientSideComponent component, boolean expected) { // When - boolean supported = SubmitForm.isSupported(data); + boolean supported = SubmitForm.isSupported(component); // Then assertThat(supported, is(expected)); @@ -213,10 +217,22 @@ void shouldReturnExpectedSupportedValue(Map data, boolean expect static Stream provideIsSupportedData() { return Stream.of( - arguments(Map.of("tagName", "FORM", "formId", "0"), true), - arguments(Map.of("tagName", "FORM"), false), - arguments(Map.of("tagName", "DIV", "formId", "0"), false), - arguments(Map.of(), false)); + arguments(componentForTag("FORM", Type.FORM, -1), true), + arguments(componentForTag("FORM", Type.FORM, 1), true), + arguments(componentForTag("DIV", Type.BUTTON, 0), false), + arguments(componentForTag("A", Type.LINK, 0), false)); + } + + private static ClientSideComponent formComponent( + int formId, String locatorType, String locatorValue) { + ClientSideComponent c = componentForTag("FORM", Type.FORM, formId); + c.setElementLocator(new ElementLocator(locatorType, locatorValue)); + return c; + } + + private static ClientSideComponent componentForTag(String tagName, Type type, int formId) { + return new ClientSideComponent( + Map.of(), tagName, "", "http://example.com", null, "", type, "", formId); } private static WebElement visibleElement() {