Make session storage a bit more resilliant

Author: Adam Tomat

src/Session/EncryptedStore.php

@@ -32,10 +32,14 @@ protected function prepareForStorage($data)
 
     protected function prepareForUnserialize($data)
     {
+        if ($data === '') {
+            return '';
+        }
+
         try {
             return $this->encrypter->decrypt($data);
         } catch (Exception $e) {
-            $this->exceptionHandler->report($e);
+            $this->exceptionHandler?->report($e);
             return '';
         }
     }

src/Session/FileSessionHandler.php

@@ -8,8 +8,13 @@
 
 class FileSessionHandler implements SessionHandlerInterface
 {
-    public function __construct(protected $path, protected $prefix = 'lumberjack_session_')
+    protected $path;
+    protected $prefix;
+
+    public function __construct($path, $prefix = 'lumberjack_session_')
     {
+        $this->path = $path;
+        $this->prefix = $prefix;
     }
 
     #[\ReturnTypeWillChange]

src/Session/SessionManager.php

@@ -5,6 +5,7 @@
 use Rareloop\Lumberjack\Application;
 use Rareloop\Lumberjack\Config;
 use Rareloop\Lumberjack\Contracts\Encrypter as EncrypterContract;
+use Rareloop\Lumberjack\Exceptions\HandlerInterface;
 use Rareloop\Lumberjack\Manager;
 
 class SessionManager extends Manager
@@ -54,8 +55,9 @@ protected function buildSession($handler)
 
         if ($this->config->get('session.encrypt')) {
             $encrypter = $this->app->get(EncrypterContract::class);
+            $exceptionHandler = $this->app->get(HandlerInterface::class);
 
-            return new EncryptedStore($this->name, $handler, $encrypter, $sessionId);
+            return new EncryptedStore($this->name, $handler, $encrypter, $sessionId, $exceptionHandler);
         }
 
         return new Store($this->name, $handler, $sessionId);

tests/Unit/Exceptions/HandlerTest.php

@@ -124,6 +124,6 @@ class HandlerWithBlacklist extends Handler
     ];
 }
 
-class BlacklistedException extends \Exception 
+class BlacklistedException extends \Exception
 {
 }

tests/Unit/Session/EncryptedStoreTest.php

@@ -54,15 +54,14 @@ public function data_is_decrypted_before_it_is_loaded()
 
     /**
      * @test
-     * @dataProvider unexpectedSessionData
      */
-    public function unexpected_session_data_is_handled_gracefully($previousSessionValue)
+    public function unexpected_session_data_is_handled_gracefully()
     {
         $encryptionKey = 'encryption-key';
 
         // Use a mock handler to fake a previously stored state
         $handler = Mockery::mock(NullSessionHandler::class . '[read]');
-        $handler->shouldReceive('read')->andReturn($previousSessionValue);
+        $handler->shouldReceive('read')->andReturn(@serialize(['foo' => 'bar']));
 
         $errorHandler = Mockery::mock(HandlerInterface::class);
         $errorHandler->shouldReceive('report')->once();
@@ -73,11 +72,40 @@ public function unexpected_session_data_is_handled_gracefully($previousSessionVa
         $this->assertSame(null, $store->get('foo'));
     }
 
-    public function unexpectedSessionData()
+    /**
+     * @test
+     */
+    public function gracefully_handle_case_with_no_exception_handler()
     {
-        return [
-            [@serialize(['foo' => 'bar'])],
-            [''],
-        ];
+        $encryptionKey = 'encryption-key';
+
+        // Use a mock handler to fake a previously stored state
+        $handler = Mockery::mock(NullSessionHandler::class . '[read]');
+        $handler->shouldReceive('read')->andReturn(@serialize(['foo' => 'bar']));
+
+        $store = new EncryptedStore('session-name', $handler, new Encrypter($encryptionKey), 'session-id');
+        $store->start();
+
+        $this->assertSame(null, $store->get('foo'));
+    }
+
+    /**
+     * @test
+     */
+    public function empty_session_data_is_ignored()
+    {
+        $encryptionKey = 'encryption-key';
+
+        // Use a mock handler to fake a previously stored state
+        $handler = Mockery::mock(NullSessionHandler::class . '[read]');
+        $handler->shouldReceive('read')->andReturn('');
+
+        $errorHandler = Mockery::mock(HandlerInterface::class);
+        $errorHandler->shouldNotHaveReceived('report');
+
+        $store = new EncryptedStore('session-name', $handler, new Encrypter($encryptionKey), 'session-id', $errorHandler);
+        $store->start();
+
+        $this->assertSame(null, $store->get('foo'));
     }
 }

tests/Unit/Session/SessionManagerTest.php

@@ -3,17 +3,20 @@
 namespace Rareloop\Lumberjack\Test;
 
 use Mockery;
+use ReflectionClass;
+use org\bovigo\vfs\vfsStream;
 use PHPUnit\Framework\TestCase;
-use Rareloop\Lumberjack\Application;
 use Rareloop\Lumberjack\Config;
-use Rareloop\Lumberjack\Contracts\Encrypter as EncrypterContract;
 use Rareloop\Lumberjack\Encrypter;
+use Rareloop\Lumberjack\Application;
+use Rareloop\Lumberjack\Session\Store;
+use Rareloop\Lumberjack\Exceptions\Handler;
 use Rareloop\Lumberjack\Session\EncryptedStore;
-use Rareloop\Lumberjack\Session\FileSessionHandler;
 use Rareloop\Lumberjack\Session\SessionManager;
-use Rareloop\Lumberjack\Session\Store;
+use Rareloop\Lumberjack\Session\FileSessionHandler;
+use Rareloop\Lumberjack\Exceptions\HandlerInterface;
 use Rareloop\Lumberjack\Test\Unit\Session\NullSessionHandler;
-use org\bovigo\vfs\vfsStream;
+use Rareloop\Lumberjack\Contracts\Encrypter as EncrypterContract;
 
 class SessionManagerTest extends TestCase
 {
@@ -93,12 +96,22 @@ public function can_create_an_unencrypted_store()
     /** @test */
     public function can_create_an_encrypted_store()
     {
-        $app = $app = $this->appWithSessionDriverConfig('file', 'lumberjack', $encrypted = true);
+        $app = $this->appWithSessionDriverConfig('file', 'lumberjack', $encrypted = true);
         $app->bind(EncrypterContract::class, new Encrypter('encryption-key'));
 
+        $handler = Mockery::mock(Handler::class);
+        $app->bind(HandlerInterface::class, $handler);
+
         $manager = new SessionManager($app);
 
-        $this->assertInstanceOf(EncryptedStore::class, $manager->driver());
+        $driver = $manager->driver();
+        $this->assertInstanceOf(EncryptedStore::class, $driver);
+
+        $reflection = new ReflectionClass($driver);
+        $property = $reflection->getProperty('exceptionHandler');
+        $property->setAccessible(true);
+
+        $this->assertInstanceOf(HandlerInterface::class, $property->getValue($driver));
     }
 }