Skip to content

Commit d0509eb

Browse files
committed
fix: encode diagnostic keys, exit codes, and six smaller issues from review
- Missing-key warnings and array-syntax deprecation notices HTML-encode the key before echoing it. With a dynamic key like ->get($_GET['sort']) a key containing HTML reached the browser unencoded - a reflected XSS vector. The trigger_error() copy carries the same encoded key. - or404() exits with status 1 like orDie(), so shell scripts and cron jobs see the failure. Output is unchanged. - json_encode() also substitutes malformed UTF-8 in keys with U+FFFD, not just values - one corrupt byte in a key no longer returns false and loses the whole document. - Array-syntax deprecation notices suggest one replacement style for reads, isset(), and unset(): ->key, ->{0}, ->{'users.id'}. Reads used to suggest ->get(0) while existence checks suggested ->{0}, so one empty() call printed two notices with different advice. Null and '' keys suggest ->get('') since ->{''} is a fatal error. - debug(1) prints closures and other objects as their type instead of throwing Unsupported type: Closure - which fired on exactly the arrays that have a load handler, the ones debug exists to inspect. - SmartNull->help() wraps in <xmp> when no Content-Type header is set, same rule as every other help() and debug(). It previously required an explicit text/html header, so typical pages got collapsed text. - filter() phpdoc notes keys are preserved like array_filter() and shows ->values() as the reindex step before json_encode(). Also: settled all remaining REVIEW comments in the test suite - each now states the ruled behavior and why (load() record-set guard, filter keys, conversion root pointer, nested isset noise, SmartNull error class names, docs coverage).
1 parent a5ad0b7 commit d0509eb

14 files changed

Lines changed: 108 additions & 89 deletions

‎CHANGELOG.md‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
77
## [Unreleased]
88

9+
### Security
10+
- Missing-key warnings and array-syntax deprecation notices HTML-encode the key before echoing it. With a dynamic key (`->get($_GET['sort'])`, `$arr[$_GET['sort']]`), a key containing HTML reached the browser unencoded - a reflected XSS vector. The `trigger_error()` copy carries the same encoded key.
11+
- `json_encode($smartArray)` also substitutes malformed UTF-8 in keys with � (U+FFFD), not just values - one corrupt byte in a key no longer makes it return false and lose the whole document.
12+
913
### Added
1014
- `at($index)` - new name for `nth()`: get an element by position, zero-based, negative indices count from the end. Matches JavaScript's `Array.at()`. `get()` is by key, `at()` is by position.
1115
- `columnAt($index)` - new name for `pluckNth()`: get the column at a position from each row, ignoring key names. `column()` is by key, `columnAt()` is by position.
@@ -25,9 +29,10 @@
2529
- `SmartArray::new($data, true)` and `SmartArrayHtml::new($data, false)` now throw like the constructors do, instead of silently ignoring the boolean. Old code that passed `true` expecting auto-encoding was silently getting raw, unencoded values - now it fails at the call site with the class to use instead. Redundant booleans (`false` on SmartArray, `true` on SmartArrayHtml) log a deprecation and proceed.
2630
- `sortBy()` second parameter renamed `$type` → `$flags` - it always held PHP sort flags, and now matches `sort()` and PHP's own sort functions. Affects named-argument calls only: `sortBy('name', flags: SORT_NATURAL)`.
2731
- `column(null)` and `column(null, null)` now match PHP's `array_column()`: whole rows renumbered from 0, instead of throwing "unexpected arguments"
32+
- Array-syntax deprecation notices suggest one replacement style for reads, `isset()`, and `unset()`: `->key` for property-safe names, `->{0}` for integer keys, `->{'users.id'}` for other keys (`->get()` also works for reads). Reads used to suggest `->get(0)` while existence checks suggested `->{0}`, so one `empty()` call printed two notices with different advice. Null and `''` keys suggest `->get('')` - the brace form is a fatal error for an empty property name.
2833
- `isset($array['key'])` and `empty($array['key'])` now follow `$onOffsetAccess` like reads, writes, and `unset()` - notice by default, exception in `'throw'` mode. Existence checks were the one silent form of the deprecated `[]` syntax; if `[]` support is removed in a future version, `isset()` on the object would silently return false instead of erroring, so these call sites need migrating with the rest. Property-syntax checks (`isset($array->key)`) are unaffected and stay signal-free. Internal existence checks now call `array_key_exists()` directly, removing two method calls from every `get()`.
2934
- `or404()` outputs `<html>` instead of `<html lang>` - an empty `lang` reads as an invalid value to accessibility checkers, and the message language is caller-supplied so it can't be declared. Matches SmartString.
30-
- `orDie()` now exits with status 1 instead of 0, so shell scripts and cron jobs see the failure. Output is unchanged. Matches SmartString.
35+
- `orDie()` and `or404()` now exit with status 1 instead of 0, so shell scripts and cron jobs see the failure. Output is unchanged. Matches SmartString.
3136
- Developer-mistake exceptions (bad types, wrong context, misuse) now throw `CallerException`, which reports your file and line instead of the library's internals - the same class SmartString uses. It extends `InvalidArgumentException`, so existing catch blocks keep working, except six throws that previously used `RuntimeException`: `load()` misuse (no handler, non-callable handler, bad or empty field name, called on a record set), `orRedirect()` after headers sent, and writing to a `SmartNull`. See UPGRADING.md. `orThrow()` still throws `RuntimeException` by contract.
3237
- Clearer messages for two of those throws: `load()` with no handler now explains handlers come from the database layer (was "No loadHandler property is defined"), and writing to a `SmartNull` now says the value came from a missing key or empty result (was "Cannot set values on SmartNull"). The unsupported-type message from `set()` no longer prefixes the library's internal method name.
3338
- Unknown methods on `SmartNull` now throw the same `Error` as the rest of the library - method name, "did you mean" hint, caller's file and line - instead of `InvalidArgumentException("Method 'x' not found")`. Chains from a missing key now fail with the same message quality as everything else.
@@ -44,6 +49,8 @@
4449
- `setLoadHandler()` - the handler is passed as the `loadHandler` constructor property, which is how the database layer (ZenDB) has always set it. The setter had no known callers, and it couldn't work on record sets anyway: rows are built during construction and snapshot the handler at that moment, so a handler set afterward never reached them.
4550

4651
### Fixed
52+
- `SmartNull->help()` wraps its output in `<xmp>` when no Content-Type header is set, same rule as every other `help()` and `debug()` - PHP's default response type is text/html. Previously it only wrapped when text/html was explicitly set via `header()`, so on typical pages (which never call `header()`) it printed as collapsed, unformatted text.
53+
- `debug(1)` no longer throws `Unsupported type: Closure` on arrays with a load handler - exactly the database results it exists to inspect. Callables and other objects in the properties block print as their type (`Closure,`) instead.
4754
- `get($key, $default)` defaults now act like stored values: Smart defaults (SmartString, SmartArray, SmartNull) unwrap and re-wrap for the array's mode. Previously a SmartNull default threw `InvalidArgumentException`, and cross-mode Smart defaults (a SmartString default on SmartArray, a raw SmartArray default on SmartArrayHtml) threw `TypeError` from the return declarations.
4855
- `sortBy()` no longer throws a bare `ValueError: Array sizes are inconsistent` when a row is missing the sort field. Missing fields sort first (treated as null for ordering, like MySQL ORDER BY); rows are returned unchanged.
4956
- `indexBy()` no longer gives rows missing the index field a leftover numeric key that looks like a real field value. Null and missing values now both index under `''` (matching how null field values were already handled), duplicates last-wins.

‎src/DeprecatedAliases.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -357,28 +357,30 @@ public function offsetUnset(mixed $offset): void
357357
*/
358358
private function triggerArrayAccessDeprecation(mixed $key, string $operation = 'get'): void
359359
{
360+
// SECURITY: the key can be user input (e.g. $arr[$_GET['sort']]) and 'notify' mode echoes
361+
// the message into the page, so encode it. $key is display-only from here on; the actual
362+
// data access already happened with the original key.
363+
if (is_string($key)) {
364+
$key = htmlspecialchars($key, self::HTML_ENCODE_FLAGS, 'UTF-8');
365+
}
360366
$keyStr = is_string($key) ? "'$key'" : (string) $key;
361367
$isValidPropName = is_string($key) && preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $key);
362368

363369
// Suggest the preferred access method. A null key only reaches 'set' via
364-
// the append syntax `$arr[] = $value`; for 'unset'/'get' it indicates a
365-
// programmer error that PHP itself treats as an empty-string key.
370+
// the append syntax `$arr[] = $value`; for 'unset'/'get'/'exists' it indicates
371+
// a programmer error that PHP itself treats as an empty-string key.
366372
$suggestion = match ($operation) {
367373
'set' => match (true) {
368374
is_null($key) => '->set($key, $value) using an explicit key',
369375
is_int($key) => "->set($key, \$value)",
370376
$isValidPropName => "->$key = \$value",
371377
default => "->set('$key', \$value) or ->{'$key'} = \$value",
372378
},
373-
'unset', 'exists' => match (true) {
374-
is_int($key) => '->{' . $key . '}',
375-
$isValidPropName => "->$key",
376-
default => "->{'$key'}",
377-
},
378379
default => match (true) {
379-
is_int($key) => "->get($key)",
380-
$isValidPropName => "->$key",
381-
default => "->get('$key')",
380+
is_int($key) => '->{' . $key . '}',
381+
$isValidPropName => "->$key",
382+
$key === '' || $key === null => "->get('')", // PHP treats a null/'' key as key ''; ->{''} is a fatal "Cannot access empty property"
383+
default => "->{'$key'}",
382384
},
383385
};
384386

‎src/SmartArrayBase.php‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -479,8 +479,12 @@ public function unique(): static
479479
* objects, and should return true to keep the element, false to remove it.
480480
* When called without a callback, removes all falsy values (empty strings, 0, null, false).
481481
*
482+
* Keys are preserved, like PHP's array_filter(), so a filtered list json_encodes as an
483+
* object ({"0":...,"2":...}) - chain ->values() first to reindex and get a JSON array.
484+
*
482485
* $active = $users->filter(fn($row) => $row['status'] === 'active');
483486
* $nonEmpty = $values->filter();
487+
* $json = json_encode($values->filter()->values()); // reindex for a JSON array
484488
*
485489
* @param callable|null $callback A function($value, $key) that returns true to keep, false to remove.
486490
* @return static A new SmartArray containing only the elements that passed the test.
@@ -1118,7 +1122,9 @@ private static function prettyPrintR(mixed $var, int $debugLevel = 0, int $depth
11181122
$varExport = 'SmartNull()';
11191123
$output = str_pad("$keyPrefix$varExport,", $commentOffset) . "$comment\n";
11201124
} else {
1121-
throw new RuntimeException("Unsupported type: $debugType");
1125+
// Anything else prints as its type, e.g. the loadHandler Closure in the
1126+
// debug(1) properties block - debug output describes, it never throws
1127+
$output = str_pad("$keyPrefix$debugType,", $commentOffset) . "$comment\n";
11221128
}
11231129

11241130
// Indent each line
@@ -1209,7 +1215,8 @@ public function __unset(string $name): void
12091215
//region Error Handling
12101216

12111217
/**
1212-
* Sends a 404 header and message if the array is empty, then exits.
1218+
* Sends a 404 header and message if the array is empty, then exits with status 1
1219+
* so shell scripts and cron jobs see the failure.
12131220
*
12141221
* @param string|null $text Plain-text message; HTML-encoded automatically before output. Defaults to "The requested URL was not found on this server."
12151222
* @return static Returns $this if not empty, exits with 404 if empty
@@ -1238,7 +1245,7 @@ public function or404(?string $text = null): static
12381245
</body>
12391246
</html>
12401247
__HTML__;
1241-
exit;
1248+
exit(1);
12421249
}
12431250

12441251
/**
@@ -1364,12 +1371,16 @@ private function warnIfMissing(string|int $key, string $warningType = 'argument'
13641371
if (empty($target->data) || array_key_exists($key, $target->data)) {
13651372
return;
13661373
}
1367-
$caller = self::getExternalCaller();
1368-
$keyOrEmptyQuotes = $key === "" ? "''" : $key; // Show empty quotes for empty string keys
1374+
$caller = self::getExternalCaller();
1375+
1376+
// SECURITY: the key can be user input (e.g. ->get($_GET['sort'])) and the warning echoes
1377+
// into the page, so encode it. The trigger_error() copy gets the same encoded key.
1378+
$keyDisplay = is_string($key) ? htmlspecialchars($key, self::HTML_ENCODE_FLAGS, 'UTF-8') : $key;
1379+
$keyOrEmptyQuotes = $keyDisplay === "" ? "''" : $keyDisplay; // Show empty quotes for empty string keys
13691380

13701381
$warning = match ($warningType) {
13711382
'offset' => "$keyOrEmptyQuotes is undefined in {$caller['file']}:{$caller['line']}\n",
1372-
'argument' => "{$caller['function']}(): '$key' doesn't exist\n",
1383+
'argument' => "{$caller['function']}(): '$keyDisplay' doesn't exist\n",
13731384
default => throw new InvalidArgumentException("Invalid warning type '$warningType'"),
13741385
};
13751386

@@ -1478,18 +1489,23 @@ public function getIterator(): Iterator
14781489
* Values are raw, not HTML-encoded, even for SmartArrayHtml: JSON is a data
14791490
* format, and HTML encoding applies only when values are output as HTML.
14801491
*
1481-
* Substitutes malformed UTF-8 with � (U+FFFD) so json_encode($smartArray) returns valid JSON
1482-
* instead of false. Nested SmartArrays scrub themselves when json_encode() descends into them.
1492+
* Substitutes malformed UTF-8 in keys and values with � (U+FFFD) so json_encode($smartArray)
1493+
* returns valid JSON instead of false. Nested SmartArrays scrub themselves when json_encode()
1494+
* descends into them.
14831495
*
14841496
* @return array The internal data array.
14851497
*/
14861498
public function jsonSerialize(): array
14871499
{
1488-
$data = $this->data;
1489-
foreach ($data as $key => $value) {
1490-
if (is_string($value) && preg_match('//u', $value) !== 1) { // isMalformed: ~5x faster than mb_check_encoding()
1491-
$data[$key] = json_decode(json_encode($value, JSON_INVALID_UTF8_SUBSTITUTE)); // json_encode's own U+FFFD substitution
1500+
$data = [];
1501+
foreach ($this->data as $key => $value) {
1502+
if (is_string($key) && preg_match('//u', $key) !== 1) { // isMalformed: ~5x faster than mb_check_encoding()
1503+
$key = json_decode(json_encode($key, JSON_INVALID_UTF8_SUBSTITUTE)); // json_encode's own U+FFFD substitution
1504+
}
1505+
if (is_string($value) && preg_match('//u', $value) !== 1) {
1506+
$value = json_decode(json_encode($value, JSON_INVALID_UTF8_SUBSTITUTE));
14921507
}
1508+
$data[$key] = $value;
14931509
}
14941510
return $data;
14951511
}

‎src/SmartNull.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ public function help(): void
101101
the final result is accessed.
102102
__TEXT__;
103103

104-
$isHtmlOutput = stripos(implode("\n", headers_list()), 'text/html') !== false;
104+
// Wrap in <xmp> for readability when output is (or will default to) HTML - same rule
105+
// as SmartArrayBase::xmpWrap(): no Content-Type header means PHP sends its default text/html
106+
$headersList = implode("\n", headers_list());
107+
$isHtmlOutput = !preg_match('|^\s*Content-Type:\s*|im', $headersList)
108+
|| preg_match('|^\s*Content-Type:\s*text/html\b|im', $headersList);
105109
if ($isHtmlOutput) {
106110
$output = "<xmp>$output</xmp>";
107111
}

‎tests/Integration/DocsCoverageTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ final class DocsCoverageTest extends SmartArrayTestCase
5151
* the forward check so the rest of the guard still runs, and each one is a docs bug
5252
* waiting to be written, not a permanent exemption.
5353
*/
54-
// REVIEW: empty as of 2026-08-02 - every public method is documented in both files.
54+
// Empty as of 2026-08-02: every public method is documented in both files.
5555
private const UNDOCUMENTED_TODAY = [];
5656

5757
//endregion

‎tests/Integration/ProductionRecipesTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ public function testA12FilteredPluckKeepsOriginalKeys(string $class): void
311311
fn() => $tables->pluck('TABLE_NAME')->filter(fn($name) => str_starts_with($name, 'cms_'))->toArray(),
312312
);
313313

314-
// REVIEW: filter() keeps the source keys, so this "flat value list" comes back
315-
// with a gap (0 and 2). json_encode() then emits an object instead of an array.
316-
// Alternative: reindex in filter(), or document ->values() as the finisher here.
314+
// filter() keeps source keys like array_filter(), so this value list comes back
315+
// with a gap (0 and 2) and json_encode() would emit an object, not an array.
316+
// Chain ->values() to reindex when that matters (noted in the filter() phpdoc).
317317
$this->assertSame([0 => 'cms_accounts', 2 => 'cms_orders'], $ours);
318318
$this->assertSame('', $output);
319319
}

‎tests/Unit/ConversionTest.php‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -423,16 +423,13 @@ public function testJsonSerializeReplacesMalformedUtf8WithReplacementCharacter(s
423423
}
424424

425425
#[DataProvider('modeProvider')]
426-
public function testJsonEncodeFailsOnMalformedUtf8Key(string $class): void
426+
public function testJsonEncodeSubstitutesMalformedUtf8InKeys(string $class): void
427427
{
428-
// REVIEW: the substitution covers values only, so one corrupt byte in a
429-
// KEY still returns false and loses the whole document - the failure the
430-
// value scrubbing exists to prevent. Keys come from column names, so this
431-
// is rare, but jsonSerialize() could scrub them the same way.
428+
// Keys scrub to U+FFFD like values, so one corrupt byte in a key can't
429+
// make json_encode() return false and lose the whole document.
432430
$sa = $class::new(["caf\xE9" => 'value']);
433431

434-
$this->assertFalse(json_encode($sa));
435-
$this->assertSame('Malformed UTF-8 characters, possibly incorrectly encoded', json_last_error_msg());
432+
$this->assertSame('{"caf\\ufffd":"value"}', json_encode($sa));
436433
}
437434

438435
//endregion

0 commit comments

Comments
 (0)