Bugfix Issue 1627 ReqMgrSystemEdit DoAction 500 - sebiboga/testlink-upgraded GitHub Wiki
Issue #1627 — reqMgrSystemEdit.php answered HTTP 500 with a 0-byte body on a missing or non-whitelisted doAction
Issue: #1627
Commits: 01f182bb9 (fix) + 7e014fea4 (regression suite)
Branch: fix/issue-1627 (pushed; the CI merge-prs.yml lands it)
Status: VERIFIED-FIXED — regression suite 24/24 PASS (bash tmp/verify_1627.sh, exit 0)
lib/reqmgrsystems/reqMgrSystemEdit.php (the legacy controller behind the Req. Management
System screen) returned HTTP 500 with an empty body for every request whose doAction
was missing or was not in the command bean's white list. The whitelisted actions were
unaffected (?doAction=create → 200/11488 B, ?doAction=edit&id=1 → 200/11715 B).
| URL | status | bytes |
|---|---|---|
/lib/reqmgrsystems/reqMgrSystemEdit.php |
500 | 0 |
…?doAction=bogus |
500 | 0 |
…?doAction=create |
200 | 11488 |
…?doAction=edit&id=1 |
200 | 11715 |
tmp/php_server.log — the 500s are fatals, and the stack proves where the fault lives:
PHP Fatal error: Uncaught Exception: Input parameter doAction - white list validation failure -
Value: - File: reqMgrSystemEdit.php - Function: init_args
in …/lib/reqmgrsystems/reqMgrSystemEdit.php:126
Stack trace:
#0 …/lib/reqmgrsystems/reqMgrSystemEdit.php(94): init_args()
#1 …/lib/reqmgrsystems/reqMgrSystemEdit.php(18): initScript()
#2 {main}
[200]/[500]: GET /lib/reqmgrsystems/reqMgrSystemEdit.php
The issue proposed that renderGui() dereferences a null $opObj
($opObj->template, $argsObj->doAction). Disproven: execution never reaches
renderGui(). The last frame is init_args(), called from initScript() at line 94 —
renderGui() is only called at line 26, after initScript() returns. The fault is in
the input-validation layer.
Also corrected: the "missing doAction" case is not an absent property.
R_PARAMS always creates every declared input, so property_exists() is true and the
value is '' — proven by the fatal's Value: (nothing after the colon) for the bare URL.
That is why the bare URL throws exactly like doAction=bogus: the flipped white list
(reqMgrSystemCommands.class.php:38-39, values
checkConnection|create|edit|delete|doCreate|doUpdate|doDelete) has no empty-string key.
The shipped view forms post an empty hidden action and only fill it in with JS:
-
gui/templates/tl-classic/reqmgrsystems/reqMgrSystemView.tpl:83—<input type="hidden" name="doAction" value="" /> -
gui/templates/tl-classic/reqmgrsystems/reqMgrSystemView.tpl:87—onclick="doAction.value='create'"(dashio copy::85/:89)
so a GET/POST of that form with JS disabled, aborted, or bookmarked posted an empty
doAction and hit the same 500. The ticket's "no in-app user can hit it today" was
therefore too optimistic.
| # | file:line | what happens |
|---|---|---|
| 1 |
reqMgrSystemView.tpl:83 (tl-classic) / :85 (dashio) |
the form ships doAction="", filled in by JS only |
| 2 | reqMgrSystemEdit.php:18 |
initScript($db) runs first |
| 3 | reqMgrSystemEdit.php:94 |
init_args(array('doAction' => $mgr->getGuiOpWhiteList())) |
| 4 | reqMgrSystemCommands.class.php:38-39 |
white list has no '' key |
| 5 | reqMgrSystemEdit.php:113 |
R_PARAMS always creates $args->doAction (declared :108) |
| 6 | reqMgrSystemEdit.php:120 |
isset($allowedValues['']) is false → guard entered for '' and 'bogus'
|
| 7 | reqMgrSystemEdit.php:125-126 |
tLog($msg,'ERROR') (good) then throw new Exception($msg)
|
| 8 | reqMgrSystemEdit.php:18 |
nothing catches it — the file installs no set_exception_handler anywhere in lib/ ⇒ PHP fatal ⇒ HTTP 500, 0 bytes
|
Why it breaks now: step 7. The throw-inside-init_args is the only place in lib/
where a missing value is escalated to a fatal
(grep -rln "white list validation failure" lib/ → this file only), and step 5 is why the
"parameter absent" branch at :118 was dead code that could never protect anything.
Latent landmine, not the observed cause (the ticket's hypothesis, still real):
renderGui():52,60 dereference $opObj, which is null when the action is not a
$commandMgr method (:22-25). get_object_vars(null) is a PHP 8 TypeError.
| query | result |
|---|---|
grep -rln "white list validation failure" lib/ |
1 file — this controller |
grep -rln "getGuiOpWhiteList" lib/ |
2 — this controller + its command bean |
| 14 sibling legacy edit controllers | all use switch ($args->doAction) with no throwing default; this one was the outlier |
| modern screen |
gui/templates/reqmgrsystems/reqMgrSystemView.html + api/reqmgrsystems/ never call this controller — unaffected |
lib/reqmgrsystems/reqMgrSystemEdit.php, two hunks:
-
init_args()— the single guard that conflated two situations is split:- an empty/absent
doActionis normalised to'create'and the form renders with 200 (that is exactly the value the shipped form posts before its JS runs, and the documented expectation on the ticket); - a non-empty, non-whitelisted value keeps the unchanged
tLog(...,'ERROR')audit row and is answered with a 302 togui/templates/reqmgrsystems/reqMgrSystemView.html— the graceful handling other legacy 2.0.1 controllers already use (lib/keywords/keywordsEdit.php:105). No new user-facing string, therefore no locale bundle touched.
- an empty/absent
-
renderGui()— a defensive 3-lineis_null($opObj)earlybreak, so the 5casevalues can never hit the PHP 8TypeErroron a null$opObj.
- try/catch around the whole script — broader than the defect and would silently swallow genuine errors for all 7 actions.
-
adding
''to the white list — would leavedoAction=bogus(the crafted-value half of the reported symptom) still fatal. -
a new 400 page with a new message — needs a
lang_get()key in all 20locale/*/strings.txtfiles for one line of text; the 302 achieves the same "no fatal" outcome with zero new strings and lands the user on a screen they can act on. -
redirecting with 303 for POST — considered; no functional difference today (the modern
list screen ignores the method), left as 302 to match
keywordsEdit.php.
Unchanged. The only newly reachable command is create, which performs no write
(reqMgrSystemCommands.class.php:93-107 only builds a gui bean), and the whole controller
stays behind checkRights() → reqmgrsystem_management (:177-180, applied by
testlinkInitPage() at :15 before init_args()). The write methods were already
whitelisted and reachable by URL before the fix.

Regression suite 1627, bash tmp/verify_1627.sh — 24/24 PASS (exit 0). It creates
and deletes its own reqmgrsystems row, so it is re-runnable on a fresh import.
| before | after |
|---|---|
no doAction → 500 / 0 B |
200 / 11488 B, create form |
?doAction= → 500 / 0 B |
200, create form |
?doAction=bogus → 500 / 0 B |
302 → reqMgrSystemView.html + ERROR event |
?doAction=CREATE, 28-char value → 500 |
302 + ERROR event |
view form posted with the empty hidden doAction → 500 |
200, create form |
?doAction=create / ?doAction=edit&id=N
|
200, unchanged |
doCreate / doUpdate / doDelete
|
302 → reqMgrSystemView.php, row really written/renamed/deleted (DB-verified) |
| anonymous | bounced to login.php?note=expired, unchanged |
| Event Viewer | 0 new E_WARNING/E_ERROR rows from the controller; 3 log_level 1 audit rows per rejected request, by design |
Negative control: with only the controller file reverted to
git show 4be7ee6ff:lib/reqmgrsystems/reqMgrSystemEdit.php, the same harness reports
15 PASS / 9 FAIL — it really detects the bug. php -l clean.
-
?doAction[]=xis still a 500: theTypeErrorcomes from the shared input layer (lib/functions/inputparameter.inc.php:229catchesException, but the failure is a PHPError) and therefore affects every legacy controller — a cross-cutting fix, not this ticket's. -
?doAction=edit&id=<unknown id>renders a broken form and logs 5E_WARNINGrows ($gui->itemisnull, unguardedreqMgrSystemCommands.class.php:164). -
?doAction=deleteis whitelisted but has no command method → HTTP 200 with a 0-byte body (renderGui'sswitchhas nodefault:). Fixed by #1722 — see Bugfix-Issue-1722-ReqMgrSystemEdit-Blank-200-Default-Render-Branch.md;renderGui()'s$renderTypeswitch now has adefault:that applies the same graceful 302. -
?doAction=checkConnectionlogsE_WARNING Undefined array key "checkConnection"(initGuiBean,reqMgrSystemCommands.class.php:72) — that is #1628.
bash tmp/verify_1627.sh # 24/24 PASS expected, exit 0