Fix broken cookie handling on PHP side

Fix broken cookie path within middleware. For some reason we used the
`/typo3/` path while storing cookies server side. But we used `/` in
JavaScript. That didn't play together and was fixed to always be `/` for
now, but it should be configurable in general. The fix revealed that the
detection of whether to store a cookie was broken, which was fixed
within the corresponding service.

Furthermore the dates how long the cookie should be stored was
different. We now always use 7 days.

Classes/Middleware/CookieSessionMiddleware.php

@@ -97,11 +97,13 @@ class CookieSessionMiddleware implements MiddlewareInterface
             throw new \Exception('Could not retrieve normalized params from request.', 1664357339);
         }
 
+        $days = 7;
+
         return new Cookie(
             $this->cookieSession->getCookieName(),
             $this->cookieSession->getCookieValue(),
-            $GLOBALS['EXEC_TIME'] + 7776000, // 90 days
-            $normalizedParams->getSitePath() . TYPO3_mainDir,
+            $GLOBALS['EXEC_TIME'] + 24 * 60 * 60 * $days,
+            $normalizedParams->getSitePath(),
             '',
             false,
             false,

Classes/Session/CookieSessionService.php

@@ -34,6 +34,14 @@ class CookieSessionService implements SessionServiceInterface
 
     private array $watchlists = [];
 
+    public function __construct(
+    ) {
+        $this->watchlists['default'] = array_filter(explode(
+            ',',
+            $this->getRequest()->getCookieParams()[$this->getCookieName()] ?? ''
+        ));
+    }
+
     // Seems to be a bug leading to different instances if we use constructor.
     public function injectPropertyMapper(PropertyMapper $propertyMapper): void
     {
@@ -42,13 +50,7 @@ class CookieSessionService implements SessionServiceInterface
 
     public function getWatchlist(string $identifier): ?Watchlist
     {
-        $cookieName = $this->getCookieName();
-        $cookie = $this->getRequest()->getCookieParams()[$cookieName] ?? '';
-        $items = array_filter(explode(
-            ',',
-            $this->getRequest()->getCookieParams()['watchlist'] ?? ''
-        ));
-
+        $items = $this->watchlists['default'];
         if ($items === []) {
             return null;
         }

README.rst

@@ -169,3 +169,17 @@ v1.0.1
 
   This is now fixed by properly calling the registerPlugin() method. That way extbase
   can find the CType definition and will add it as CType instead of list_type.
+
+* Fix broken cookie handling.
+
+  * For some reason we used the `/typo3/` path while storing cookies server side.
+    But we used `/` in JavaScript.
+    That didn't play together and was fixed to always be `/` for now, but it should be configurable in general.
+    The fix revealed that the detection of whether to store a cookie was broken, which was fixed within the corresponding service.
+
+  * Furthermore the dates how long the cookie should be stored was different.
+    We now always use 7 days.
+
+  * And we now properly encode the value of the cookie,
+    in order to prevent issues with special meanings like `;`.
+    This was already done on PHP side but not within JS.

Resources/Public/JavaScript/Watchlist.js

@@ -27,7 +27,7 @@
                 return [];
             }
 
-            return cookieValue.split(',');
+            return decodeURIComponent(cookieValue).split(',');
         },
         save: function(items) {
             var cookieValue = items.join(',');
@@ -35,7 +35,7 @@
             if (cookieValue == '') {
                 cookie.delete();
             } else {
-                cookie.set(items.join(','));
+                cookie.set(encodeURIComponent(items.join(',')));
             }
         },
         toggleItem: function(identifier) {

Tests/Functional/BasicsTest.php

@@ -145,7 +145,7 @@ class BasicsTest extends FunctionalTestCase
         self::assertSame('watchlist', $cookie->getName());
         self::assertSame('page-1', $cookie->getValue());
         self::assertNull($cookie->getDomain());
-        self::assertSame('/typo3/', $cookie->getPath());
+        self::assertSame('/', $cookie->getPath());
         self::assertSame('strict', $cookie->getSameSite());
         self::assertFalse($cookie->isSecure());
     }

Tests/Unit/Session/CookieSessionServiceTest.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Copyright (C) 2024 Daniel Siepmann <coding@daniel-siepmann.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+namespace WerkraumMedia\Watchlist\Tests\Unit\Session;
+
+use PHPUnit\Framework\TestCase;
+use TYPO3\CMS\Core\Http\ServerRequest;
+use WerkraumMedia\Watchlist\Session\CookieSessionService;
+
+/**
+ * @covers \WerkraumMedia\Watchlist\Session\CookieSessionService
+ */
+final class CookieSessionServiceTest extends TestCase
+{
+    protected function tearDown(): void
+    {
+        unset($GLOBALS['TYPO3_REQUEST']);
+
+        parent::tearDown();
+    }
+
+    /**
+     * @test
+     */
+    public function returnsCookieValueFromCurrentRequest(): void
+    {
+        $request = $this->createStub(ServerRequest::class);
+        $request->method('getCookieParams')->willReturn([
+            'watchlist' => 'page-1',
+        ]);
+        $GLOBALS['TYPO3_REQUEST'] = $request;
+
+        $subject = new CookieSessionService();
+        $result = $subject->getCookieValue();
+
+        self::assertSame('page-1', $result);
+    }
+}