From 1d6804feac8cb7f3f5ec52f9bdf0d10f5d862c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 18 Sep 2023 19:54:00 +0200 Subject: [PATCH] Always assert request URL path starts with '/' (#2110) --- src/App.php | 26 +++++++++++--------------- src/Callback.php | 7 +++++-- tests/AppTest.php | 10 ++++++++++ tests/CallbackTest.php | 6 ++++-- tests/CreateAppTrait.php | 16 ++++++++++++---- tests/ExecutorFactoryTest.php | 14 ++++---------- tests/FormFieldUiTest.php | 2 +- tests/FormTest.php | 15 ++++++--------- tests/GridTest.php | 2 +- tests/TableColumnColorRatingTest.php | 2 +- tests/TableColumnLinkTest.php | 2 +- 11 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/App.php b/src/App.php index c6a251110f..b79c68e898 100644 --- a/src/App.php +++ b/src/App.php @@ -70,22 +70,16 @@ class App /** @var bool Will replace an exception handler with our own, that will output errors nicely. */ public $catchExceptions = true; - /** @var bool Will display error if callback wasn't triggered. */ - public $catchRunawayCallbacks = true; + /** Will display error if callback wasn't triggered. */ + protected bool $catchRunawayCallbacks = true; - /** @var bool Will always run application even if developer didn't explicitly executed run();. */ - public $alwaysRun = true; + /** Will always run application even if developer didn't explicitly executed run();. */ + protected bool $alwaysRun = true; - /** - * Will be set to true after app->run() is called, which may be done automatically - * on exit. - */ + /** Will be set to true after app->run() is called, which may be done automatically on exit. */ public bool $runCalled = false; - /** - * Will be set to true, when exit is called. Sometimes exit is intercepted by shutdown - * handler and we don't want to execute 'beforeExit' multiple times. - */ + /** Will be set to true after exit is called. */ private bool $exitCalled = false; public bool $isRendering = false; @@ -220,6 +214,11 @@ public function __construct(array $defaults = []) $this->uiPersistence = new UiPersistence(); } + if (!str_starts_with($this->getRequest()->getUri()->getPath(), '/')) { + throw (new Exception('Request URL path must always start with \'/\'')) + ->addMoreInfo('url', (string) $this->getRequest()->getUri()); + } + if ($this->session === null) { $this->session = new App\SessionManager(); } @@ -670,9 +669,6 @@ public function url($page = [], array $extraRequestUrlArgs = []): string if ($pagePath === null) { $pagePath = $request->getUri()->getPath(); - if ($pagePath === '') { // TODO path must always start with '/' - $pagePath = '/'; - } } if (str_ends_with($pagePath, '/')) { $pagePath .= $this->urlBuildingIndexPage; diff --git a/src/Callback.php b/src/Callback.php index a29460d522..665d0b1ca2 100644 --- a/src/Callback.php +++ b/src/Callback.php @@ -104,7 +104,7 @@ public function isTriggered(): bool public function getTriggeredValue(): string { - return $_GET[self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger] ?? ''; + return $_GET[self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger]; } /** @@ -147,6 +147,9 @@ public function getUrl(string $value = 'callback'): string */ private function getUrlArguments(string $value = null): array { - return [self::URL_QUERY_TARGET => $this->urlTrigger, self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger => $value ?? $this->getTriggeredValue()]; + return [ + self::URL_QUERY_TARGET => $this->urlTrigger, + self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger => $value ?? ($this->isTriggered() ? $this->getTriggeredValue() : ''), + ]; } } diff --git a/tests/AppTest.php b/tests/AppTest.php index 900b0d5f3e..16f065529b 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -5,6 +5,7 @@ namespace Atk4\Ui\Tests; use Atk4\Core\Phpunit\TestCase; +use Atk4\Ui\Exception; use Atk4\Ui\Exception\LateOutputError; use Atk4\Ui\HtmlTemplate; use Nyholm\Psr7\Factory\Psr17Factory; @@ -67,6 +68,15 @@ public function testUnexpectedOutputLateError(): void } } + public function testEmptyRequestPathException(): void + { + $request = (new Psr17Factory())->createServerRequest('GET', ''); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Request URL path must always start with \'/\''); + $this->createApp(['request' => $request]); + } + public function provideUrlCases(): iterable { foreach (['/', '/page.html', '/d/', '/0/index.php'] as $requestPage) { diff --git a/tests/CallbackTest.php b/tests/CallbackTest.php index 9f8d75a30c..6b3ecca922 100644 --- a/tests/CallbackTest.php +++ b/tests/CallbackTest.php @@ -26,17 +26,19 @@ protected function outputResponse(string $data): void class CallbackTest extends TestCase { + use CreateAppTrait; + /** @var string */ private $htmlDoctypeRegex = '~^app = new AppMock(['alwaysRun' => false, 'catchExceptions' => false]); + $this->app = $this->createApp([AppMock::class]); $this->app->initLayout([Layout\Centered::class]); } diff --git a/tests/CreateAppTrait.php b/tests/CreateAppTrait.php index 43c36d268a..b27337cf37 100644 --- a/tests/CreateAppTrait.php +++ b/tests/CreateAppTrait.php @@ -5,17 +5,25 @@ namespace Atk4\Ui\Tests; use Atk4\Ui\App; +use Nyholm\Psr7\Factory\Psr17Factory; trait CreateAppTrait { /** - * @param array $defaults + * @param array<0|string, mixed> $seed */ - protected function createApp(array $defaults = []): App + protected function createApp(array $seed = []): App { - return new App(array_merge([ + $appClass = $seed[0] ?? App::class; + unset($seed[0]); + + if (!isset($seed['request'])) { + $seed['request'] = (new Psr17Factory())->createServerRequest('GET', '/'); + } + + return new $appClass(array_merge([ 'catchExceptions' => false, 'alwaysRun' => false, - ], $defaults)); + ], $seed)); } } diff --git a/tests/ExecutorFactoryTest.php b/tests/ExecutorFactoryTest.php index 038383d591..d960ba3ca8 100644 --- a/tests/ExecutorFactoryTest.php +++ b/tests/ExecutorFactoryTest.php @@ -40,10 +40,12 @@ protected function init(): void class ExecutorFactoryTest extends TestCase { + use CreateAppTrait; + /** @var Model */ - public $model; + protected $model; /** @var App */ - public $app; + protected $app; protected function setUp(): void { @@ -55,14 +57,6 @@ protected function setUp(): void $this->app->initLayout([Layout\Admin::class]); } - protected function createApp(): App - { - return new App([ - 'catchExceptions' => false, - 'alwaysRun' => false, - ]); - } - public function testExecutorFactory(): void { $view = View::addTo($this->app); diff --git a/tests/FormFieldUiTest.php b/tests/FormFieldUiTest.php index a136e1d028..55780f627c 100644 --- a/tests/FormFieldUiTest.php +++ b/tests/FormFieldUiTest.php @@ -30,7 +30,7 @@ class FormFieldUiTest extends TestCase use CreateAppTrait; /** @var Model */ - public $m; + protected $m; protected function setUp(): void { diff --git a/tests/FormTest.php b/tests/FormTest.php index 1409dc8d7e..f2c7df2768 100644 --- a/tests/FormTest.php +++ b/tests/FormTest.php @@ -20,20 +20,17 @@ class FormTest extends TestCase use CreateAppTrait; /** @var Form|null */ - public $form; + protected $form; /** @var string */ - public $formError; + protected $formError; protected function setUp(): void { parent::setUp(); $this->form = new Form(); - $this->form->setApp(new AppFormTestMock([ - 'catchExceptions' => false, - 'alwaysRun' => false, - ])); + $this->form->setApp($this->createApp([AppFormTestMock::class])); $this->form->invokeInit(); } @@ -62,7 +59,7 @@ public function testAddControlAlreadyExistsException(): void * @param \Closure(Model): void $submitFx * @param \Closure(string): void $checkExpectedErrorsFx */ - public function assertFormSubmit(array $postData, \Closure $submitFx = null, \Closure $checkExpectedErrorsFx = null): void + protected function assertFormSubmit(array $postData, \Closure $submitFx = null, \Closure $checkExpectedErrorsFx = null): void { $wasSubmitCalled = false; $_POST = array_merge(array_map(static fn () => '', $this->form->controls), $postData); @@ -133,7 +130,7 @@ public function testTextareaSubmit(): void }); } - public function assertFormControlError(string $field, string $error): void + protected function assertFormControlError(string $field, string $error): void { $n = preg_match_all('~\.form\(\'add prompt\', \'([^\']*)\', \'([^\']*)\'\)~', $this->formError, $matchesAll, \PREG_SET_ORDER); self::assertGreaterThan(0, $n); @@ -148,7 +145,7 @@ public function assertFormControlError(string $field, string $error): void self::assertTrue($matched, 'Form control ' . $field . ' did not produce error'); } - public function assertFormControlNoErrors(string $field): void + protected function assertFormControlNoErrors(string $field): void { $n = preg_match_all('~\.form\(\'add prompt\', \'([^\']*)\', \'([^\']*)\'\)~', $this->formError, $matchesAll, \PREG_SET_ORDER); self::assertGreaterThan(0, $n); diff --git a/tests/GridTest.php b/tests/GridTest.php index a20624c3cc..8c5e83ca39 100644 --- a/tests/GridTest.php +++ b/tests/GridTest.php @@ -15,7 +15,7 @@ class GridTest extends TestCase use TableTestTrait; /** @var MyModel */ - public $m; + protected $m; protected function setUp(): void { diff --git a/tests/TableColumnColorRatingTest.php b/tests/TableColumnColorRatingTest.php index cf6a39d19a..35705882e4 100644 --- a/tests/TableColumnColorRatingTest.php +++ b/tests/TableColumnColorRatingTest.php @@ -16,7 +16,7 @@ class TableColumnColorRatingTest extends TestCase use TableTestTrait; /** @var Table */ - public $table; + protected $table; protected function setUp(): void { diff --git a/tests/TableColumnLinkTest.php b/tests/TableColumnLinkTest.php index 6a270fdbf4..e062252c53 100644 --- a/tests/TableColumnLinkTest.php +++ b/tests/TableColumnLinkTest.php @@ -15,7 +15,7 @@ class TableColumnLinkTest extends TestCase use TableTestTrait; /** @var Table */ - public $table; + protected $table; protected function setUp(): void {