Fix infinite parser bug, improve tokenizer

csrdelft · Apr 23, 2020 · d474bf4 · d474bf4
1 parent 4ce7b21
commit d474bf4
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 51 deletions.
diff --git a/src/Parser.php b/src/Parser.php
@@ -149,7 +149,7 @@ public function getHtml($bbcode) {
 
         // Create the parsearray with the buildarray function, pretty nice ;)
         $this->tags_counted = 0;
-        $this->parseArray = $this->buildArray($this->bbcode);
+        $this->parseArray = $this->tokenize($this->bbcode);
 
         // Fix html rights
         $this->htmlFix();
@@ -160,66 +160,45 @@ public function getHtml($bbcode) {
         return $this->HTML;
     }
 
-    /**
-     * Breaks the inputted BB code into an array of text and tags
-     */
-    private function buildArray($string) {
+    private function tokenize($str) {
+        $tokens = "[]";
 
-        if (strlen($string) == 0) // Empty or no string
-            return false;
+        $index = 0;
+        $prevIndex = 0;
+        $numTags = 0;
 
-        $opensign = strpos($string, '[');
-        $nextopensign = strpos($string, '[', $opensign + 1);
-        $closesign = strpos($string, ']');
+        $word = strtok($str, $tokens);
 
-        // if there are no more opensigns, or closesigns, or if the closesign is on position 0
-        if ($opensign === false || $closesign == false)
-            return Array($string);
+        while (false !== $word) {
+            $index = strpos($str, $word, $index);
 
-        // Check max tags limit
-        if ($this->tags_counted >= $this->max_tags) {
-            return Array('<b style="color:red">[max # of tags reached, quitting splitting procedure]</b>' . $string);
-        }
+            // This word is a tag if it it surrounded by [ and ]
+            if (strpos($str, "[$word]", $index - 1) === $index - 1) {
+                $words[] = "[$word]";
+                $index += strlen($word) + 1;
 
-        // Nothing's been found yet ;)
-        $found = false;
+                $numTags++;
+            } else {
+                $words[] = substr($str, $prevIndex, $index + strlen($word) - $prevIndex);
+                $index += strlen($word);
+            }
 
-        while (!$found) {
+            $word = strtok($tokens);
 
-            if ($closesign > $opensign && $closesign < $nextopensign) { // Parfait
-                $found = true;
-            } elseif ($closesign > $opensign) { // Maar er komt er nog een opensign
-                $opensign = $nextopensign;
-            } else { // Close is na open!
-                while ($closesign < $opensign) {
-                    $closesign = strpos($string, ']', $closesign + 1);
-                }
-            }
+            $prevIndex = $index;
 
-            $nextopensign = strpos($string, '[', $opensign + 1);
-            if ($nextopensign === false) { // No more opensigns, stop looping
-                $found = true;
+            if ($numTags > $this->max_tags) {
+                return ['<b style="color:red">[max # of tags reached, quitting splitting procedure]</b>' . $str];
             }
         }
 
-        $tag = substr($string, $opensign, $closesign - $opensign + 1);
-        $pretext = substr($string, 0, $opensign);
-
-        $current = Array($tag);
+        // The string can have trailing tokens, we want to return these.
+        $words[] = substr($str, $index);
 
-        // Only non [br] tags must count toward max tag limit.
-        if ($tag != self::BR_TAG) {
-            $this->tags_counted++;
-        }
-
-        if (!empty($pretext))
-            array_unshift($current, $pretext);
+        // Cleanup
+        strtok('', '');
 
-        $rec_arr = $this->buildArray(substr($string, $closesign + 1));
-        if (!is_array($rec_arr)) {
-            $rec_arr = Array($rec_arr);
-        }
-        return array_merge($current, $rec_arr);
+        return $words;
     }
 
     /**

diff --git a/tests/TagsTest.php b/tests/TagsTest.php
@@ -1,10 +1,21 @@
 <?php
 
+namespace CsrDelft\bb\test;
+
 use CsrDelft\bb\DefaultParser;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
-use CsrDelft\bb\Parser;
+use Spatie\Snapshots\Drivers\VarDriver;
 use Spatie\Snapshots\MatchesSnapshots;
 
+class VarDriverPlatformIndependent extends VarDriver {
+    public function match($expected, $actual) {
+        $evaluated = eval(substr($expected, strlen('<?php ')));
+
+        Assert::assertEquals(str_replace("\r\n", "\n", $evaluated), $actual);
+    }
+}
+
 final class TagsTest extends TestCase
 {
     use MatchesSnapshots;
@@ -54,8 +65,11 @@ public function testHeading() {
         $this->assertBbCodeMatchSnapshot("[h=2 id=foo]heading[/h]");
     }
 
-    private function assertBbCodeMatchSnapshot($code) {
+    public function testBrokenCode() {
+        $this->assertBbCodeMatchSnapshot("=][");
+    }
 
-        $this->assertMatchesSnapshot($this->parser->getHtml($code));
+    private function assertBbCodeMatchSnapshot($code) {
+        $this->assertMatchesSnapshot($this->parser->getHtml($code), new VarDriverPlatformIndependent());
     }
 }
diff --git a/tests/__snapshots__/TagsTest__testBrokenCode__1.php b/tests/__snapshots__/TagsTest__testBrokenCode__1.php
@@ -0,0 +1 @@
+<?php return '=][';