Fix handling of multiple link references with the same index in plain…

… text message (#8021) Second attempt that should work on all supported PHP versions
roundcube · Apr 25, 2021 · 4b5e9c1 · 4b5e9c1
1 parent 32ff7c8
commit 4b5e9c1
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 20 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -27,6 +27,7 @@ CHANGELOG Roundcube Webmail
 - Fix login page rendering after oauth failure (#7812,#7923)
 - Fix bug where assigning users to groups via menu (not drag'n'drop) could fail in Elastic theme (#7973)
 - Fix HTML5 parser issue with a messy HTML code from Outlook (#7356)
+- Fix handling of multiple link references with the same index in plain text message (#8021)
 
 RELEASE 1.5-beta
 ----------------

diff --git a/program/include/rcmail_string_replacer.php b/program/include/rcmail_string_replacer.php
@@ -36,7 +36,7 @@ class rcmail_string_replacer extends rcube_string_replacer
      * @return int Index of saved string value
      * @see rcube_string_replacer::mailto_callback()
      */
-    public function mailto_callback($matches)
+    protected function mailto_callback($matches)
     {
         $href   = $matches[1];
         $suffix = $this->parse_url_brackets($href);

diff --git a/program/lib/Roundcube/rcube_string_replacer.php b/program/lib/Roundcube/rcube_string_replacer.php
@@ -103,7 +103,7 @@ public function get_replacement($i)
      * @return string Return valid link for recognized schemes, otherwise
      *                return the unmodified URL.
      */
-    public function link_callback($matches)
+    protected function link_callback($matches)
     {
         $i          = -1;
         $scheme     = strtolower($matches[1]);
@@ -134,36 +134,53 @@ public function link_callback($matches)
     /**
      * Callback to add an entry to the link index
      *
-     * @param array $matches Matches result from preg_replace_callback
+     * @param array $matches Matches result from preg_replace_callback with PREG_OFFSET_CAPTURE
      *
      * @return string Replacement string
      */
-    public function linkref_addindex($matches)
+    protected function linkref_addindex($matches)
     {
-        $key = $matches[1];
-        $this->linkrefs[$key] = isset($this->urls[$matches[3]]) ? $this->urls[$matches[3]] : null;
+        $key = $matches[1][0];
+
+        if (!isset($this->linkrefs[$key])) {
+            $this->linkrefs[$key] = [];
+        }
 
-        return $this->get_replacement($this->add('['.$key.']')) . $matches[2];
+        // Store the reference and its occurrence position
+        $this->linkrefs[$key][] = [
+            isset($this->urls[$matches[3][0]]) ? $this->urls[$matches[3][0]] : null,
+            $matches[0][1]
+        ];
+
+        return $this->get_replacement($this->add('['.$key.']')) . $matches[2][0];
     }
 
     /**
      * Callback to replace link references with real links
      *
-     * @param array $matches Matches result from preg_replace_callback
+     * @param array $matches Matches result from preg_replace_callback with PREG_OFFSET_CAPTURE
      *
      * @return string Replacement string
      */
-    public function linkref_callback($matches)
+    protected function linkref_callback($matches)
     {
         $i = 0;
-        if (!empty($this->linkrefs[$matches[1]])) {
-            $url    = $this->linkrefs[$matches[1]];
+        $key = $matches[1][0];
+
+        if (!empty($this->linkrefs[$key])) {
             $attrib = isset($this->options['link_attribs']) ? (array) $this->options['link_attribs'] : [];
-            $attrib['href'] = $url;
-            $i = $this->add(html::a($attrib, rcube::Q($matches[1])));
+
+            foreach ($this->linkrefs[$key] as $linkref) {
+                $attrib['href'] = $linkref[0];
+                if ($linkref[1] >= $matches[1][1]) {
+                    break;
+                }
+            }
+
+            $i = $this->add(html::a($attrib, rcube::Q($matches[1][0])));
         }
 
-        return $i > 0 ? '[' . $this->get_replacement($i) . ']' : $matches[0];
+        return $i > 0 ? '[' . $this->get_replacement($i) . ']' : $matches[0][0];
     }
 
     /**
@@ -173,7 +190,7 @@ public function linkref_callback($matches)
      *
      * @return string Replacement string
      */
-    public function mailto_callback($matches)
+    protected function mailto_callback($matches)
     {
         $href   = $matches[1];
         $suffix = $this->parse_url_brackets($href);
@@ -190,7 +207,7 @@ public function mailto_callback($matches)
      *
      * @return string Value at index $matches[1]
      */
-    public function replace_callback($matches)
+    protected function replace_callback($matches)
     {
         return isset($this->values[$matches[1]]) ? $this->values[$matches[1]] : null;
     }
@@ -207,9 +224,36 @@ public function replace($str)
         // search for patterns like links and e-mail addresses
         $str = preg_replace_callback($this->link_pattern, [$this, 'link_callback'], $str);
         $str = preg_replace_callback($this->mailto_pattern, [$this, 'mailto_callback'], $str);
+
         // resolve link references
-        $str = preg_replace_callback($this->linkref_index, [$this, 'linkref_addindex'], $str);
-        $str = preg_replace_callback($this->linkref_pattern, [$this, 'linkref_callback'], $str);
+/*
+        This code requires PHP 7.4 and could be used instead of the two if() statements below,
+        when we get there.
+
+        $str = preg_replace_callback($this->linkref_index,
+            [$this, 'linkref_addindex'], $str, -1, $count, PREG_OFFSET_CAPTURE
+        );
+        $str = preg_replace_callback($this->linkref_pattern,
+            [$this, 'linkref_callback'], $str, -1, $count, PREG_OFFSET_CAPTURE
+        );
+*/
+        if (preg_match_all($this->linkref_index, $str, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER)) {
+            $diff = 0;
+            foreach ($matches as $m) {
+                $replace = $this->linkref_addindex($m);
+                $str     = substr_replace($str, $replace, $m[0][1] + $diff, strlen($m[0][0]));
+                $diff   += strlen($replace) - strlen($m[0][0]);
+            }
+        }
+
+        if (preg_match_all($this->linkref_pattern, $str, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER)) {
+            $diff = 0;
+            foreach ($matches as $m) {
+                $replace = $this->linkref_callback($m);
+                $str     = substr_replace($str, $replace, $m[0][1] + $diff, strlen($m[0][0]));
+                $diff   += strlen($replace) - strlen($m[0][0]);
+            }
+        }
 
         return $str;
     }

diff --git a/tests/Framework/Text2Html.php b/tests/Framework/Text2Html.php
@@ -136,4 +136,25 @@ function test_text2html_xss()
 
         $this->assertEquals($expected, $html);
     }
+
+    /**
+     * Test bug #8021
+     */
+    function test_text2html_8021()
+    {
+        $input = "Test1 [1]\n\n[1] http://d1.tld\n\nyou wrote:\n> Test2 [1]\n>\n> [1] http://d2.tld";
+        $expected = '<div class="pre">Test1 [<a href="http://d1.tld">1</a>]'
+            . "<br>\n<br>\n"
+            . '[1] <a href="http://d1.tld">http://d1.tld</a>'
+            . "<br>\n<br>\n"
+            . 'you wrote:<blockquote>Test2 [<a href="http://d2.tld">1</a>]'
+            . "<br>\n<br>\n"
+            . '[1] <a href="http://d2.tld">http://d2.tld</a></blockquote></div>';
+
+        $t2h = new rcube_text2html($input);
+        $html = $t2h->get_html();
+        $html = preg_replace('/ (rel|target)="(noreferrer|_blank)"/', '', $html);
+
+        $this->assertEquals($expected, $html);
+    }
 }
diff --git a/tests/Rcmail/StringReplacer.php b/tests/Rcmail/StringReplacer.php
@@ -14,11 +14,11 @@ function test_mailto_callback()
     {
         $replacer = new rcmail_string_replacer();
 
-        $result = $replacer->mailto_callback(['email@address.com', 'email@address.com']);
+        $result = invokeMethod($replacer, 'mailto_callback', [['email@address.com', 'email@address.com']]);
 
         $this->assertRegExp($replacer->pattern, $result);
 
-        $result = $replacer->mailto_callback(['address.com', 'address.com']);
+        $result = invokeMethod($replacer, 'mailto_callback', [['address.com', 'address.com']]);
 
         $this->assertSame('address.com', $result);
     }