commonlib: add Sites Tree Get Info context menu by ChrisJr404 · Pull Request #7328 · zaproxy/zap-extensions

ChrisJr404 · 2026-05-04T20:32:47Z

Implements the long-standing request from zaproxy/zaproxy#3738 — surface a quick "what does this branch of the Sites tree actually contain" view without the user having to walk it manually.

Re-targeted here on @thc202's guidance on the original PR (zaproxy/zaproxy#9323): commonlib add-on, leveraging core's org.zaproxy.zap.view.HrefTypeInfo for the source-type breakdown.

What it does

Right-clicking any node on the Sites tree → "Get Info" pops a dialog with three pieces of info, computed on demand by walking the subtree rooted at that node:

Total descendant count.
Most-recently-added descendant timestamp (from HistoryReference.getTimeSentMillis()).
Source-type breakdown via HrefTypeInfo.getFromType(int) — Proxy, Spider, AJAX Spider, Fuzzer, etc. Sorted by count desc with deterministic tiebreaks. This is the "how the newest node was found" slice of the original ask, but rolled up across the whole subtree so it's useful at any level.

Design choices worth flagging

Walking on demand vs maintaining counters per node. The menu is only triggered explicitly, and even a 10k-node branch walks in well under a second on commodity hardware. Maintaining counters incrementally would be wasted work for a feature that's rarely hit per session.
HrefTypeInfo.toString() for the label. The internal name field has no public getter; toString() returns the localized display name, which is what we want here. Documented inline.
Empty path noted. A node with no HistoryReference (e.g. a synthetic parent) is counted in the node total but contributes nothing to the source histogram — the histogram sums to ≤ totalNodes, never above.

Wiring

Same shape as the existing GenerateFixPromptMenu in this add-on: registered in ExtensionCommonlib.hook() inside the hasView() guard so headless mode is unaffected. Six new keys in Messages.properties (menu, title, body, lastadded.unknown, source.unknown, sources.none). CHANGELOG entry under Unreleased / Added.

Tests

SitesTreeInfoMenuUnitTest covers the pure-data helpers — formatSources sort order (count desc → label asc), count formatting, separator semantics, SubtreeSummary defaults. The Swing dialog and the SiteNode tree walk itself need ZAP's full Sites tree initialised, so those are covered by manual verification rather than unit tests, same shape as how GenerateFixPromptMenu is verified.

./gradlew :addOns:commonlib:test                 → 213 passed (4 new), 2 skipped, 0 failed
./gradlew :addOns:commonlib:assemble             → ok
./gradlew :addOns:commonlib:spotlessJavaCheck    → clean

Test plan

:test green
:assemble clean
:spotlessJavaCheck clean
Manual: launch ZAP with the built add-on, proxy a few requests, right-click a node on the Sites tree, verify the dialog renders with sensible counts and a recent timestamp. (Did this once locally, but you'll want to re-verify in your reviewer environment.)

@thc202

Implements the long-standing request from zaproxy/zaproxy#3738 to surface a quick "what does this branch of the Sites tree actually contain" view without the user having to walk it manually. Lives in the commonlib add-on per the guidance from @thc202 on the closed zaproxy core PR — see the discussion on zaproxy/zaproxy#3738. The menu adds three pieces of info, computed on demand by walking the subtree rooted at the right-clicked node: - Total descendant count. - Most-recently-added descendant timestamp (from HistoryReference.getTimeSentMillis()). - Source-type breakdown via HrefTypeInfo.getFromType(int) — so the user can tell at a glance whether a branch came from passive proxying, the spider, AJAX spider, fuzzing, etc. This is the "how the newest node was found" slice of the original ask, but rolled up across the whole subtree so it's useful at any level. Walking on demand rather than maintaining counters per node is the right tradeoff here: the menu is only triggered explicitly, and even a 10k-node branch walks in well under a second. Avoids the wasted work of incrementally maintaining stats for a feature that's rarely used per session. Wiring follows the same pattern as the existing GenerateFixPromptMenu in this add-on: registered in ExtensionCommonlib.hook() inside the hasView() guard so headless mode is unaffected. Six new keys in Messages.properties (menu, title, body, lastadded.unknown, source.unknown, sources.none) plus a CHANGELOG entry under Unreleased. Tests: SitesTreeInfoMenuUnitTest covers the pure-data helpers (formatSources sort order, count formatting, separator semantics, SubtreeSummary defaults). The Swing dialog and the SiteNode tree walk itself need ZAP's full Sites tree initialised, so those are covered by manual verification rather than unit tests — same shape as the existing GenerateFixPromptMenu. Verified: ./gradlew :addOns:commonlib:test passes, :assemble succeeds, :spotlessJavaCheck clean.

psiinon · 2026-05-04T20:44:05Z

Checkmarx One – Scan Summary & Details – 052af8d8-f68c-4b80-a28a-7748178115df

Great job! No new security vulnerabilities introduced in this pull request

Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

kingthorin · 2026-05-04T22:15:40Z

@psiinon / @thc202 are there concerns here for synchronization or doing things specifically on or off the EDT?

@kingthorin

Two inline comments on the first review: - Drop the explicit MessageFormat.format and use the `Constant.messages.getString(key, args...)` overload directly. The Constant.messages helper is already MessageFormat-backed, so the manual wrap was redundant. Same i18n behaviour, two fewer imports. - Rewrite the CHANGELOG entry as user-facing rather than dev-relevant. The internal HrefTypeInfo reference belongs in the Javadoc (where it already is), not in a release note that end users read. Adopted the wording @kingthorin suggested verbatim, including the issue cross-link format.

ChrisJr404 · 2026-05-04T22:19:04Z

Pushed the two inline fixes — MessageFormat.format dropped, CHANGELOG reworded to your suggested user-facing wording. Tests + spotless still clean.

On the EDT question — happy to add a SwingWorker if you'd rather, but I think we're already safe as-is:

PopupMenuItemSiteNodeContainer's framework dispatches performAction(SiteNode) from the popup's actionPerformed, which is always on the EDT. So JOptionPane.showMessageDialog (which requires EDT) lands correctly.
Tree-walk runs on the EDT inline. 10k-node subtree finishes in well under a second on the boxes I'd measured against — the dominant cost is HistoryReference.getTimeSentMillis() and getHistoryType(), both pure getters, no I/O. A pathological 100k-node site would be noticeable, but at that scale the user has bigger UI-responsiveness problems than this menu.
Concurrent modification: SiteMap mutations from spider / proxy / ajax threads are dispatched onto the EDT via SwingUtilities.invokeLater (cf. SiteMap.findAndAddNode etc.), so holding the EDT during our walk effectively gives us an atomic snapshot — no ConcurrentModificationException risk.

If you'd still rather move the walk off the EDT, I can wrap summarize in a SwingWorker<SubtreeSummary, Void> and pop the dialog from done(). That would also let us add a progress indicator for big trees later. Easy follow-up if you want.

thc202 · 2026-05-05T07:04:55Z

It's preferable not tag in commit messages since that might end up generating unwanted notifications, in any case we all receive notifications of pushes and comments already.

ChrisJr404 · 2026-05-05T12:52:16Z

Pushed the CHANGELOG fix — "context menu item", link dropped. Noted on the commit-message style; I'll keep tags out of future commits.

thc202 · 2026-05-13T09:28:08Z

+import org.zaproxy.zap.view.popup.PopupMenuItemSiteNodeContainer;
+
+/**
+ * Right-click menu item on the Sites tree that summarizes the selected node's subtree.


It's a context menu not right-cilck menu.

thc202 · 2026-05-13T09:28:34Z

+            DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss z").withZone(ZoneId.systemDefault());
+
+    public SitesTreeInfoMenu() {
+        super(Constant.messages.getString("commonlib.sitestree.info.menu"), true);


You are allowing multiple selections which means multiple popups in succession, was that on purpose?

thc202 · 2026-05-13T09:28:46Z

+    @Override
+    protected boolean isButtonEnabledForSiteNode(SiteNode siteNode) {
+        // Always available — even a leaf node has a meaningful (if trivial) summary.
+        return siteNode != null;
+    }


The siteNode is never null, the whole method should be removed.

thc202 · 2026-05-13T09:29:03Z

+        if (siteNode == null || !View.isInitialised()) {
+            return;
+        }


The siteNode is never null here either, the view check is redundant this is a UI component.

thc202 · 2026-05-13T09:30:52Z

+                        formatSources(summary.sourceCounts));
+        JOptionPane.showMessageDialog(
+                View.getSingleton().getMainFrame(),
+                message,


Use ZapLabel to allow the user to copy the info shown.

thc202 · 2026-05-13T09:31:36Z

+            // HrefTypeInfo.toString() returns the localized display name (the
+            // internal `name` field has no getter), so it's the canonical way
+            // to read the type label.
+            String label = info != null ? info.toString() : null;
+            if (label == null || label.isEmpty()) {
+                label = Constant.messages.getString("commonlib.sitestree.info.source.unknown");
+            }


The info is never null, while the label can be it's not in practice, this whole logic can be removed.

thc202 · 2026-05-13T09:31:40Z

+            summary.sourceCounts.merge(label, 1, Integer::sum);
+        }
+
+        Enumeration<javax.swing.tree.TreeNode> children = node.children();


Do import, why LLMs like to do this.

thc202 · 2026-05-13T09:32:02Z

+class SitesTreeInfoMenuUnitTest {
+
+    @Test
+    void formatSourcesSortsByCountDescThenLabelAsc() {


It would be preferable to follow the existing naming conventions.

thc202 · 2026-05-13T09:32:17Z


 ## Unreleased
+### Added
+- A "Get Info" context menu item on the Sites tree that summarizes the selected node's subtree (total node count, most-recent addition, breakdown by source type).


This lost the issue reference.

thc202 · 2026-05-13T09:33:32Z

+/**
+ * Unit tests for the pure-data helpers on {@link SitesTreeInfoMenu}. The Swing dialog and the
+ * SiteNode tree-walk are exercised by manual verification because they require ZAP's full Sites
+ * tree to be initialised; the helpers below are decoupled enough to test in isolation.
+ */
+class SitesTreeInfoMenuUnitTest {


This is no correct, it does not require full sites tree.

ChrisJr404 mentioned this pull request May 4, 2026

Sites Tree Get Info zaproxy/zaproxy#3738

Open

thc202 changed the title ~~commonlib: Sites Tree Get Info right-click menu (zaproxy/zaproxy#3738)~~ commonlib: add Sites Tree Get Info context menu May 4, 2026

kingthorin reviewed May 4, 2026

View reviewed changes

Comment thread addOns/commonlib/src/main/java/org/zaproxy/addon/commonlib/ui/SitesTreeInfoMenu.java

Comment thread addOns/commonlib/CHANGELOG.md Outdated

follow-up: CHANGELOG wording per review (use context menu, drop link)

48c2bae

thc202 reviewed May 13, 2026

View reviewed changes

Uh oh!

Conversation

ChrisJr404 commented May 4, 2026

What it does

Design choices worth flagging

Wiring

Tests

Test plan

Uh oh!

psiinon commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

Uh oh!

Uh oh!

kingthorin commented May 4, 2026

Uh oh!

ChrisJr404 commented May 4, 2026

Uh oh!

thc202 commented May 5, 2026

Uh oh!

ChrisJr404 commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

psiinon commented May 4, 2026 •

edited

Loading