Merge pull request #5772 from GunoH/issue-8460

reports: fix missing alert details in Risk and Confidence HTML
zaproxy · Sep 30, 2024 · 8b19457 · 8b19457
2 parents 46a8ed1 + e8b52a2
commit 8b19457
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 9 deletions.
diff --git a/addOns/reports/CHANGELOG.md b/addOns/reports/CHANGELOG.md
@@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Checkmarx rebrand.
 
+### Fixed
+- An issue where alert details were missing from some Risk and Confidence HTML reports (Issue 8460).
+
 ## [0.33.0] - 2024-09-02
 ### Changed
 - Maintenance changes related to Passive Scanner add-on (Issue 7959).

diff --git a/addOns/reports/src/main/java/org/zaproxy/addon/reports/ReportHelper.java b/addOns/reports/src/main/java/org/zaproxy/addon/reports/ReportHelper.java
@@ -19,11 +19,14 @@
  */
 package org.zaproxy.addon.reports;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.text.StringEscapeUtils;
 import org.parosproxy.paros.Constant;
 import org.parosproxy.paros.control.Control;
@@ -73,18 +76,27 @@ public static int getPortForSite(String site) {
         if (site == null) {
             return 80;
         }
-        String[] schemeHostPort = site.split(":");
-        if (schemeHostPort.length == 3) {
-            try {
-                return Integer.parseInt(schemeHostPort[2]);
-            } catch (NumberFormatException e) {
-                // Ignore
+
+        try {
+            var uri = new URI(site);
+            int port = uri.getPort();
+            if (port != -1) {
+                return port;
             }
+
+            return getPortFromScheme(site);
+
+        } catch (URISyntaxException e) {
+            return getPortFromScheme(site);
         }
-        if (schemeHostPort[0].equalsIgnoreCase("https")) {
+    }
+
+    private static int getPortFromScheme(String site) {
+        if (StringUtils.startsWithIgnoreCase(site, "https")) {
             return 443;
+        } else {
+            return 80;
         }
-        return 80;
     }
 
     public static boolean isSslSite(String site) {
@@ -129,7 +141,7 @@ public static List<Alert> getAlertsForSite(AlertNode rootNode, String site) {
     }
 
     /**
-     * @deprecated Use {@link getAlertInstancesForSite(AlertNode, String, String int)} instead -
+     * @deprecated Use {@link #getAlertInstancesForSite(AlertNode, String, String, int)} instead -
      *     this method can return the instances for different alerts with the same pluginId.
      */
     @Deprecated

diff --git a/addOns/reports/src/test/java/org/zaproxy/addon/reports/ReportHelperUnitTest.java b/addOns/reports/src/test/java/org/zaproxy/addon/reports/ReportHelperUnitTest.java
@@ -59,13 +59,21 @@ void shouldGetPortForSites() throws Exception {
         // Given / When / Then
         assertThat(ReportHelper.getPortForSite("https://www.example.com:443"), is(443));
         assertThat(ReportHelper.getPortForSite("https://www.example.com:8443"), is(8443));
+        assertThat(
+                ReportHelper.getPortForSite("https://www.example.com:8443/some/path/"), is(8443));
         assertThat(ReportHelper.getPortForSite("https://www.example.com:8080"), is(8080));
         assertThat(ReportHelper.getPortForSite("http://www.example.com:8080"), is(8080));
+        assertThat(ReportHelper.getPortForSite("http://www.example.com:8080/some/path/"), is(8080));
         assertThat(ReportHelper.getPortForSite("https://www.example.com"), is(443));
         assertThat(ReportHelper.getPortForSite("http://www.example.com"), is(80));
+        assertThat(ReportHelper.getPortForSite("HTTPS://www.example.com"), is(443));
+        assertThat(ReportHelper.getPortForSite("HTTP://www.example.com"), is(80));
+        assertThat(ReportHelper.getPortForSite("http://www.example.com/some/path"), is(80));
         assertThat(ReportHelper.getPortForSite("www.example.com"), is(80));
         assertThat(ReportHelper.getPortForSite("https://www.example.com:bad"), is(443));
         assertThat(ReportHelper.getPortForSite("http://www.example.com:bad"), is(80));
+        assertThat(ReportHelper.getPortForSite("HTTPS://www.example.com:bad"), is(443));
+        assertThat(ReportHelper.getPortForSite("HTTP://www.example.com:bad"), is(80));
         assertThat(ReportHelper.getPortForSite(null), is(80));
     }