perf(qualys): parse large Qualys XML reports in linear time

dojo/tools/qualys/parser.py

@@ -201,7 +201,7 @@ def split_cvss(value, _temp):
         _temp["CVSS_value"] = float(value)
 
 
-def parse_finding(host, tree):
+def parse_finding(host, glossary):
     ret_rows = []
     issue_row = {}
 
@@ -265,8 +265,7 @@ def parse_finding(host, tree):
                 # DefectDojo does not support cvssv2
                 temp["CVSS_vector"] = None
 
-        search = f".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS[@id='{gid}']"
-        vuln_item = tree.find(search)
+        vuln_item = glossary.get(gid)
         if vuln_item is not None:
             finding = Finding()
             # Vuln name
@@ -373,6 +372,21 @@ def qualys_parser(qualys_xml_file):
     host_list = tree.find("HOST_LIST")
     finding_list = []
     if host_list is not None:
+        glossary = vuln_details_by_id(tree)
         for host in host_list:
-            finding_list += parse_finding(host, tree)
+            finding_list += parse_finding(host, glossary)
     return finding_list
+
+
+def vuln_details_by_id(tree):
+    """
+    The report's glossary entries by QID, read once per report.
+
+    Looking a QID up with a search from the document root walks every host before it reaches the
+    glossary, so doing that for each finding made a parse cost findings x report size. The first
+    entry for an id wins, as that search returned it.
+    """
+    glossary = {}
+    for vuln_item in tree.iterfind(".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS"):
+        glossary.setdefault(vuln_item.get("id"), vuln_item)
+    return glossary

unittests/tools/test_qualys_parser.py

@@ -1,4 +1,8 @@
 import datetime
+import io
+import re
+from unittest import mock
+from xml.etree import ElementPath
 
 from django.test import override_settings
 
@@ -7,8 +11,63 @@
 from unittests.dojo_test_case import DojoTestCase, get_unit_tests_scans_path
 
 
+def _report_with_hosts(count):
+    """The sample report with its hosts repeated to ``count`` hosts, each with its own address."""
+    sample = (get_unit_tests_scans_path("qualys") / "Qualys_Sample_Report.xml").read_text(encoding="utf-8")
+    head, rest = sample.split("<HOST_LIST>", 1)
+    hosts, tail = rest.split("</HOST_LIST>", 1)
+    blocks = re.findall(r"<HOST>.*?</HOST>", hosts, flags=re.DOTALL)
+    repeated = [
+        re.sub(r"<IP>[^<]*</IP>", f"<IP>192.0.2.{index + 1}</IP>", blocks[index % len(blocks)], count=1)
+        for index in range(count)
+    ]
+    return _named(f"{head}<HOST_LIST>{''.join(repeated)}</HOST_LIST>{tail}")
+
+
+def _named(content):
+    report = io.BytesIO(content.encode())
+    report.name = "report.xml"
+    return report
+
+
 class TestQualysParser(DojoTestCase):
 
+    def test_glossary_is_searched_once_per_report_not_once_per_finding(self):
+        # Every finding used to look its QID up with a search from the document root, which walks
+        # all the hosts before it reaches the glossary: the parse was findings x document size.
+        # Every find, findall, findtext and iterfind, on an element or on the whole tree, resolves its
+        # path through ElementPath.iterfind, so a per-QID lookup from any starting point is counted.
+        searches = []
+        original = ElementPath.iterfind
+
+        def counting(elem, path, namespaces=None):
+            if "VULN_DETAILS" in path:
+                searches.append(path)
+            return original(elem, path, namespaces)
+
+        patcher = mock.patch.object(ElementPath, "iterfind", counting)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+        small = QualysParser().get_findings(_report_with_hosts(2), Test())
+        small_searches = len(searches)
+        searches.clear()
+        large = QualysParser().get_findings(_report_with_hosts(14), Test())
+
+        self.assertGreater(len(large), len(small))
+        self.assertEqual(small_searches, len(searches))
+
+    def test_repeated_glossary_entry_keeps_the_first(self):
+        sample = (get_unit_tests_scans_path("qualys") / "Qualys_Sample_Report.xml").read_text(encoding="utf-8")
+        details = re.search(r"<VULN_DETAILS id=\"qid_6\">.*?</VULN_DETAILS>", sample, flags=re.DOTALL).group(0)
+        shadow = re.sub(r"<TITLE><!\[CDATA\[[^\]]*\]\]></TITLE>", "<TITLE><![CDATA[Shadowed]]></TITLE>", details, count=1)
+        report = _named(sample.replace("</VULN_DETAILS_LIST>", f"{shadow}</VULN_DETAILS_LIST>", 1))
+
+        titles = {finding.title for finding in QualysParser().get_findings(report, Test())}
+
+        self.assertIn("QID-6 | DNS Host Name", titles)
+        self.assertNotIn("QID-6 | Shadowed", titles)
+
     @override_settings(USE_FIRST_SEEN=True)
     def test_parse_file_with_no_vuln_has_no_findings_first_seen(self):
         self.parse_file_with_no_vuln_has_no_findings()