add a list of reserved page IDs containing 'media' and 'assets'. close #503

refactor: Modelpage::add() throws exceptions instead of returning a bool

avoid infinite loop by deactivating Apache trailing slash in .htaccess

Author: vincent-peugnet

.htaccess

@@ -1,7 +1,12 @@
+# prevent Apache from adding trailing slash for names that match folder
+# in order to avoid conflict with Controllerpage::pagepermanentredirect()
+# and 'assets' and 'media' folder
+DirectorySlash off
 RewriteEngine on
 # everything that does not contain asssets|media
 RewriteCond %{REQUEST_URI} !^(.*)/(assets|media)/ [OR]
 # or that isn't a file
 RewriteCond %{REQUEST_FILENAME} !-f
 # is redirect to index
 RewriteRule . index.php [L]
+

MANUAL.md

@@ -291,7 +291,7 @@ This will print a HTML list of every [authors](#authors) of the page. If they ha
 
     %PAGEID% | %ID%
 
-You have two options to print the page ID.
+You have two options to print the [page ID](#page-id).
 
 This is powerfull when combined with [URL commands](#url-based-command-interface) and [BODY templating](#body-template).
 
@@ -954,6 +954,8 @@ Page metadatas can be set through the page [edition interface](#edition-interfac
 ##### Page ID
 
 __The unique identifier of a page__. It can only contain lowercases characters from `a-z`, numbers `0-9`, underscore `_` and hyphen `-`.
+Also, `assets` and `media` are reserved paths, therefore cannot be used as page ID.
+
 Normaly W will take care of cleaning your pages's ID, by lowering uppercases, removing some accents, and remplacing special characters or spaces with hyphens.
 
 

app/class/Controllerapipage.php

@@ -97,6 +97,9 @@ public function update(string $page)
         }
     }
 
+    /**
+     * @throws RuntimeException when saving page fails
+     */
     public function add(string $page)
     {
         if (!Model::idcheck($page)) {
@@ -111,13 +114,12 @@ public function add(string $page)
 
         $this->page = $this->pagemanager->newpage(array_merge($this->recievejson(), ['id' => $page]));
         $this->page->addauthor($this->user->id());
-        if ($this->pagemanager->add($this->page)) {
-            http_response_code(200);
-        } else {
-            http_response_code(500);
-        }
+        $this->pagemanager->add($this->page);
     }
 
+    /**
+     * @throws RuntimeException when saving page fails
+     */
     public function put(string $page)
     {
         if (!Model::idcheck($page)) {
@@ -134,11 +136,8 @@ public function put(string $page)
         if (!$exist) { // If it's a page creation, add the user as an author
             $this->page->addauthor($this->user->id());
         }
-        if ($this->pagemanager->add($this->page)) {
-            http_response_code($exist ? 200 : 201);
-        } else {
-            $this->shortresponse(500, "Error while trying to save page in database");
-        }
+        $this->pagemanager->add($this->page);
+        http_response_code($exist ? 200 : 201);
     }
 
     public function delete(string $page)

app/class/Controllerpage.php

@@ -20,6 +20,13 @@ public function __construct($router)
         $this->mediamanager = new Modelmedia();
     }
 
+    /**
+     * Check a given page ID. If it's not a valid ID, then redirect to the clean ID version.
+     * If it's valid, setup $this->page` with a new empty Page using given ID.
+     *
+     * @param $id                           The id received by the router
+     * @param $route                        The route that was used
+     */
     protected function setpage(string $id, string $route): void
     {
         $cleanid = Model::idclean($id);
@@ -318,25 +325,39 @@ public function log($page): void
         }
     }
 
+    /**
+     * When a client want to add a page.
+     * Match domain.com/PAGE_ID/add
+     *
+     * @throws RuntimeException if page creation failed
+     */
     public function add(string $page): void
     {
         $this->setpage($page, 'pageadd');
 
         $this->pageconnect('pageadd');
 
-        if ($this->user->iseditor() && !$this->importpage()) {
-            $this->page->reset();
-            if (isset($_SESSION['dirtyid'])) {
-                $this->page->settitle($_SESSION['dirtyid'][$page]);
-                unset($_SESSION['dirtyid']);
-            }
-            $this->page->addauthor($this->user->id());
-            $this->pagemanager->add($this->page);
-            $this->routedirect('pageedit', ['page' => $this->page->id()]);
-        } else {
+        if (!$this->user->iseditor()) {
             http_response_code(403);
             $this->showtemplate('forbidden', ['route' => 'pageedit', 'id' => $this->page->id()]);
+            exit;
         }
+
+        if ($this->importpage()) {
+            http_response_code(403);
+            $message = 'page already exist with this ID';
+            $this->showtemplate('forbidden', ['route' => 'pageedit', 'id' => $this->page->id(), 'message' => $message]);
+            exit;
+        }
+
+        $this->page->reset();
+        if (isset($_SESSION['dirtyid'])) {
+            $this->page->settitle($_SESSION['dirtyid'][$page]);
+            unset($_SESSION['dirtyid']);
+        }
+        $this->page->addauthor($this->user->id());
+        $this->pagemanager->add($this->page);
+        $this->routedirect('pageedit', ['page' => $this->page->id()]);
     }
 
     public function addascopy(string $page, string $copy): void
@@ -403,8 +424,12 @@ public function upload(): void
             $page->setdaterender($page->datecreation('date'));
 
             if ($_POST['erase'] || !$this->pagemanager->exist($page)) {
-                if ($this->pagemanager->add($page)) {
+                try {
+                    $this->pagemanager->add($page);
                     $this->sendflashmessage('Page successfully uploaded', self::FLASH_SUCCESS);
+                } catch (RuntimeException $e) {
+                    $this->sendflashmessage($e->getMessage(), self::FLASH_ERROR);
+                    Logger::errorex($e);
                 }
             } else {
                 $this->sendflashmessage(

app/class/Modelpage.php

@@ -10,11 +10,17 @@
 use InvalidArgumentException;
 use RangeException;
 use RuntimeException;
+use Wcms\Exception\Databaseexception;
 use Wcms\Exception\Filesystemexception;
 use Wcms\Exception\Filesystemexception\Notfoundexception;
 
 class Modelpage extends Modeldb
 {
+    public const RESERVED_IDS = [
+        'media',
+        'assets',
+    ];
+
     public const SECURE_LEVELS = [
         0 => 'public',
         1 => 'private',
@@ -82,14 +88,21 @@ public function pagelistbyid(array $idlist = []): array
      * Store new page in the database
      *
      * @param Page $page                    Page object
-     * @return bool                         depending on database storing
+     *
+     * @throws RuntimeException if page ID is illegal
+     * @throws Databaseexception if error occured whiling saving document
      */
-    public function add(Page $page): bool
+    public function add(Page $page): void
     {
-
+        if (in_array($page->id(), self::RESERVED_IDS)) {
+            $id = $page->id();
+            throw new RuntimeException("'$id' is a reserved page ID");
+        }
         $pagedata = new Document($page->dry());
         $pagedata->setId($page->id());
-        return $this->storedoc($pagedata);
+        if (!$this->storedoc($pagedata)) {
+            throw new Databaseexception('Error wile trying to save document to database. Check logs for more info');
+        }
     }
 
     /**