Bugfix Issue 1893 SearchMgmt Array Target E_WARNING - sebiboga/testlink-upgraded GitHub Wiki

Issue #1893 — searchMgmt.php array-shaped target wrote an E_WARNING row into events

Issue: #1893 Branch: fix/issue-1893-searchmgmt-array-param Status: VERIFIED-FIXED — regression suite Issue #1893 6/6 PASS

Symptom

The 2.0.1 session-guarded redirect shim lib/search/searchMgmt.php (Refs #1785) answered an authenticated GET with an array-shaped target by writing an E_WARNING row into the events table on every request:

GET /lib/search/searchMgmt.php?target[]=x        (authenticated)
-> HTTP 302 with Location: ...&target=Array
   +1 events row: log_level 2 | E_WARNING
   Array to string conversion - in .../lib/search/searchMgmt.php - Line 24

5 requests = 5 rows. Third instance of the array-shaped-$_REQUEST family (#1731, #1732), found while regression-testing #1732. Reachable from ANY authenticated session; no rights check beyond testlinkInitPage.

Pre-fix Event Viewer (7 rows from the malformed requests):

Event Viewer showing the pre-fix E_WARNING rows

Root cause chain

# file:line what happens
1 request ?target[]=x PHP parses it into $_REQUEST['target'] = array('x'); isset() is true for arrays.
2 lib/search/searchMgmt.php:24 (pre-fix) isset($_REQUEST['target']) ? trim((string)$_REQUEST['target']) : '' → (string) of an array raises E_WARNING Array to string conversion (persisted by watchPHPErrors, lib/functions/logger.class.php:1407-1446) and yields the literal "Array".
3 lib/search/searchMgmt.php:25-36 the coerced "Array" passes the strlen <= 200 clamp and is rawurlencoded into the Location header.

Introduced by the 2.0.1 shim rewrite (db1a4335e, Refs #1785); the legacy controller never had this raw $_REQUEST read.

Investigation (measured before any code was touched)

Environment: http://localhost:8082, PHP 8.3.35, fresh testlink DB, admin/admin.

probe result
events log_level in (1,2) before the repro 0
GET ?target[]=x 302, Location: ...&target=Array
events after 1 request 1 row (E_WARNING @ Line 24)
events after 5 more requests 7 (+1 per request)
anonymous GET ?target[]=x login page, no row (session guard runs first)

The fix (minimal, 1 file, +19/-1)

Read target through the same is_scalar()-guarded shimReqScalar() idiom already established in the repo (listTestCases.php:57-63 #1732, reqMgrSystemEdit.php:80-86 #1731):

function shimReqScalar($name)   // null when absent OR array-shaped, else the trimmed string
{
    if (!isset($_REQUEST[$name]) || !is_scalar($_REQUEST[$name])) {
        return null;
    }
    return trim((string)$_REQUEST[$name]);
}

$target = shimReqScalar('target');
if ($target === null) {
  $target = '';
}

A present-but-non-scalar target is treated as absent (empty target): the coercion "Array" is never built, the E_WARNING is never raised, and the redirect drops the target parameter entirely. No user-facing string changed, so no i18n keys were added.

After the fix

?target[]=x × 3 → the modern search screen, events log_level in (1,2) = 0:

Event Viewer clean after the fix

Verification

case before after
?target[]=x × 5 +1 log_level=2 row per request +0 rows, 302
?target=foo%20bar 302 → ...&target=foo%20bar unchanged
(no target) 302 → ...?tproject_id=0&tplan_id=0 unchanged
anonymous ?target[]=x login page, no row unchanged
php -l lib/search/searchMgmt.php — No syntax errors detected

Regression suite Issue #1893 (6/6 PASS) in tmp/TLU_Test_Cases.md; suite gate 7/7 PASS.

How to re-test in one command

# login admin/admin via curl session cookie, then:
curl -s -b c.jar -D- -o /dev/null \
  'http://localhost:8082/lib/search/searchMgmt.php?target%5B%5D=x'
# expect: HTTP/1.1 302 Found + no target=Array
mysql -h 127.0.0.1 -utestlink -ptestlink testlink \
  -e "SELECT COUNT(*) FROM events WHERE log_level IN (1,2);"   # expect 0