From 7f8e27181770c76ba89517649e69ee86beed2321 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Thu, 20 Aug 2026 05:58:23 -0400 Subject: [PATCH 1/2] refactor: simplify strict value hydration, collector cleanup reporting, sensitive-key lookup, and toolbar message validation without changing public contracts. --- CHANGELOG.md | 1 + resources/assets/dist/js/focus.min.js | 2 +- resources/src/toolbar/focus.js | 29 ++--- src/Collector/CollectorCoordinator.php | 28 +++-- src/Helper/SensitiveDataRedactor.php | 18 +++- src/Storage/DebugValue.php | 140 ++++++++++--------------- 6 files changed, 108 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 402a96b..850854b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,3 +24,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - feat(ui): add sensitive queue-payload redaction and recognize Yii3 queue producers for Dump, Mail, and Queue parity. - fix(ui): add keyboard-resizable drawers with Escape handling and focus restoration. - fix: harden packaging, privacy, lifecycle, snapshot recovery, dump and toolbar security, and accelerate value hydration. +- refactor: simplify strict value hydration, collector cleanup reporting, sensitive-key lookup, and toolbar message validation without changing public contracts. diff --git a/resources/assets/dist/js/focus.min.js b/resources/assets/dist/js/focus.min.js index 5c32a40..37c617a 100644 --- a/resources/assets/dist/js/focus.min.js +++ b/resources/assets/dist/js/focus.min.js @@ -1 +1 @@ -function e(e,t){if(!e)return!1;var n=e.querySelector(t);return!n||typeof n.focus!=`function`?!1:(n.focus(),!0)}function t(e,t){if(!e||!t)return!1;for(var n=e.querySelectorAll(`[data-debug-url]`),r=0;rshutdown(); } catch (Throwable $cleanupFailure) { - if ($cleanupFailureHandler !== null) { - try { - $cleanupFailureHandler($cleanupFailure); - } catch (Throwable) { - // Diagnostic observers must not replace the primary application failure. - } - } + self::reportCleanupFailure($cleanupFailure, $cleanupFailureHandler); } throw $primaryFailure; @@ -239,4 +233,24 @@ public function startup(): void $this->started = true; } + + /** + * Reports a secondary cleanup failure without allowing the observer to replace the primary failure. + * + * @param (callable(Throwable): void)|null $cleanupFailureHandler Secondary-failure observer. + */ + private static function reportCleanupFailure( + Throwable $cleanupFailure, + callable|null $cleanupFailureHandler, + ): void { + if ($cleanupFailureHandler === null) { + return; + } + + try { + $cleanupFailureHandler($cleanupFailure); + } catch (Throwable) { + // Diagnostic observers must not replace the primary application failure. + } + } } diff --git a/src/Helper/SensitiveDataRedactor.php b/src/Helper/SensitiveDataRedactor.php index 3a0b0d8..5272055 100644 --- a/src/Helper/SensitiveDataRedactor.php +++ b/src/Helper/SensitiveDataRedactor.php @@ -70,7 +70,7 @@ final class SensitiveDataRedactor */ public static function isSensitiveKey(string $key, array $sensitiveKeys = self::DEFAULT_KEYS): bool { - return isset(array_fill_keys(array_map(strtolower(...), $sensitiveKeys), true)[strtolower($key)]); + return isset(self::keyMap($sensitiveKeys)[strtolower($key)]); } /** @@ -85,11 +85,21 @@ public static function isSensitiveKey(string $key, array $sensitiveKeys = self:: */ public static function redact(#[SensitiveParameter] array $value, array $sensitiveKeys = self::DEFAULT_KEYS): array { - $keys = array_fill_keys(array_map(strtolower(...), $sensitiveKeys), true); - $nodes = 0; - return self::walk($value, $keys, 0, $nodes); + return self::walk($value, self::keyMap($sensitiveKeys), 0, $nodes); + } + + /** + * Normalizes configured keys into a case-insensitive lookup map. + * + * @param list $sensitiveKeys Exact key names to normalize. + * + * @return array Normalized key lookup. + */ + private static function keyMap(array $sensitiveKeys): array + { + return array_fill_keys(array_map(strtolower(...), $sensitiveKeys), true); } /** diff --git a/src/Storage/DebugValue.php b/src/Storage/DebugValue.php index 1cdb570..2a934be 100644 --- a/src/Storage/DebugValue.php +++ b/src/Storage/DebugValue.php @@ -40,6 +40,14 @@ */ final readonly class DebugValue implements JsonSerializable { + /** + * @var array + */ + private const array ENTRY_SHAPE = [ + 'keyType' => true, + 'key' => true, + 'value' => true, + ]; private const int MAX_DEPTH = 10; private const int MAX_NODES = 10000; @@ -234,11 +242,7 @@ public function toDisplayValue(): mixed */ private static function bool(array $payload, string $key, string $path): bool { - if (!array_key_exists($key, $payload)) { - throw HydrationException::at("{$path}.{$key}", 'a required field'); - } - - $value = $payload[$key]; + $value = self::required($payload, $key, $path); if (!is_bool($value)) { throw HydrationException::at("{$path}.{$key}", 'a boolean'); @@ -273,49 +277,14 @@ private function displayLabel(): string */ private static function entryObject(mixed $value, string $path): array { - if (!is_array($value) || (array_is_list($value) && $value !== [])) { - throw HydrationException::at($path, 'an object'); - } - - foreach ($value as $key => $_) { - if (!is_string($key)) { - throw HydrationException::at($path, 'an object with string keys'); - } - } + $entry = self::object($value, $path); - if (!array_key_exists('keyType', $value)) { - throw HydrationException::at("{$path}.keyType", 'a required field'); - } - - if (!array_key_exists('key', $value)) { - throw HydrationException::at("{$path}.key", 'a required field'); - } - - if (!array_key_exists('value', $value)) { - throw HydrationException::at("{$path}.value", 'a required field'); - } - - if (count($value) !== 3) { - $unknown = array_diff_key( - $value, - [ - 'keyType' => true, - 'key' => true, - 'value' => true, - ], - ); - - if ($unknown !== []) { - $key = array_key_first($unknown); - - throw HydrationException::at("{$path}.{$key}", 'a declared field'); - } - } + self::validateShape($entry, self::ENTRY_SHAPE, $path); return [ - 'keyType' => $value['keyType'], - 'key' => $value['key'], - 'value' => $value['value'], + 'keyType' => self::required($entry, 'keyType', $path), + 'key' => self::required($entry, 'key', $path), + 'value' => self::required($entry, 'value', $path), ]; } @@ -483,11 +452,7 @@ private static function hydrateEntries(array $payload, string $path, int $depth, */ private static function int(array $payload, string $key, string $path): int { - if (!array_key_exists($key, $payload)) { - throw HydrationException::at("{$path}.{$key}", 'a required field'); - } - - $value = $payload[$key]; + $value = self::required($payload, $key, $path); if (!is_int($value)) { throw HydrationException::at("{$path}.{$key}", 'an integer'); @@ -505,11 +470,7 @@ private static function int(array $payload, string $key, string $path): int */ private static function list(array $payload, string $key, string $path): array { - if (!array_key_exists($key, $payload)) { - throw HydrationException::at("{$path}.{$key}", 'a required field'); - } - - $value = $payload[$key]; + $value = self::required($payload, $key, $path); if (!is_array($value) || !array_is_list($value)) { throw HydrationException::at("{$path}.{$key}", 'a list'); @@ -676,11 +637,7 @@ className: $value::class, */ private static function nullableString(array $payload, string $key, string $path): string|null { - if (!array_key_exists($key, $payload)) { - throw HydrationException::at("{$path}.{$key}", 'a required field'); - } - - $value = $payload[$key]; + $value = self::required($payload, $key, $path); if ($value !== null && !is_string($value)) { throw HydrationException::at("{$path}.{$key}", 'a string or null'); @@ -696,11 +653,7 @@ private static function nullableString(array $payload, string $key, string $path */ private static function number(array $payload, string $key, string $path): float { - if (!array_key_exists($key, $payload)) { - throw HydrationException::at("{$path}.{$key}", 'a required field'); - } - - $value = $payload[$key]; + $value = self::required($payload, $key, $path); if (!is_int($value) && (!is_float($value) || !is_finite($value))) { throw HydrationException::at("{$path}.{$key}", 'a number'); @@ -709,6 +662,26 @@ private static function number(array $payload, string $key, string $path): float return (float) $value; } + /** + * Returns a decoded JSON object with string keys. + * + * @return array Validated object fields. + */ + private static function object(mixed $value, string $path): array + { + if (!is_array($value) || (array_is_list($value) && $value !== [])) { + throw HydrationException::at($path, 'an object'); + } + + foreach ($value as $key => $_) { + if (!is_string($key)) { + throw HydrationException::at($path, 'an object with string keys'); + } + } + + return $value; + } + /** * Returns a safe display label for an object. * @@ -742,17 +715,27 @@ private static function objectLabel(object $value): string } /** - * Returns a required string without coercion. + * Returns a required field without coercion. * * @param array $payload Validated payload. */ - private static function string(array $payload, string $key, string $path): string + private static function required(array $payload, string $key, string $path): mixed { if (!array_key_exists($key, $payload)) { throw HydrationException::at("{$path}.{$key}", 'a required field'); } - $value = $payload[$key]; + return $payload[$key]; + } + + /** + * Returns a required string without coercion. + * + * @param array $payload Validated payload. + */ + private static function string(array $payload, string $key, string $path): string + { + $value = self::required($payload, $key, $path); if (!is_string($value)) { throw HydrationException::at("{$path}.{$key}", 'a string'); @@ -770,21 +753,8 @@ private static function string(array $payload, string $key, string $path): strin */ private static function taggedObject(mixed $value, string $path, string &$type): array { - if (!is_array($value) || (array_is_list($value) && $value !== [])) { - throw HydrationException::at($path, 'an object'); - } - - foreach ($value as $key => $_) { - if (!is_string($key)) { - throw HydrationException::at($path, 'an object with string keys'); - } - } - - if (!array_key_exists('type', $value)) { - throw HydrationException::at("{$path}.type", 'a required field'); - } - - $rawType = $value['type']; + $payload = self::object($value, $path); + $rawType = self::required($payload, 'type', $path); if (!is_string($rawType)) { throw HydrationException::at("{$path}.type", 'a string'); @@ -797,9 +767,9 @@ private static function taggedObject(mixed $value, string $path, string &$type): 'a known debug-value type', ); - self::validateShape($value, $shape, $path); + self::validateShape($payload, $shape, $path); - return $value; + return $payload; } /** From 4281e81e4409bad7cce40986cda638121376d24c Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Thu, 20 Aug 2026 06:17:48 -0400 Subject: [PATCH 2/2] Apply fixed coderabbitai review. --- resources/assets/dist/js/focus.min.js | 2 +- resources/src/toolbar/focus.js | 5 ++++- resources/tests/toolbar-runtime.test.js | 12 ++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/resources/assets/dist/js/focus.min.js b/resources/assets/dist/js/focus.min.js index 37c617a..3a9055d 100644 --- a/resources/assets/dist/js/focus.min.js +++ b/resources/assets/dist/js/focus.min.js @@ -1 +1 @@ -function e(e,t){if(!e)return!1;var n=e.querySelector(t);return!n||typeof n.focus!=`function`?!1:(n.focus(),!0)}function t(e,t){if(!e||!t)return!1;for(var n=e.querySelectorAll(`[data-debug-url]`),r=0;r { ), false, ); + var callableData = function () {}; + callableData.source = "yii-debug-toolbar"; + callableData.type = "theme"; + + assert.equal( + isToolbarDrawerThemeMessage( + { ...message, data: callableData }, + "https://example.test", + frameWindow, + ), + false, + ); }); test("embedded debug pages request drawer closure after an unhandled Escape", () => {