Bugfix Issue 1739 reqEdit Unrenderable Action - sebiboga/testlink-upgraded GitHub Wiki

Bugfix: #1739 — legacy reqEdit.php refuses unrenderable doAction with a 302 and no Event Viewer noise

Issue: #1739 Branch: fix/issue-1739-reqedit-dispatch Status: VERIFIED-FIXED — regression suite Issue #1739 18/18 PASS Direct successor of: #1736, which removed the identical method_exists() dispatch from lib/requirements/reqSpecEdit.php; this is the last remaining occurrence in lib/ (the other, planMilestonesEdit.php, was retired by #1726 / #1894).

Symptom

The legacy Requirement editor lib/requirements/reqEdit.php answered an unrecognised (or absent) doAction with a blank HTTP 200 (0-byte body) plus 4 E_WARNING rows in the Event Viewer, and answered a doAction that names an internal helper with a silent HTTP 500 (0 bytes, 0 log rows). A completely undiagnosable failure: no page, no error, nothing to grep.

Investigation (measured before any code was touched)

URL HTTP bytes new events rows
/lib/requirements/reqEdit.php (bare) 200 0 4 × E_WARNING (reqEdit.php:160,172,173)
?doAction=xyz 200 0 4 × E_WARNING
?doAction=simpleCompare 500 0 0
?doAction=initGuiBean 200 0 4 × E_WARNING
?doAction=initGuiObjForAttachmentOperations 200 0 4 × E_WARNING
?doAction[]=x 200 0 4 × E_WARNING
?doAction=create&req_spec_id=1 (control) 200 20518 normal

Fixture for the run: fresh (empty) DB; test project id=1 BugFix Project/FIX1739 created with POST /api/projectedit?action=create + node nodes_hierarchy.id=1,node_type_id=1; session pinned with GET /index.php?tproject_id=1.

Root cause chain

# file:line what happens
1 lib/requirements/reqEdit.php:41 if(method_exists($commandMgr,$pFn)) — matches every method of reqCommands, internal helpers and private members included; not a GUI-action whitelist
2 lib/requirements/reqEdit.php:42 $op = $commandMgr->$pFn($args,$_REQUEST); — fixed 2-arg signature; simpleCompare needs 4 → ArgumentCountError → 500 / 0 bytes / 0 rows
3 lib/requirements/reqEdit.php:160,172,173 renderGui()'s default: / $guiObj->askForRevision dereference $opObj->suggest_revision / $opObj->prompt_for_log on a null $opObj → 4 E_WARNING rows + the blank 200
4 lib/requirements/reqEdit.php:79 doAction declared tlInputParameter::STRING_N; a non-scalar ?doAction[]=x was dispatched, not refused

Why it breaks now: same family as #1736. #1736 gave reqSpecEdit.php a real whitelist + a graceful 302 refusal (commit e24d3b83f); #1726/#1894 retired planMilestonesEdit.php. reqEdit.php was left behind, so this was the last live method_exists($commandMgr,$pFn) dispatch in lib/.

The fix (1 file, +162 / −1)

lib/requirements/reqEdit.php:

reqEditDropShapedAction();                       // C3: drop a non-scalar ?doAction[]=x
...
if(is_string($pFn) && $pFn !== '' && isset(reqEditActionWhitelist()[$pFn])) {
  $op = $commandMgr->$pFn($args,$_REQUEST);      // C1: only names renderGui() can dispatch
} else {
  reqEditRefuseUnknownAction($pFn);              // C2: fixed-literal INFO + 302 to reqSpecMgmt
}
renderGui($args,$gui,$op,$templateCfg,$editorCfg,$db);
  • C1 reqEditActionWhitelist() returns the exact 24-case list of renderGui()'s GUI-rendering switch (reqEdit.php:198-221) — the set the legacy templates actually ask for (reqSpecView.tpl, reqViewVersions.tpl, reqReorder.tpl, reqEdit.tpl). Arity and visibility are guaranteed by construction.
  • C2 reqEditRefuseUnknownAction() writes one tLog(..., 'INFO') row with a fixed literal message (the attacker-controlled value is deliberately NOT interpolated) and 302s to gui/templates/requirements/reqSpecMgmt.html with tproject_id carried forward. INFO does not persist below WARNING, so a crafted URL cannot write an Error/Warning row.
  • C3 reqEditDropShapedAction() unsets a non-scalar $_REQUEST['doAction'] before R_PARAMS(), so the whitelist refuses the array with the same 302.

Why this method (and what was rejected)

  • The whitelist, not method_exists() — the only way to guarantee the invoked name has the ($args,$_REQUEST) shape and public visibility.
  • 302, never blank — matches the house contract #1627/#1722 established for the sibling editors.
  • INFO, not ERROR — a crafted query string must not be able to write Event Viewer noise; the refusal is silent apart from the redirect (measured 0 rows).
  • Rejected — convert the whole controller to a redirect shim (like planMilestonesEdit.php). Not applicable: the legacy dashio templates (reqViewVersions.tpl, reqReorder.tpl, reqEdit.tpl, reqViewVersionsViewer.tpl) still post real doActions (doDelete, doDeleteVersion, doFreezeVersion, doUnfreezeVersion, doDeleteRelation, doReorder, fileUpload, deleteFile, doCreate/doUpdate) to this controller. It is live, so the minimal dispatch fix is the correct blast radius.

Verification

before after
bare / xyz → 200 / 0 B / 4 E_WARNING 302 → reqSpecMgmt.html?tproject_id=1, 0 B, 0 events
simpleCompare → 500 / 0 B 302, 0 events
initGuiBean / initGuiObjForAttachmentOperations / setAuditContext → 200 / 0 B / warnings 302, 0 events
doAction[]=x / doAction[0]=x 302, 0 events
create → 200, 20517 B unchanged
edit → 200, 1536 B unchanged
reorder → 200, 5843 B unchanged
deleteFile → 200, 2109 B (legacy) unchanged
grep -rn 'method_exists($commandMgr' lib/ 0 live occurrences

Regression suite Issue #1739 appended to tmp/TLU_Test_Cases.md — 18/18 PASS, suite gate 7/7. Browser: reqEdit.php?doAction=xyz lands on "BugFix Project - Requirement Specification Management".

refusal lands on reqSpecMgmt.html

How to re-test in one command

# an unrenderable doAction must be 302 (never 200/0 or 500) and add zero events rows
curl -s -b cookie.jar -D - -o /dev/null \
  "http://localhost:8082/lib/requirements/reqEdit.php?doAction=simpleCompare" \
  | grep -i "^HTTP\|^location"