Bugfix Issue 1407 PlanImport BFF Wrong Root Count Null - sebiboga/testlink-upgraded GitHub Wiki

Issue 1407 — planImport (modern): well-formed XML with wrong root element ⇒ HTTP 500 count(null) at api/planimport/index.php:151

Issue: #1407 Branch: fix/issue-1407 Status: VERIFIED-FIXED (2026-09-11)

Screen api/planimport/index.php (POST ?action=import) + gui/templates/plans/planImport.html
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 — no JSON error, nothing imported.
Root cause xpath('//executables') (line 145) matches at ANY depth, but $xml->executables (line 150) only reaches a DIRECT child of the root; on non-<testplan> roots it resolves to an empty node whose ->children() is null, and count(null) (line 151) 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 behave byte-identically to the already-graceful "no <executables> anywhere" case (status:ok + empty result_map). 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 /api/auth/login, JSON {"login":"admin","password":"admin"}).

  2. Fixture tmp/fixtures_pimp.php: tproject 1 PIMP, plan 12 PIMP-Plan, TCs Login/Logout/Settings (external ids 1/2/3, version 1, tcversions 4/7/10), platform 1 PIMP-Android (not yet linked).

  3. POST /api/planimport/?action=import&tproject_id=1&tplan_id=12 with uploadedFile = a well-formed file rooted at <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; tmp/php_server.log shows Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in .../api/planimport/index.php:151. After fix: HTTP 200, {"status":"ok","result_map":[],"tplan_name":"PIMP-Plan"}.

Root cause

if ($status_ok && $xml->xpath('//executables')) {          // :145 matches at ANY depth
    ...
    $xmlLinks = $xml->executables->children();             // :150 direct-child access only
    $loops2do = count($xmlLinks);                          // :151 count(null) → TypeError
  1. xpath('//executables') is depth-insensitive → block entered for <xml>-wrapped files.
  2. $xml->executables is a direct-child property access on the document root; root = <xml> (direct child <testplan>), so ->executables resolves to an empty phantom node whose ->children() is null.
  3. count(null) is a PHP 8 TypeError (PHP < 8 coerced null→0). Uncaught ⇒ the request dies before out() emits JSON, hence the empty 500 body. The graceful invalid-XML path (simplexml_load_file_wrapper_error, line 289) is never reached because parsing itself succeeds.

Blast radius: only :150-151 crashes. Wrong roots with no <executables> at any depth already returned graceful status:ok + empty result_map (else branch :282-287, legacy parity) — so the fix normalises the crashing case to the previously-safe ones. The legacy twin lib/plan/planImport.php:284-286 has the identical unguarded count($xml->executables->children()) and crashes on the same file → filed separately as #1433 (label bug).

Fix (minimal)

api/planimport/index.php:150-153:

$xmlLinks = $xml->executables instanceof SimpleXMLElement
            ? $xml->executables->children() : null;
$loops2do = is_null($xmlLinks) ? 0 : count($xmlLinks);

$loops2do = 0 skips the legacy loop body entirely; the route emits status:ok + empty result_map — byte-identical to every other wrong-structure input. Supported <testplan>-root imports are unchanged. No user-facing string → no i18n bundle changes. Rejected: (a) auto-navigating into $xml->testplan to import old planner exports — a new feature (legacy never supported it either), out of scope; (b) a new "wrong root" message — inconsistent with the silent handling of other wrong-root shapes, and heavy (new keys in all locale strings.txt).

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

# Case Result
1 Primary repro: <xml> wrapper (old planner format) HTTP 200 status:ok + empty result_map; no fatal (pre-fix: 500 + TypeError :151)
2 Arbitrary wrong root <foo><bar/></foo> HTTP 200 ok, empty result_map (unchanged)
3 Valid <testplan>-root import, one create-link HTTP 200, Platform PIMP-Android has been linked to test plan. + Test Case with external id 1 version 1 has been linked to Test Plan for Platform PIMP-Android; DB testplan_tcversions row (tcversion 4, platform 1)
4 Valid <testplan>-root, no <executables> HTTP 200, empty result_map (unchanged)
5 Invalid XML (parse fail) HTTP 200, simplexml_load_file_wrapper_error / Not imported (unchanged)
6 Browser: planImport.html → upload wrong-root file Import report "No import results to display."; POST 200; zero console errors
7 Hygiene php -l clean; events table only AUDIT/LOGIN/CREATE (no Error/Warning); no fatal logged after the fix

Regression suite: tmp/TLU_Test_Cases.md → Regression — Issue #1407 — 8/8 PASS.

Discoveries while testing

  • The crash was reachable only when <executables> exists at an unexpected depth; a wrong root without <executables> was already graceful — the fix therefore makes the crashing case identical to the safe ones rather than adding special handling.
  • The legacy twin crashes on the same file (lib/plan/planImport.php:286) — filed as #1433 (label bug), out of this run's one-issue scope.
  • Code-review subagent APPROVED; its single LOW note (under PHP 8.3 the ->executables property access yields a phantom empty node, so the children() call — not the property — is the null) was incorporated into the NOTE comment; the fix is correct for both mechanisms because is_null($xmlLinks) catches the null ->children() result.

Evidence

Commit range (fix), (regression suite), (docs + wiki) on branch fix/issue-1407. Docs mirror: docs/Bugfix-Issue-1407-PlanImport-BFF-Wrong-Root-Count-Null.md + docs/screenshots/issue-1407-planimport-graceful.png.

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