]> BookStack Code Mirror - bookstack/commitdiff
projects / bookstack / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side (parent: ffceb40)
Prevented page text content includes
authorDan Brown <redacted>
Sat, 5 Jan 2019 17:18:40 +0000 (17:18 +0000)
committerDan Brown <redacted>
Sat, 5 Jan 2019 17:18:40 +0000 (17:18 +0000)
Avoids possible permission issues where included content shown in search or preview
where the user would not normally have permission to view the included content.

Closes #1178

app/Entities/Repos/EntityRepo.php patch | blob | history
app/Entities/Repos/PageRepo.php patch | blob | history
tests/Entity/PageContentTest.php patch | blob | history

diff --git a/app/Entities/Repos/EntityRepo.php b/app/Entities/Repos/EntityRepo.php
index 44fb9ad123dba1ee4de72e70a8bd2e2f90126844..576ed70f00bdedb45597da098cf85e5c0c15a902 100644 (file)
--- a/app/Entities/Repos/EntityRepo.php
+++ b/app/Entities/Repos/EntityRepo.php
@@ -599,24 +599,48 @@ class EntityRepo
     }
 
     /**
-     * Render the page for viewing, Parsing and performing features such as page transclusion.
+     * Render the page for viewing
      * @param Page $page
-     * @param bool $ignorePermissions
-     * @return mixed|string
+     * @param bool $blankIncludes
+     * @return string
      */
-    public function renderPage(Page $page, $ignorePermissions = false)
+    public function renderPage(Page $page, bool $blankIncludes = false) : string
     {
         $content = $page->html;
+
         if (!config('app.allow_content_scripts')) {
             $content = $this->escapeScripts($content);
         }
 
-        $matches = [];
-        preg_match_all("/{{@\s?([0-9].*?)}}/", $content, $matches);
-        if (count($matches[0]) === 0) {
-            return $content;
+        if ($blankIncludes) {
+            $content = $this->blankPageIncludes($content);
+        } else {
+            $content = $this->parsePageIncludes($content);
         }
 
+        return $content;
+    }
+
+    /**
+     * Remove any page include tags within the given HTML.
+     * @param string $html
+     * @return string
+     */
+    protected function blankPageIncludes(string $html) : string
+    {
+        return preg_replace("/{{@\s?([0-9].*?)}}/", '', $html);
+    }
+
+    /**
+     * Parse any include tags "{{@<page_id>#section}}" to be part of the page.
+     * @param string $html
+     * @return mixed|string
+     */
+    protected function parsePageIncludes(string $html) : string
+    {
+        $matches = [];
+        preg_match_all("/{{@\s?([0-9].*?)}}/", $html, $matches);
+
         $topLevelTags = ['table', 'ul', 'ol'];
         foreach ($matches[1] as $index => $includeId) {
             $splitInclude = explode('#', $includeId, 2);
@@ -625,14 +649,14 @@ class EntityRepo
                 continue;
             }
 
-            $matchedPage = $this->getById('page', $pageId, false, $ignorePermissions);
+            $matchedPage = $this->getById('page', $pageId);
             if ($matchedPage === null) {
-                $content = str_replace($matches[0][$index], '', $content);
+                $html = str_replace($matches[0][$index], '', $html);
                 continue;
             }
 
             if (count($splitInclude) === 1) {
-                $content = str_replace($matches[0][$index], $matchedPage->html, $content);
+                $html = str_replace($matches[0][$index], $matchedPage->html, $html);
                 continue;
             }
 
@@ -640,7 +664,7 @@ class EntityRepo
             $doc->loadHTML(mb_convert_encoding('<body>'.$matchedPage->html.'</body>', 'HTML-ENTITIES', 'UTF-8'));
             $matchingElem = $doc->getElementById($splitInclude[1]);
             if ($matchingElem === null) {
-                $content = str_replace($matches[0][$index], '', $content);
+                $html = str_replace($matches[0][$index], '', $html);
                 continue;
             }
             $innerContent = '';
@@ -652,25 +676,22 @@ class EntityRepo
                     $innerContent .= $doc->saveHTML($childNode);
                 }
             }
-            $content = str_replace($matches[0][$index], trim($innerContent), $content);
+            $html = str_replace($matches[0][$index], trim($innerContent), $html);
         }
 
-        return $content;
+        return $html;
     }
 
     /**
      * Escape script tags within HTML content.
      * @param string $html
-     * @return mixed
+     * @return string
      */
-    protected function escapeScripts(string $html)
+    protected function escapeScripts(string $html) : string
     {
         $scriptSearchRegex = '/<script.*?>.*?<\/script>/ms';
         $matches = [];
         preg_match_all($scriptSearchRegex, $html, $matches);
-        if (count($matches) === 0) {
-            return $html;
-        }
 
         foreach ($matches[0] as $match) {
             $html = str_replace($match, htmlentities($match), $html);
diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php
index d9f9c27206ad8e3c726dbb72700781edb5cafef3..3558b29b3d2e109d6b901ee7adc641856c8876fc 100644 (file)
--- a/app/Entities/Repos/PageRepo.php
+++ b/app/Entities/Repos/PageRepo.php
@@ -194,9 +194,9 @@ class PageRepo extends EntityRepo
      * @param \BookStack\Entities\Page $page
      * @return string
      */
-    public function pageToPlainText(Page $page)
+    protected function pageToPlainText(Page $page) : string
     {
-        $html = $this->renderPage($page);
+        $html = $this->renderPage($page, true);
         return strip_tags($html);
     }
 
diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php
index 6a7112bcbac45135353ded3b7e2e4293efff4f70..86abadf147a788d63684a8a71a3a616ef3d57766 100644 (file)
--- a/tests/Entity/PageContentTest.php
+++ b/tests/Entity/PageContentTest.php
@@ -40,15 +40,18 @@ class PageContentTest extends TestCase
     {
         $page = Page::first();
         $secondPage = Page::where('id', '!=', $page->id)->first();
+
         $this->asEditor();
-        $page->html = "<p>{{@$secondPage->id}}</p>";
+        $includeTag = '{{@' . $secondPage->id . '}}';
+        $page->html = '<p>' . $includeTag . '</p>';
 
         $resp = $this->put($page->getUrl(), ['name' => $page->name, 'html' => $page->html, 'summary' => '']);
 
         $resp->assertStatus(302);
 
         $page = Page::find($page->id);
-        $this->assertContains("{{@$secondPage->id}}", $page->html);
+        $this->assertContains($includeTag, $page->html);
+        $this->assertEquals('', $page->text);
     }
 
     public function test_page_includes_do_not_break_tables()
The core source code for the BookStack project.