Bugfix Issue 1433 PlanImport Legacy Wrong Root Count Null - sebiboga/testlink-upgraded GitHub Wiki

Issue 1433 — planImport (legacy): well-formed XML with wrong root element ⇒ HTTP 500 count(null) at lib/plan/planImport.php:286

Issue: #1433 Branch: fix/issue-1433 Status: VERIFIED-FIXED (2026-09-11)
Screen lib/plan/planImport.php (legacy POST, Smarty page) — importTestPlanLinksFromXML()
Symptom Uploading a WELL-FORMED XML file whose root is not <testplan> (e.g. the old planner export root <xml> wrapping <testplan><executables>...) returns an empty HTTP 500 body.
Root cause xpath('//executables') (line 279) matches at ANY depth, but $xml->executables (line 285) only reaches a DIRECT child of the root; on non-<testplan> roots it resolves to null, and count(null) (line 286) is a PHP 8 TypeError → uncaught fatal → empty 500.
Fix $xmlLinks = $xml->executables instanceof SimpleXMLElement ? $xml->executables->children() : null; $loops2do = is_null($xmlLinks) ? 0 : count($xmlLinks); — non-<testplan> roots now yield $loops2do = 0 (empty import report), restoring the legacy PHP 5/7 behavior instead of fataling. Supported <testplan>-root imports unchanged. No i18n changes (no user-facing string).

planImport graceful empty result for wrong-root XML

Repro steps

  1. Log in as admin (POST /login.php, tl_login=admin&tl_password=admin, keep the session cookie).

  2. Fixture tmp/fixtures_pimp.php: tproject 1 PIMP, plan 12 PIMP-Plan, platform 1 PIMP-Android.

  3. curl -b cookies.txt -F "importType=XML" -F "uploadFile=1" -F "[email protected]" "http://localhost:8082/lib/plan/planImport.php?tplan_id=12" with wrongroot.xml:

    <?xml version="1.0"?>
<xml><testplan><executables><link>
  <platform><name>PIMP-Android</name></platform>
  <testcase><externalid>1</externalid><name>Login</name><version>1</version><execution_order>1</execution_order></testcase>
</link></executables></testplan></xml>

  4. Before fix: HTTP 500, empty body; fatal at lib/plan/planImport.php:286. After fix: HTTP 200 (import screen renders, empty import report).

Expected: graceful non-500 page, mirroring the modern BFF fix #1407.

Root cause

lib/plan/planImport.php:279-286:

if( $status_ok && $xml->xpath('//executables') )
{
    ...
    $xmlLinks = $xml->executables->children();
    $loops2do = count($xmlLinks);        // line 286 → count(null) on PHP 8

Chain:

  1. :279 — $xml->xpath('//executables') matches <executables> at any depth, so the block is entered for the <xml>-wrapped file.
  2. :285 — $xml->executables is a direct-child property access on the document root; root is <xml> whose direct child is <testplan>, so ->executables yields null → ->children() null.
  3. :286 — count(null) is a TypeError on PHP 8.x (PHP < 8 coerced null→0). Uncaught ⇒ request dies before the Smarty page renders.

Blast radius:

  • Only lib/plan/planImport.php:285-286 crashes. Wrong-root files with no <executables> at any depth were already safe (block not entered). Crash was specific to files where <executables> exists at depth (the <xml> wrapper / old planner exports).
  • Modern BFF twin api/planimport/index.php:145-151 had the identical pattern, already fixed by #1407 (commit 6b3aca759). This legacy copy was the remaining unguarded one; legacy screen is slated for removal via task #1409 (minimal patch only).

Fix (minimal)

lib/plan/planImport.php:285-289 — mirror the verified #1407 guard:

$xmlLinks = $xml->executables instanceof SimpleXMLElement
            ? $xml->executables->children() : null;
$loops2do = is_null($xmlLinks) ? 0 : count($xmlLinks);
  • $loops2do = 0 ⇒ loop skipped ⇒ function returns $msg normally ⇒ page renders with an empty import report — byte-equivalent to the legacy PHP 5/7 (count(null) = 0) behavior.
  • Supported <testplan>-root import path unchanged.
  • No i18n bundle changes (no user-facing string).

Rejected alternative: actually importing <xml>-wrapped planner exports (walking into $xml->testplan) — a new feature the legacy import never supported, out of scope for a minimal bug fix.

Verification (regression matrix, all PASS on localhost:8082)

  • primary repro (<xml> wrapper): pre-fix HTTP 500 + fatal at :286; post-fix HTTP 200, no fatal in tmp/php_server.log ([200]: POST /lib/plan/planImport.php).
  • valid <testplan>-root import: HTTP 200, report Test case link #1 has platform but Test Plan has no linked platforms: Not imported — legacy processing intact (fixture plan has zero platforms). Proves the supported path is untouched.
  • hygiene: php -l lib/plan/planImport.php clean; events table — no new Error/Warning (ERROR=1, WARNING=2 log_level), only pre-existing AUDIT(16) rows.
  • browser (chrome-devtools): login admin/admin → planImport.php?tplan_id=12 → upload wrongroot.xml → POST returns 200, empty import report. Screenshot: planImport-graceful-issue1433.png.
  • diff hygiene: only lib/plan/planImport.php touched (+8/-2), zero i18n bundles.

Discoveries while testing

  • The #1407 fix left the legacy twin unguarded — the same crash, one codebase later. Confirms the value of fixing the pattern once and carrying it to the twin.
  • 2.0.1 log levels: ERROR=1, WARNING=2, INFO=4, DEBUG=8, AUDIT=16 (lib/functions/logger.class.php:50-55) — used for the Event Viewer check.
  • Code-review subagent (AGENTS.md rule 16) reviewed the full diff before push.

Evidence

Commit range (fix), (regression suite), (docs + wiki) on branch fix/issue-1433. Docs mirror: docs/Bugfix-Issue-1433-PlanImport-Legacy-Wrong-Root-Count-Null.md, screenshot docs/screenshots/issue-1433-planimport-graceful.png. Regression suite: tmp/TLU_Test_Cases.md → "Regression — Issue #1433".

⚠️ **GitHub.com Fallback** ⚠️