From d3f76a10300800d72c200f905e47b5cd4281392a Mon Sep 17 00:00:00 2001 From: mscherer Date: Wed, 13 May 2026 20:13:27 +0200 Subject: [PATCH] Fix deprecations panel empty check, escape variables panel error, reset $data in serialize loop * DeprecationsPanel template: the empty-check sum was missing count($other), and the surrounding condition was inverted -- the "No deprecations" flash showed up exactly when deprecations existed. Include $other and flip to `if (!$hasAny)`. * Variables panel template: $error came through `printf %s` unescaped. The message can carry serializer-exception text that echoes back data from whatever variable failed to serialize, so escape it via h(). * ToolbarService::saveData: $data was declared inside `try`, so when a panel's data() itself threw, the catch block's `gettype($data ?? null)` diagnostic reported the previous iteration's panel data type instead of NULL. Reset $data = null per iteration. Adds integration tests covering each fix (deprecations empty / non-empty / other-only, variables panel error escaping, panel data() throwing). --- src/ToolbarService.php | 1 + templates/element/deprecations_panel.php | 4 +- templates/element/variables_panel.php | 5 +- .../Controller/PanelsControllerTest.php | 98 +++++++++++++++++++ tests/TestCase/ToolbarServiceTest.php | 49 ++++++++++ tests/test_app/Panel/ThrowingDataPanel.php | 26 +++++ 6 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 tests/test_app/Panel/ThrowingDataPanel.php diff --git a/src/ToolbarService.php b/src/ToolbarService.php index 220013e72..ad544bcf4 100644 --- a/src/ToolbarService.php +++ b/src/ToolbarService.php @@ -290,6 +290,7 @@ public function saveData(ServerRequest $request, ResponseInterface $response): R foreach ($this->registry->loaded() as $name) { $panel = $this->registry->{$name}; + $data = null; try { $data = $panel->data(); diff --git a/templates/element/deprecations_panel.php b/templates/element/deprecations_panel.php index 9cb03c3bf..38da14bd5 100644 --- a/templates/element/deprecations_panel.php +++ b/templates/element/deprecations_panel.php @@ -23,7 +23,7 @@ use function Cake\Core\h; -$hasAny = count($app) + count($plugins) + count($cake) + count($vendor); +$hasAny = count($app) + count($plugins) + count($cake) + count($vendor) + count($other); $printer = function ($section, $data) { ?> @@ -41,7 +41,7 @@ } ?>
- +

No deprecations

%s

', $error); + printf('

%s

', h($error)); endif; if (isset($varsMaxDepth)) { diff --git a/tests/TestCase/Controller/PanelsControllerTest.php b/tests/TestCase/Controller/PanelsControllerTest.php index 461a26fef..0092a0de2 100644 --- a/tests/TestCase/Controller/PanelsControllerTest.php +++ b/tests/TestCase/Controller/PanelsControllerTest.php @@ -98,6 +98,104 @@ public function testViewNotExists() $this->assertResponseContains('Error page'); } + /** + * Deprecations panel renders the "No deprecations" flash only when + * every category is empty (including the `other` bucket, which was + * previously missing from the check). + * + * @return void + */ + public function testViewDeprecationsPanelEmpty() + { + $request = $this->makeRequest(); + $panel = $this->makePanel( + $request, + 'DebugKit.Deprecations', + 'Deprecations', + 'DebugKit.deprecations_panel', + ['app' => [], 'cake' => [], 'vendor' => [], 'plugins' => [], 'other' => []], + ); + + $this->get("/debug-kit/panels/view/{$panel->id}"); + + $this->assertResponseOk(); + $this->assertResponseContains('No deprecations'); + } + + /** + * @return void + */ + public function testViewDeprecationsPanelWithEntries() + { + $request = $this->makeRequest(); + $entry = ['niceFile' => 'src/Foo.php', 'line' => 1, 'message' => 'deprecated thing']; + $panel = $this->makePanel( + $request, + 'DebugKit.Deprecations', + 'Deprecations', + 'DebugKit.deprecations_panel', + ['app' => [$entry], 'cake' => [], 'vendor' => [], 'plugins' => [], 'other' => []], + ); + + $this->get("/debug-kit/panels/view/{$panel->id}"); + + $this->assertResponseOk(); + $this->assertResponseNotContains('No deprecations'); + $this->assertResponseContains('deprecated thing'); + } + + /** + * Deprecations only present in the `other` bucket should also suppress + * the "No deprecations" flash. Guards against a regression of M3 where + * `count($other)` was missing from the empty-check sum. + * + * @return void + */ + public function testViewDeprecationsPanelOtherOnly() + { + $request = $this->makeRequest(); + $entry = ['niceFile' => 'src/Bar.php', 'line' => 2, 'message' => 'only-other deprecation']; + $panel = $this->makePanel( + $request, + 'DebugKit.Deprecations', + 'Deprecations', + 'DebugKit.deprecations_panel', + ['app' => [], 'cake' => [], 'vendor' => [], 'plugins' => [], 'other' => [$entry]], + ); + + $this->get("/debug-kit/panels/view/{$panel->id}"); + + $this->assertResponseOk(); + $this->assertResponseNotContains('No deprecations'); + $this->assertResponseContains('only-other deprecation'); + } + + /** + * The Variables panel surfaces serialization-error messages from + * `ToolbarService`; those messages can echo back attacker-controlled + * data (e.g. via `__sleep` exceptions), so the template must escape + * them. Guards against a regression of M4. + * + * @return void + */ + public function testViewVariablesPanelErrorIsEscaped() + { + $request = $this->makeRequest(); + $panel = $this->makePanel( + $request, + 'DebugKit.Variables', + 'Variables', + 'DebugKit.variables_panel', + ['error' => '', 'variables' => [], 'errors' => []], + ); + + $this->get("/debug-kit/panels/view/{$panel->id}"); + + $this->assertResponseOk(); + $this->assertResponseNotContains(''); + $this->assertResponseContains('<script>alert(1)</script>'); + } + /** * @return void */ diff --git a/tests/TestCase/ToolbarServiceTest.php b/tests/TestCase/ToolbarServiceTest.php index 789d30f08..7cfc36c7f 100644 --- a/tests/TestCase/ToolbarServiceTest.php +++ b/tests/TestCase/ToolbarServiceTest.php @@ -27,6 +27,7 @@ use DebugKit\Model\Entity\Request as RequestEntity; use DebugKit\Panel\SqlLogPanel; use DebugKit\TestApp\Panel\SimplePanel; +use DebugKit\TestApp\Panel\ThrowingDataPanel; use DebugKit\ToolbarService; use PHPUnit\Framework\Attributes\DataProvider; @@ -559,4 +560,52 @@ public function testSaveDataSerializationError() $this->assertArrayHasKey('error', $content, 'Should have error key'); $this->assertStringContainsString('SimplePanel', $content['error'], 'Error should mention panel name'); } + + /** + * Test that saveData survives a panel whose data() method itself throws, + * not just one whose serialize() fails. + * + * @return void + */ + public function testSaveDataPanelDataThrows() + { + $request = new Request([ + 'url' => '/articles', + 'environment' => ['REQUEST_METHOD' => 'GET'], + ]); + $response = new Response([ + 'statusCode' => 200, + 'type' => 'text/html', + 'body' => 'test

some text

', + ]); + + $bar = new ToolbarService($this->events, []); + $bar->loadPanels(); + $bar->registry()->load('DebugKit.TestApp\Panel\ThrowingDataPanel', [ + 'className' => ThrowingDataPanel::class, + ]); + + $row = $bar->saveData($request, $response); + $this->assertNotEmpty($row, 'Should save data even when a panel data() throws'); + + $requests = $this->getTableLocator()->get('DebugKit.Requests'); + $result = $requests->find() + ->orderBy(['Requests.requested_at' => 'DESC']) + ->contain('Panels') + ->first(); + + $throwingPanel = null; + foreach ($result->panels as $p) { + if ($p->panel === 'DebugKit.TestApp\Panel\ThrowingDataPanel') { + $throwingPanel = $p; + break; + } + } + + $this->assertNotNull($throwingPanel, 'ThrowingDataPanel should be present'); + $content = unserialize($throwingPanel->content); + $this->assertArrayHasKey('error', $content); + $this->assertStringContainsString('ThrowingDataPanel', $content['error']); + $this->assertStringContainsString('Simulated data() failure', $content['error']); + } } diff --git a/tests/test_app/Panel/ThrowingDataPanel.php b/tests/test_app/Panel/ThrowingDataPanel.php new file mode 100644 index 000000000..096644a55 --- /dev/null +++ b/tests/test_app/Panel/ThrowingDataPanel.php @@ -0,0 +1,26 @@ +