Fix an issue with private tags and fix nomarkdown tag (#1399)

Fix an issue with private tags and fix nomarkdown tag

13 files changed, 145 insertions, 33 deletions

diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php
index a56cc92b..9c59e139 100644
--- a/application/bookmark/BookmarkFileService.php
+++ b/application/bookmark/BookmarkFileService.php
@@ -8,6 +8,7 @@ use Exception;
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
 use Shaarli\Bookmark\Exception\EmptyDataStoreException;
 use Shaarli\Config\ConfigManager;
+use Shaarli\Formatter\BookmarkMarkdownFormatter;
 use Shaarli\History;
 use Shaarli\Legacy\LegacyLinkDB;
 use Shaarli\Legacy\LegacyUpdater;
@@ -130,7 +131,7 @@ class BookmarkFileService implements BookmarkServiceInterface
         }
 
         if ($visibility === null) {
-            $visibility = $this->isLoggedIn ? 'all' : 'public';
+            $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC;
         }
 
         $bookmark = $this->bookmarks[$id];
@@ -287,9 +288,13 @@ class BookmarkFileService implements BookmarkServiceInterface
         $caseMapping = [];
         foreach ($bookmarks as $bookmark) {
             foreach ($bookmark->getTags() as $tag) {
-                if (empty($tag) || (! $this->isLoggedIn && startsWith($tag, '.'))) {
+                if (empty($tag)
+                    || (! $this->isLoggedIn && startsWith($tag, '.'))
+                    || $tag === BookmarkMarkdownFormatter::NO_MD_TAG
+                ) {
                     continue;
                 }
+
                 // The first case found will be displayed.
                 if (!isset($caseMapping[strtolower($tag)])) {
                     $caseMapping[strtolower($tag)] = $tag;
diff --git a/application/formatter/BookmarkDefaultFormatter.php b/application/formatter/BookmarkDefaultFormatter.php
index 7550c556..c6c59064 100644
--- a/application/formatter/BookmarkDefaultFormatter.php
+++ b/application/formatter/BookmarkDefaultFormatter.php
@@ -34,7 +34,7 @@ class BookmarkDefaultFormatter extends BookmarkFormatter
      */
     protected function formatTagList($bookmark)
     {
-        return escape($bookmark->getTags());
+        return escape(parent::formatTagList($bookmark));
     }
 
     /**
diff --git a/application/formatter/BookmarkFormatter.php b/application/formatter/BookmarkFormatter.php
index c82c3452..a80d83fc 100644
--- a/application/formatter/BookmarkFormatter.php
+++ b/application/formatter/BookmarkFormatter.php
@@ -20,6 +20,9 @@ abstract class BookmarkFormatter
      */
     protected $conf;
 
+    /** @var bool */
+    protected $isLoggedIn;
+
     /**
      * @var array Additional parameters than can be used for specific formatting
      *            e.g. index_url for Feed formatting
@@ -30,9 +33,10 @@ abstract class BookmarkFormatter
      * LinkDefaultFormatter constructor.
      * @param ConfigManager $conf
      */
-    public function __construct(ConfigManager $conf)
+    public function __construct(ConfigManager $conf, bool $isLoggedIn)
     {
         $this->conf = $conf;
+        $this->isLoggedIn = $isLoggedIn;
     }
 
     /**
@@ -172,7 +176,7 @@ abstract class BookmarkFormatter
      */
     protected function formatTagList($bookmark)
     {
-        return $bookmark->getTags();
+        return $this->filterTagList($bookmark->getTags());
     }
 
     /**
@@ -184,7 +188,7 @@ abstract class BookmarkFormatter
      */
     protected function formatTagString($bookmark)
     {
-        return implode(' ', $bookmark->getTags());
+        return implode(' ', $this->formatTagList($bookmark));
     }
 
     /**
@@ -253,4 +257,29 @@ abstract class BookmarkFormatter
         }
         return 0;
     }
+
+    /**
+     * Format tag list, e.g. remove private tags if the user is not logged in.
+     *
+     * @param array $tags
+     *
+     * @return array
+     */
+    protected function filterTagList(array $tags): array
+    {
+        if ($this->isLoggedIn === true) {
+            return $tags;
+        }
+
+        $out = [];
+        foreach ($tags as $tag) {
+            if (strpos($tag, '.') === 0) {
+                continue;
+            }
+
+            $out[] = $tag;
+        }
+
+        return $out;
+    }
 }
diff --git a/application/formatter/BookmarkMarkdownFormatter.php b/application/formatter/BookmarkMarkdownFormatter.php
index 7797bfbf..077e5312 100644
--- a/application/formatter/BookmarkMarkdownFormatter.php
+++ b/application/formatter/BookmarkMarkdownFormatter.php
@@ -36,10 +36,12 @@ class BookmarkMarkdownFormatter extends BookmarkDefaultFormatter
      * LinkMarkdownFormatter constructor.
      *
      * @param ConfigManager $conf instance
+     * @param bool          $isLoggedIn
      */
-    public function __construct(ConfigManager $conf)
+    public function __construct(ConfigManager $conf, bool $isLoggedIn)
     {
-        parent::__construct($conf);
+        parent::__construct($conf, $isLoggedIn);
+
         $this->parsedown = new \Parsedown();
         $this->escape = $conf->get('security.markdown_escape', true);
         $this->allowedProtocols = $conf->get('security.allowed_protocols', []);
@@ -79,7 +81,7 @@ class BookmarkMarkdownFormatter extends BookmarkDefaultFormatter
     protected function formatTagList($bookmark)
     {
         $out = parent::formatTagList($bookmark);
-        if (($pos = array_search(self::NO_MD_TAG, $out)) !== false) {
+        if ($this->isLoggedIn === false && ($pos = array_search(self::NO_MD_TAG, $out)) !== false) {
             unset($out[$pos]);
             return array_values($out);
         }
diff --git a/application/formatter/FormatterFactory.php b/application/formatter/FormatterFactory.php
index 0d2c0466..5f282f68 100644
--- a/application/formatter/FormatterFactory.php
+++ b/application/formatter/FormatterFactory.php
@@ -16,14 +16,19 @@ class FormatterFactory
     /** @var ConfigManager instance */
     protected $conf;
 
+    /** @var bool */
+    protected $isLoggedIn;
+
     /**
      * FormatterFactory constructor.
      *
      * @param ConfigManager $conf
+     * @param bool          $isLoggedIn
      */
-    public function __construct(ConfigManager $conf)
+    public function __construct(ConfigManager $conf, bool $isLoggedIn)
     {
         $this->conf = $conf;
+        $this->isLoggedIn = $isLoggedIn;
     }
 
     /**
@@ -33,7 +38,7 @@ class FormatterFactory
      *
      * @return BookmarkFormatter instance.
      */
-    public function getFormatter($type = null)
+    public function getFormatter(string $type = null)
     {
         $type = $type ? $type : $this->conf->get('formatter', 'default');
         $className = '\\Shaarli\\Formatter\\Bookmark'. ucfirst($type) .'Formatter';
@@ -41,6 +46,6 @@ class FormatterFactory
             $className = '\\Shaarli\\Formatter\\BookmarkDefaultFormatter';
         }
 
-        return new $className($this->conf);
+        return new $className($this->conf, $this->isLoggedIn);
     }
 }
diff --git a/index.php b/index.php
index 013b01af..a39fc762 100644
--- a/index.php
+++ b/index.php
@@ -70,6 +70,7 @@ use Shaarli\Bookmark\BookmarkFileService;
 use \Shaarli\Config\ConfigManager;
 use \Shaarli\Feed\CachedPage;
 use \Shaarli\Feed\FeedBuilder;
+use Shaarli\Formatter\BookmarkMarkdownFormatter;
 use Shaarli\Formatter\FormatterFactory;
 use \Shaarli\History;
 use \Shaarli\Languages;
@@ -351,7 +352,7 @@ function showDailyRSS($bookmarkService, $conf, $loginManager)
     echo '<language>en-en</language>';
     echo '<copyright>'. $pageaddr .'</copyright>'. PHP_EOL;
 
-    $factory = new FormatterFactory($conf);
+    $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
     $formatter = $factory->getFormatter();
     $formatter->addContextData('index_url', index_url($_SERVER));
     // For each day.
@@ -441,7 +442,7 @@ function showDaily($pageBuilder, $bookmarkService, $conf, $pluginManager, $login
         $linksToDisplay = [];
     }
 
-    $factory = new FormatterFactory($conf);
+    $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
     $formatter = $factory->getFormatter();
     // We pre-format some fields for proper output.
     foreach ($linksToDisplay as $key => $bookmark) {
@@ -630,7 +631,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
         // Get only bookmarks which have a thumbnail.
         // Note: we do not retrieve thumbnails here, the request is too heavy.
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         $formatter = $factory->getFormatter();
         foreach ($links as $key => $link) {
             if ($link->getThumbnail() !== false) {
@@ -753,7 +754,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
             exit;
         }
 
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         // Generate data.
         $feedGenerator = new FeedBuilder(
             $bookmarkService,
@@ -1183,7 +1184,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
         $bookmarkService->addOrSet($bookmark, false);
 
         // To preserve backward compatibility with 3rd parties, plugins still use arrays
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         $formatter = $factory->getFormatter('raw');
         $data = $formatter->format($bookmark);
         $pluginManager->executeHooks('save_link', $data);
@@ -1230,7 +1231,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
         if (!count($ids)) {
             die('no id provided');
         }
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         $formatter = $factory->getFormatter('raw');
         foreach ($ids as $id) {
             $id = (int) escape($id);
@@ -1286,7 +1287,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
         } else {
             $private = $_GET['newVisibility'] === 'private';
         }
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         $formatter = $factory->getFormatter('raw');
         foreach ($ids as $id) {
             $id = (int) escape($id);
@@ -1324,14 +1325,18 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
             exit;
         }
 
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         $formatter = $factory->getFormatter('raw');
         $formattedLink = $formatter->format($link);
+        $tags = $bookmarkService->bookmarksCountPerTag();
+        if ($conf->get('formatter') === 'markdown') {
+            $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
+        }
         $data = array(
             'link' => $formattedLink,
             'link_is_new' => false,
             'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''),
-            'tags' => $bookmarkService->bookmarksCountPerTag(),
+            'tags' => $tags,
         );
         $pluginManager->executeHooks('render_editlink', $data);
 
@@ -1391,17 +1396,21 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
                 'private' => $private,
             ];
         } else {
-            $factory = new FormatterFactory($conf);
-        $formatter = $factory->getFormatter('raw');
+            $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
+            $formatter = $factory->getFormatter('raw');
             $link = $formatter->format($bookmark);
         }
 
+        $tags = $bookmarkService->bookmarksCountPerTag();
+        if ($conf->get('formatter') === 'markdown') {
+            $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
+        }
         $data = [
             'link' => $link,
             'link_is_new' => $link_is_new,
             'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''),
             'source' => (isset($_GET['source']) ? $_GET['source'] : ''),
-            'tags' => $bookmarkService->bookmarksCountPerTag(),
+            'tags' => $tags,
             'default_private_links' => $conf->get('privacy.default_private_links', false),
         ];
         $pluginManager->executeHooks('render_editlink', $data);
@@ -1451,7 +1460,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
         }
 
         try {
-            $factory = new FormatterFactory($conf);
+            $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
             $formatter = $factory->getFormatter('raw');
             $PAGE->assign(
                 'links',
@@ -1633,7 +1642,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
         $bookmark->setThumbnail($thumbnailer->get($bookmark->getUrl()));
         $bookmarkService->set($bookmark);
 
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
         echo json_encode($factory->getFormatter('raw')->format($bookmark));
         exit;
     }
@@ -1655,7 +1664,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
  */
 function buildLinkList($PAGE, $linkDb, $conf, $pluginManager, $loginManager)
 {
-    $factory = new FormatterFactory($conf);
+    $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
     $formatter = $factory->getFormatter();
 
     // Used in templates
diff --git a/tests/bookmark/BookmarkFileServiceTest.php b/tests/bookmark/BookmarkFileServiceTest.php
index 1b438a7f..4900d41d 100644
--- a/tests/bookmark/BookmarkFileServiceTest.php
+++ b/tests/bookmark/BookmarkFileServiceTest.php
@@ -12,6 +12,7 @@ use ReflectionClass;
 use Shaarli;
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
 use Shaarli\Config\ConfigManager;
+use Shaarli\Formatter\BookmarkMarkdownFormatter;
 use Shaarli\History;
 
 /**
@@ -1026,6 +1027,46 @@ class BookmarkFileServiceTest extends TestCase
     }
 
     /**
+     * Test linksCountPerTag public tags with filter.
+     * Equal occurrences should be sorted alphabetically.
+     */
+    public function testCountTagsNoMarkdown()
+    {
+        $expected = [
+            'cartoon' => 3,
+            'dev' => 2,
+            'tag1' => 1,
+            'tag2' => 1,
+            'tag3' => 1,
+            'tag4' => 1,
+            'web' => 4,
+            'gnu' => 2,
+            'hashtag' => 2,
+            'sTuff' => 2,
+            '-exclude' => 1,
+            '.hidden' => 1,
+            'Mercurial' => 1,
+            'css' => 1,
+            'free' => 1,
+            'html' => 1,
+            'media' => 1,
+            'newTagToCount' => 1,
+            'samba' => 1,
+            'software' => 1,
+            'stallman' => 1,
+            'ut' => 1,
+            'w3c' => 1,
+        ];
+        $bookmark = new Bookmark();
+        $bookmark->setTags(['newTagToCount', BookmarkMarkdownFormatter::NO_MD_TAG]);
+        $this->privateLinkDB->add($bookmark);
+
+        $tags = $this->privateLinkDB->bookmarksCountPerTag();
+
+        $this->assertEquals($expected, $tags, var_export($tags, true));
+    }
+
+    /**
      * Allows to test LinkDB's private methods
      *
      * @see
diff --git a/tests/feed/FeedBuilderTest.php b/tests/feed/FeedBuilderTest.php
index a43ff672..54671891 100644
--- a/tests/feed/FeedBuilderTest.php
+++ b/tests/feed/FeedBuilderTest.php
@@ -51,7 +51,7 @@ class FeedBuilderTest extends \PHPUnit\Framework\TestCase
         $refLinkDB = new \ReferenceLinkDB();
         $refLinkDB->write(self::$testDatastore);
         $history = new History('sandbox/history.php');
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, true);
         self::$formatter = $factory->getFormatter();
         self::$bookmarkService = new BookmarkFileService($conf, $history, true);
 
diff --git a/tests/formatter/BookmarkDefaultFormatterTest.php b/tests/formatter/BookmarkDefaultFormatterTest.php
index fe42a208..382a560e 100644
--- a/tests/formatter/BookmarkDefaultFormatterTest.php
+++ b/tests/formatter/BookmarkDefaultFormatterTest.php
@@ -29,7 +29,7 @@ class BookmarkDefaultFormatterTest extends TestCase
     {
         copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
         $this->conf = new ConfigManager(self::$testConf);
-        $this->formatter = new BookmarkDefaultFormatter($this->conf);
+        $this->formatter = new BookmarkDefaultFormatter($this->conf, true);
     }
 
     /**
@@ -153,4 +153,25 @@ class BookmarkDefaultFormatterTest extends TestCase
             $link['description']
         );
     }
+
+    /**
+     * Make sure that private tags are properly filtered out when the user is logged out.
+     */
+    public function testFormatTagListRemovePrivate(): void
+    {
+        $this->formatter = new BookmarkDefaultFormatter($this->conf, false);
+
+        $bookmark = new Bookmark();
+        $bookmark->setId($id = 11);
+        $bookmark->setTags($tags = ['bookmark', '.private', 'othertag']);
+
+        $link = $this->formatter->format($bookmark);
+
+        unset($tags[1]);
+        $tags = array_values($tags);
+
+        $this->assertSame(11, $link['id']);
+        $this->assertSame($tags, $link['taglist']);
+        $this->assertSame(implode(' ', $tags), $link['tags']);
+    }
 }
diff --git a/tests/formatter/BookmarkMarkdownFormatterTest.php b/tests/formatter/BookmarkMarkdownFormatterTest.php
index 0ca7f802..f1f12c04 100644
--- a/tests/formatter/BookmarkMarkdownFormatterTest.php
+++ b/tests/formatter/BookmarkMarkdownFormatterTest.php
@@ -29,7 +29,7 @@ class BookmarkMarkdownFormatterTest extends TestCase
     {
         copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
         $this->conf = new ConfigManager(self::$testConf);
-        $this->formatter = new BookmarkMarkdownFormatter($this->conf);
+        $this->formatter = new BookmarkMarkdownFormatter($this->conf, true);
     }
 
     /**
diff --git a/tests/formatter/BookmarkRawFormatterTest.php b/tests/formatter/BookmarkRawFormatterTest.php
index ceb6fb73..4491b035 100644
--- a/tests/formatter/BookmarkRawFormatterTest.php
+++ b/tests/formatter/BookmarkRawFormatterTest.php
@@ -29,7 +29,7 @@ class BookmarkRawFormatterTest extends TestCase
     {
         copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
         $this->conf = new ConfigManager(self::$testConf);
-        $this->formatter = new BookmarkRawFormatter($this->conf);
+        $this->formatter = new BookmarkRawFormatter($this->conf, true);
     }
 
     /**
diff --git a/tests/formatter/FormatterFactoryTest.php b/tests/formatter/FormatterFactoryTest.php
index 317c0b2d..5adf3ffd 100644
--- a/tests/formatter/FormatterFactoryTest.php
+++ b/tests/formatter/FormatterFactoryTest.php
@@ -28,7 +28,7 @@ class FormatterFactoryTest extends TestCase
     {
         copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
         $this->conf = new ConfigManager(self::$testConf);
-        $this->factory = new FormatterFactory($this->conf);
+        $this->factory = new FormatterFactory($this->conf, true);
     }
 
     /**
diff --git a/tests/netscape/BookmarkExportTest.php b/tests/netscape/BookmarkExportTest.php
index 011d19ac..6c948bba 100644
--- a/tests/netscape/BookmarkExportTest.php
+++ b/tests/netscape/BookmarkExportTest.php
@@ -46,7 +46,7 @@ class BookmarkExportTest extends \PHPUnit\Framework\TestCase
         self::$refDb->write(self::$testDatastore);
         $history = new History('sandbox/history.php');
         self::$bookmarkService = new BookmarkFileService($conf, $history, true);
-        $factory = new FormatterFactory($conf);
+        $factory = new FormatterFactory($conf, true);
         self::$formatter = $factory->getFormatter('raw');
     }