Always assert request URL path starts with '/' (#2110)

atk4 · Sep 18, 2023 · 1d6804f · 1d6804f
1 parent 39cdb6b
commit 1d6804f
Show file tree

Hide file tree

Showing 11 changed files with 56 additions and 46 deletions.
diff --git 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
@@ -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
@@ -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
@@ -26,17 +26,19 @@ protected function outputResponse(string $data): void
 
 class CallbackTest extends TestCase
 {
+    use CreateAppTrait;
+
     /** @var string */
     private $htmlDoctypeRegex = '~^<!DOCTYPE~';
 
     /** @var App */
-    public $app;
+    protected $app;
 
     protected function setUp(): void
     {
         parent::setUp();
 
-        $this->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
@@ -5,17 +5,25 @@
 namespace Atk4\Ui\Tests;
 
 use Atk4\Ui\App;
+use Nyholm\Psr7\Factory\Psr17Factory;
 
 trait CreateAppTrait
 {
     /**
-     * @param array<string, mixed> $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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -15,7 +15,7 @@ class TableColumnLinkTest extends TestCase
     use TableTestTrait;
 
     /** @var Table */
-    public $table;
+    protected $table;
 
     protected function setUp(): void
     {