[HttpFoundation] MongoDbSessionHandler::read() now checks for valid session age

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php

@@ -42,6 +42,24 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
      *  * id_field: The field name for storing the session id [default: _id]
      *  * data_field: The field name for storing the session data [default: data]
      *  * time_field: The field name for storing the timestamp [default: time]
+     *  * expiry_field: The field name for storing the expiry-timestamp [default: expires_at]
+     *
+     * It is strongly recommended to put an index on the `expiry_field` for
+     * garbage-collection. Alternatively it's possible to automatically expire
+     * the sessions in the database as described below:
+     *
+     * A TTL collections can be used on MongoDB 2.2+ to cleanup expired sessions
+     * automatically. Such an index can for example look like this:
+     *
+     *     db.<session-collection>.ensureIndex(
+     *         { "<expiry-field>": 1 },
+     *         { "expireAfterSeconds": 0 }
+     *     )
+     *
+     * More details on: http://docs.mongodb.org/manual/tutorial/expire-data/
+     *
+     * If you use such an index, you can drop `gc_probability` to 0 since
+     * no garbage-collection is required.
      *
      * @param \Mongo|\MongoClient $mongo   A MongoClient or Mongo instance
      * @param array               $options An associative array of field options
@@ -65,6 +83,7 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
             'id_field' => '_id',
             'data_field' => 'data',
             'time_field' => 'time',
+            'expiry_field' => 'expires_at',
         ), $options);
     }
 
@@ -101,18 +120,8 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
      */
     public function gc($maxlifetime)
     {
-        /* Note: MongoDB 2.2+ supports TTL collections, which may be used in
-         * place of this method by indexing the "time_field" field with an
-         * "expireAfterSeconds" option. Regardless of whether TTL collections
-         * are used, consider indexing this field to make the remove query more
-         * efficient.
-         *
-         * See: http://docs.mongodb.org/manual/tutorial/expire-data/
-         */
-        $time = new \MongoDate(time() - $maxlifetime);
-
         $this->getCollection()->remove(array(
-            $this->options['time_field'] => array('$lt' => $time),
+            $this->options['expiry_field'] => array('$lt' => new \MongoDate()),
         ));
 
         return true;
@@ -123,12 +132,17 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
      */
     public function write($sessionId, $data)
     {
+        $expiry = new \MongoDate(time() + (int) ini_get('session.gc_maxlifetime'));
+
+        $fields = array(
+            $this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
+            $this->options['time_field'] => new \MongoDate(),
+            $this->options['expiry_field'] => $expiry,
+        );
+
         $this->getCollection()->update(
             array($this->options['id_field'] => $sessionId),
-            array('$set' => array(
-                $this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
-                $this->options['time_field'] => new \MongoDate(),
-            )),
+            array('$set' => $fields),
             array('upsert' => true, 'multiple' => false)
         );
 
@@ -141,7 +155,8 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
     public function read($sessionId)
     {
         $dbData = $this->getCollection()->findOne(array(
-            $this->options['id_field'] => $sessionId,
+            $this->options['id_field']   => $sessionId,
+            $this->options['expiry_field'] => array('$gte' => new \MongoDate()),
         ));
 
         return null === $dbData ? '' : $dbData[$this->options['data_field']]->bin;

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php

@@ -40,6 +40,7 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
             'id_field' => '_id',
             'data_field' => 'data',
             'time_field' => 'time',
+            'expiry_field' => 'expires_at',
             'database' => 'sf2-test',
             'collection' => 'session-test',
         );
@@ -73,6 +74,42 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($this->storage->close(), 'The "close" method should always return true');
     }
 
+    public function testRead()
+    {
+        $collection = $this->createMongoCollectionMock();
+
+        $this->mongo->expects($this->once())
+            ->method('selectCollection')
+            ->with($this->options['database'], $this->options['collection'])
+            ->will($this->returnValue($collection));
+
+        $that = $this;
+
+        // defining the timeout before the actual method call
+        // allows to test for "greater than" values in the $criteria
+        $testTimeout = time();
+
+        $collection->expects($this->once())
+            ->method('findOne')
+            ->will($this->returnCallback(function ($criteria) use ($that, $testTimeout) {
+                $that->assertArrayHasKey($that->options['id_field'], $criteria);
+                $that->assertEquals($criteria[$that->options['id_field']], 'foo');
+
+                $that->assertArrayHasKey($that->options['expiry_field'], $criteria);
+                $that->assertArrayHasKey('$gte', $criteria[$that->options['expiry_field']]);
+                $that->assertInstanceOf('MongoDate', $criteria[$that->options['expiry_field']]['$gte']);
+                $that->assertGreaterThanOrEqual($criteria[$that->options['expiry_field']]['$gte']->sec, $testTimeout);
+
+                return array(
+                    $that->options['id_field'] => 'foo',
+                    $that->options['data_field'] => new \MongoBinData('bar', \MongoBinData::BYTE_ARRAY),
+                    $that->options['id_field'] => new \MongoDate(),
+                );
+            }));
+
+        $this->assertEquals('bar', $this->storage->read('foo'));
+    }
+
     public function testWrite()
     {
         $collection = $this->createMongoCollectionMock();
@@ -94,10 +131,13 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
                 $data = $updateData['$set'];
             }));
 
+        $expectedExpiry = time() + (int) ini_get('session.gc_maxlifetime');
         $this->assertTrue($this->storage->write('foo', 'bar'));
 
         $this->assertEquals('bar', $data[$this->options['data_field']]->bin);
         $that->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
+        $this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
+        $this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec);
     }
 
     public function testReplaceSessionData()
@@ -153,11 +193,11 @@ class MongoDbSessionHandlerTest extends \PHPUnit_Framework_TestCase
         $collection->expects($this->once())
             ->method('remove')
             ->will($this->returnCallback(function ($criteria) use ($that) {
-                $that->assertInstanceOf('MongoDate', $criteria[$that->options['time_field']]['$lt']);
-                $that->assertGreaterThanOrEqual(time() - -1, $criteria[$that->options['time_field']]['$lt']->sec);
+                $that->assertInstanceOf('MongoDate', $criteria[$that->options['expiry_field']]['$lt']);
+                $that->assertGreaterThanOrEqual(time() - 1, $criteria[$that->options['expiry_field']]['$lt']->sec);
             }));
 
-        $this->assertTrue($this->storage->gc(-1));
+        $this->assertTrue($this->storage->gc(1));
     }
 
     private function createMongoCollectionMock()