fixes #3990

Closes #3990

Merge request studip/studip!2839

5 files changed, 96 insertions, 26 deletions

diff --git a/app/views/course/forum/index/_post.php b/app/views/course/forum/index/_post.php
index 7b6aaa0..192a414 100644
--- a/app/views/course/forum/index/_post.php
+++ b/app/views/course/forum/index/_post.php
@@ -91,7 +91,7 @@
 
             <span data-show-topic="<?= $post['topic_id'] ?>" data-topic-content="<?= $post['topic_id'] ?>" <?= $edit_posting != $post['topic_id'] ? '' : 'style="display: none;"' ?>>
                 <?= ForumHelpers::highlight($post['content'], $highlight) ?>
-                <?= OpenGraph::extract(formatReady(ForumEntry::removeQuotes($post['content_raw'])))->render() ?>
+                <?= OpenGraph::extract(ForumEntry::removeQuotes($post['content_raw']))->render() ?>
             </span>
         </div>
 
diff --git a/lib/classes/OpenGraph.php b/lib/classes/OpenGraph.php
index 75326aa..fb0ac5c 100644
--- a/lib/classes/OpenGraph.php
+++ b/lib/classes/OpenGraph.php
@@ -11,34 +11,71 @@ class OpenGraph
     /**
      * Extracts urls and their according open graph infos from a given string
      *
-     * @param String $string Text to extract urls and open graph infos from
+     * @param string $string Text to extract urls and open graph infos from
      * @return OpenGraphURLCollection containing the extracted urls
      */
-    public static function extract($string)
+    public static function extract(string $string): OpenGraphURLCollection
     {
-        $collection = new OpenGraphURLCollection;
-
-        if (Config::get()->OPENGRAPH_ENABLE) {
-            $regexp = StudipCoreFormat::getStudipMarkup('links')['start'];
-            $matched = preg_match_all('/' . $regexp . '/ums', $string, $matches, PREG_SET_ORDER);
-            foreach ($matches as $match) {
-                $url = $match[2];
-
-                if (!$url) {
-                    continue;
-                }
-
-                if (!isLinkIntern($url)) {
-                    $og_url = OpenGraphURL::fromURL($url);
-                    if ($og_url && !$collection->find($og_url->id)) {
-                        $og_url->store();
-
-                        $collection[] = $og_url;
-                    }
-                }
+        $collection = new OpenGraphURLCollection();
+
+        if (!Config::get()->OPENGRAPH_ENABLE) {
+            return $collection;
+        }
+
+        if (Studip\Markup::isHtml($string)) {
+            $urls = self::extractUrlsFromHtml($string);
+        } else {
+            $urls = self::extractUrlsFromText($string);
+        }
+
+        foreach ($urls as $url) {
+            $og_url = OpenGraphURL::fromURL($url);
+            if ($og_url && !$collection->find($og_url->id)) {
+                $og_url->store();
+
+                $collection[] = $og_url;
             }
         }
 
         return $collection;
     }
+
+    public static function filterURLs(array $urls): array
+    {
+        return array_filter($urls, function (string $url): bool {
+            if (!$url) {
+                return false;
+            }
+
+            return !isLinkIntern($url);
+        });
+    }
+
+    public static function extractUrlsFromText(string $text): array
+    {
+        $regexp = StudipCoreFormat::getStudipMarkup('links')['start'];
+        preg_match_all('/' . $regexp . '/ums', $text, $matches, PREG_SET_ORDER);
+        $urls = array_column($matches, 2);
+
+        return self::filterURLs($urls);
+    }
+
+    public static function extractUrlsFromHtml(string $html): array
+    {
+        $document = new DOMDocument();
+        $document->loadHTML($html, LIBXML_NOWARNING | LIBXML_NOERROR);
+
+        $elements = $document->getElementsByTagName('a');
+
+        $urls = [];
+        foreach ($elements as $element) {
+            if (!$element->hasAttribute('href')) {
+                continue;
+            }
+
+            $urls[] = $element->getAttribute('href');
+        }
+
+        return self::filterURLs($urls);
+    }
 }
diff --git a/templates/shared/opengraphinfo_wide.php b/templates/shared/opengraphinfo_wide.php
index ddcd19c..44ce918 100644
--- a/templates/shared/opengraphinfo_wide.php
+++ b/templates/shared/opengraphinfo_wide.php
@@ -18,12 +18,12 @@ if (Config::get()->LOAD_EXTERNAL_MEDIA === "proxy" && Seminar_Session::is_curren
 ?>
 <div class="opengraph <? if (count($videofiles) > 0) echo 'video'; ?> <? if (count($audiofiles) > 0) echo 'audio'; ?>">
 <? if ($og['image'] && count($videofiles) === 0): ?>
-    <a href="<?= URLHelper::getLink($og['url'], [], false) ?>" class="image"
+    <a href="<?= URLHelper::getLink($og['url'], [], true) ?>" class="image"
        target="_blank" rel="noopener noreferrer"
        style="background-image:url(<?= htmlReady($media_url_func($og['image'])) ?>)">
     </a>
 <? endif; ?>
-    <a href="<?= URLHelper::getLink($og['url'], [], false) ?>" class="info"
+    <a href="<?= URLHelper::getLink($og['url'], [], true) ?>" class="info"
        target="_blank" rel="noopener noreferrer">
         <strong><?= htmlReady($og['title']) ?></strong>
     <? if (!count($videofiles)) : ?>
diff --git a/tests/unit/lib/VisualTest.php b/tests/unit/lib/VisualTest.php
index ae9263f..2e75b1f 100644
--- a/tests/unit/lib/VisualTest.php
+++ b/tests/unit/lib/VisualTest.php
@@ -10,7 +10,6 @@
  */
 
 require_once 'lib/classes/SimpleORMap.class.php';
-require_once 'lib/models/OpenGraphURL.class.php';
 require_once 'lib/visual.inc.php';
 require_once 'lib/classes/Config.class.php';
 
diff --git a/tests/unit/lib/classes/OpenGraphTest.php b/tests/unit/lib/classes/OpenGraphTest.php
new file mode 100644
index 0000000..416660b
--- /dev/null
+++ b/tests/unit/lib/classes/OpenGraphTest.php
@@ -0,0 +1,34 @@
+<?php
+/**
+ * @author Jan-Hendrik Willms <tleilax+studip@gmail.com>
+ * @license GPL2 or any later version
+ */
+class OpenGraphTest extends \Codeception\Test\Unit
+{
+    public function setUp(): void
+    {
+        static $config = [
+            'OPENGRAPH_ENABLE'    => true,
+        ];
+
+        Config::set(new Config($config));
+    }
+
+    public function testURLExtraction()
+    {
+        $text = 'this is a link: https://example.org?foo=bar&bar=foo - believe it or not';
+        $urls = OpenGraph::extractUrlsFromText($text);
+
+        $this->assertCount(1, $urls);
+        $this->assertEquals('https://example.org?foo=bar&bar=foo', $urls[0]);
+    }
+
+    public function testURLExtractionFromHTML()
+    {
+        $html = Studip\Markup::HTML_MARKER . '<a href="https://example.org?foo=bar&amp;bar=foo">this is a link</a> - believe it or not';
+        $urls = OpenGraph::extractUrlsFromHtml($html);
+
+        $this->assertCount(1, $urls);
+        $this->assertEquals('https://example.org?foo=bar&bar=foo', $urls[0]);
+    }
+}