[Maffooch]

Description

The Qualys XML parser looked up each finding's QID details with tree.find(".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS[@id='...']"). That search starts at the document root and walks every host before it reaches the glossary, which sits at the end of the report. So a parse cost (number of findings) x (size of the report), and grew quadratically with the number of hosts.

The glossary is now indexed once per report (vuln_details_by_id), and each finding reads its entry from that index. The first entry for an id still wins, as the search returned it.

One behavior changes: the old search interpolated the QID into an XPath predicate, so a QID containing an apostrophe made the whole parse fail with SyntaxError: invalid predicate. It now parses, because the id is a dictionary key rather than part of a path.

Measured on the sample report with its hosts repeated (unittests/scans/qualys/Qualys_Sample_Report.xml). Old and new parser ran interleaved in one process, best CPU time of 5:

hosts	findings	element-iteration steps before / after	CPU before / after
7	301	919,254 / 5,010	1.98 s / 1.53 s
14	602	3,647,518 / 8,015	3.50 s / 2.01 s
28	1,204	14,531,076 / 14,025	12.09 s / 6.56 s
56	2,408	58,006,312 / 26,045	40.00 s / 14.13 s

After the change, iteration steps equal the number of elements in the document. Total Python calls for the 56-host parse fell from 74.6M to 16.0M. The parsed findings are identical to before on every size above: title, severity, description, mitigation, impact, references, date, CVSS, status, vulnerability ids and endpoints were all compared.

End to end, through POST /api/v2/reimport-scan/ on the 56-host report (a reimport where half the hosts changed, group_by=finding_title, close_old_findings), with old and new parser run alternately on identical data: CPU per request fell from 65.3 / 65.6 / 68.6 s to 29.0 / 27.2 / 28.2 s across three rounds, with the same query counts.

Test results

• New test_glossary_is_searched_once_per_report_not_once_per_finding: the number of QID-details lookups must not grow from a 2-host report to a 14-host report. It counts calls to xml.etree.ElementPath.iterfind, which every find, findall, findtext and iterfind goes through, on a tree or an element, so a per-QID lookup fails it wherever the search starts. It fails on the old parser (45 vs 602) and on a variant that searches from the root element once per finding (45 vs 602), and passes now. It counts lookups rather than timing them, so it does not depend on machine load.
• New test_repeated_glossary_entry_keeps_the_first: pins that a repeated glossary id resolves to its first entry, as before. It passes on both the old and the new parser.
• All 14 tests in unittests/tools/test_qualys_parser.py pass. Ruff 0.16.10 is clean on both files.

Documentation

No user-facing change; nothing to document.

Checklist

• Make sure to rebase your PR against the very latest dev.
• Submit all PRs, features and bug fixes alike, against the dev branch.
• Give a meaningful name to your PR, as it may end up being used in the release notes.
• Your code is Ruff compliant (see ruff.toml).
• Your code is python 3.13 compliant.
• If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
• Model changes must include the necessary migrations in the dojo/db_migrations folder.
• Add applicable tests to the unit tests.
• Add the proper label to categorize your PR.

🤖 Generated with Claude Code

[Maffooch]

Adversarial review (full tier, backend parser only): changes requested

Reviewed head 932b7569ea against base dev. The change is correct and the speedup is real. There are two small this-pr items to fix before merge (R1, R2) and one pre-existing bug, which is now filed separately (R3).

What I ran

Output identity, old vs new parser. I loaded both parsers side by side in one process and compared every finding field in order: title, severity, description, mitigation, impact, references, date, mitigated, is_mitigated, active, cvssv3, cvssv3_score, verified, vuln_id_from_tool, vulnerability ids and endpoints. Each input ran under three settings: the defaults, USE_FIRST_SEEN=True, and USE_QUALYS_LEGACY_SEVERITY_PARSING=False. Findings were compared as ordered lists, so a change in order would show as a diff. 20 inputs × 3 settings = 60 runs, and 57 were identical:

• Samples: Qualys_Sample_Report.xml (301 findings) and empty.xml (0).
• Duplicate QIDs: a shadow entry appended to the glossary, a shadow entry prepended, and a whole second GLOSSARY block after the first. All match, so first-entry-wins holds in document order.
• Odd ordering: the glossary placed before HOST_LIST, nested two levels deeper, or a second glossary inside a HOST that shadows the real one. All match.
• Namespaces: a default namespace on the root (0 findings both), and a default-namespaced or prefixed GLOSSARY (same exception both).
• Missing data: a deleted entry, an empty glossary, no glossary at all, a VULN_DETAILS with no id, and an id padded with a space. All raise the same exception on old and new (see R3).
• A QID containing ]: same output on both.
• Hosts repeated to 14 and 56 (602 and 2,408 findings): identical.
• The 3 differing runs are one input under all three settings (see R2).

Complexity, counted with cProfile rather than timed. Total Python calls, old / new:

• 2 hosts: 254k / 226k
• 7 hosts: 2.98M / 2.00M
• 14 hosts: 7.73M / 3.96M
• 28 hosts: 22.7M / 7.86M
• 56 hosts: 74.2M / 15.7M

New is linear at about 280k calls per host; old grows quadratically. Best-of-3 CPU at 56 hosts on a loaded machine was 36.4 s old against 15.3 s new, which matches the PR's numbers.

Memory. tracemalloc peak at 56 hosts: 28.0 MB old, 27.6 MB new. The index only holds references to elements already in the tree. A 300-host, 17.9 MB report (12,823 findings) parses on the new code at a 138 MB peak.

XML security. defusedxml.ElementTree.parse is unchanged, and the PR adds no new parse. An external file entity and a billion-laughs payload both raise EntitiesForbidden on old and new. The external-DTD-only sample header parses the same, with the network disabled.

Tests. I ran the test module's 14 tests against three parsers:

• Old parser: only test_glossary_is_searched_once_per_report_not_once_per_finding fails (45 != 602), as claimed.
• New parser: all 14 pass.
• A deliberately regressed parser (see R1): all 14 pass.

The 14 tests were called directly, without the DB test runner, because none of them touch the database. CI is green on the head commit. Ruff 0.16.10 (the requirements-lint.txt pin) is clean on both files.

Findings

R1: suggestion, scope this-pr. The new guard test misses the regression it is meant to catch if the per-finding search moves to an Element. It patches only ElementTree.find/findall/iterfind, the tree-level methods. I changed qualys_parser so each finding looked its QID up with tree.getroot().find(".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS[@id='...']"). That brings back the findings × document cost exactly, and all 14 tests still pass.

Suggested fix: patch xml.etree.ElementPath.iterfind instead. Both the C Element methods and the ElementTree methods route path searches through it, and ElementPath.find calls it too. With that one change, the same guard reports old 45 vs 602 (fail), the regressed variant 45 vs 602 (fail), and this PR 1 vs 1 (pass).

R2: nit, scope this-pr. The description says the parsed output is identical to before. There is one input where it is not: a QID id containing an apostrophe. The old code put the id into an XPath predicate, so such an id made the import fail with SyntaxError: invalid predicate. The new code parses it, and that run returns 301 findings. This is an improvement, and real Qualys ids are qid_<number>, so it is unlikely in practice. Please note it in the description so the behavior change is on record.

R3: blocker-class bug, scope pre-existing (not this PR's to fix), filed as #16633. I confirmed the suspected missing-glossary bug on dev and also on this PR, where it is unchanged. When any finding's QID has no glossary entry, get_severity(vuln_item.findtext("SEVERITY")) runs outside the if vuln_item is not None guard. It raises AttributeError: 'NoneType' object has no attribute 'findtext', and the whole import fails.

These trigger it: a deleted entry, an empty or missing glossary, a namespaced glossary, a VULN_DETAILS without an id, or an id with stray whitespace. The other suspected symptom, repeating the previous finding's details, does not happen: vuln_item is reassigned on every iteration, so the parser always raises before that could occur.

Verdict

Changes requested. Fix R1 and R2 on this branch. R3 is tracked in #16633. Not approved and not merged by this review.

[Maffooch]

Resolutions:

• R1: fixed in e26b5e0. The guard now counts calls to xml.etree.ElementPath.iterfind (every find/findall/findtext/iterfind on a tree or an element goes through it). Checked three ways: the old parser fails (45 vs 602), a variant that looks each QID up from the root element fails (45 vs 602), and this PR passes (14/14 in unittests/tools/test_qualys_parser.py).
• R2: fixed in the PR description. It now records that a QID containing an apostrophe used to abort the parse with SyntaxError: invalid predicate and now parses.
• R3: issue Qualys XML parser crashes when a QID has no glossary entry #16633 (pre-existing, left out of this PR).