From 3f6372dd1b8ec79e610ffa9d03b82dc4f95b5393 Mon Sep 17 00:00:00 2001 From: Benjamin Zikarsky Date: Wed, 19 Nov 2014 13:44:15 +0100 Subject: [PATCH 1/2] MongoDbSessionHandler::read() now checks for valid session age --- .../Storage/Handler/MongoDbSessionHandler.php | 5 ++- .../Handler/MongoDbSessionHandlerTest.php | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php index eb35e5aa3121..e1b9cc3a269b 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php @@ -159,8 +159,11 @@ public function write($sessionId, $data) */ public function read($sessionId) { + $timeoutThreshold = new \MongoDate(time() - (int) ini_get('session.gc_maxlifetime')); + $dbData = $this->getCollection()->findOne(array( - $this->options['id_field'] => $sessionId, + $this->options['id_field'] => $sessionId, + $this->options['time_field'] => array('$gt' => $timeoutThreshold), )); return null === $dbData ? '' : $dbData[$this->options['data_field']]->bin; diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php index 63d6d1e92383..573706ac37fa 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php @@ -73,6 +73,42 @@ public function testCloseMethodAlwaysReturnTrue() $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() - (int) ini_get('session.gc_maxlifetime'); + + $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['time_field'], $criteria); + $that->assertArrayHasKey('$gt', $criteria[$that->options['time_field']]); + $that->assertInstanceOf('MongoDate', $criteria[$that->options['time_field']]['$gt']); + $that->assertGreaterThanOrEqual($criteria[$that->options['time_field']]['$gt']->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(); From a3fbf345172d6f401b58c9b5096cdbf895c53f6d Mon Sep 17 00:00:00 2001 From: Benjamin Zikarsky Date: Sat, 29 Nov 2014 16:35:46 +0100 Subject: [PATCH 2/2] Session expiry date is now defined on session-write --- .../Storage/Handler/MongoDbSessionHandler.php | 55 ++++++------ .../Handler/MongoDbSessionHandlerTest.php | 83 +++---------------- 2 files changed, 35 insertions(+), 103 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php index e1b9cc3a269b..f2f664a5a415 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php +++ b/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..ensureIndex( + * { "": 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,7 +83,7 @@ public function __construct($mongo, array $options) 'id_field' => '_id', 'data_field' => 'data', 'time_field' => 'time', - 'expiry_field' => false, + 'expiry_field' => 'expires_at', ), $options); } @@ -102,21 +120,8 @@ public function destroy($sessionId) */ 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/ - */ - if (false !== $this->options['expiry_field']) { - return true; - } - $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; @@ -127,24 +132,14 @@ public function gc($maxlifetime) */ 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, ); - /* Note: As discussed in the gc method of this class. You can utilise - * TTL collections in MongoDB 2.2+ - * We are setting the "expiry_field" as part of the write operation here - * You will need to create the index on your collection that expires documents - * at that time - * e.g. - * db.MySessionCollection.ensureIndex( { "expireAt": 1 }, { expireAfterSeconds: 0 } ) - */ - if (false !== $this->options['expiry_field']) { - $expiry = new \MongoDate(time() + (int) ini_get('session.gc_maxlifetime')); - $fields[$this->options['expiry_field']] = $expiry; - } - $this->getCollection()->update( array($this->options['id_field'] => $sessionId), array('$set' => $fields), @@ -159,11 +154,9 @@ public function write($sessionId, $data) */ public function read($sessionId) { - $timeoutThreshold = new \MongoDate(time() - (int) ini_get('session.gc_maxlifetime')); - $dbData = $this->getCollection()->findOne(array( $this->options['id_field'] => $sessionId, - $this->options['time_field'] => array('$gt' => $timeoutThreshold), + $this->options['expiry_field'] => array('$gte' => new \MongoDate()), )); return null === $dbData ? '' : $dbData[$this->options['data_field']]->bin; diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php index 573706ac37fa..f22b9e33d8d8 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php @@ -40,6 +40,7 @@ protected function setUp() 'id_field' => '_id', 'data_field' => 'data', 'time_field' => 'time', + 'expiry_field' => 'expires_at', 'database' => 'sf2-test', 'collection' => 'session-test', ); @@ -86,7 +87,7 @@ public function testRead() // defining the timeout before the actual method call // allows to test for "greater than" values in the $criteria - $testTimeout = time() - (int) ini_get('session.gc_maxlifetime'); + $testTimeout = time(); $collection->expects($this->once()) ->method('findOne') @@ -94,10 +95,10 @@ public function testRead() $that->assertArrayHasKey($that->options['id_field'], $criteria); $that->assertEquals($criteria[$that->options['id_field']], 'foo'); - $that->assertArrayHasKey($that->options['time_field'], $criteria); - $that->assertArrayHasKey('$gt', $criteria[$that->options['time_field']]); - $that->assertInstanceOf('MongoDate', $criteria[$that->options['time_field']]['$gt']); - $that->assertGreaterThanOrEqual($criteria[$that->options['time_field']]['$gt']->sec, $testTimeout); + $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', @@ -130,49 +131,13 @@ public function testWrite() $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']]); - } - - public function testWriteWhenUsingExpiresField() - { - $this->options = array( - 'id_field' => '_id', - 'data_field' => 'data', - 'time_field' => 'time', - 'database' => 'sf2-test', - 'collection' => 'session-test', - 'expiry_field' => 'expiresAt', - ); - - $this->storage = new MongoDbSessionHandler($this->mongo, $this->options); - - $collection = $this->createMongoCollectionMock(); - - $this->mongo->expects($this->once()) - ->method('selectCollection') - ->with($this->options['database'], $this->options['collection']) - ->will($this->returnValue($collection)); - - $that = $this; - $data = array(); - - $collection->expects($this->once()) - ->method('update') - ->will($this->returnCallback(function ($criteria, $updateData, $options) use ($that, &$data) { - $that->assertEquals(array($that->options['id_field'] => 'foo'), $criteria); - $that->assertEquals(array('upsert' => true, 'multiple' => false), $options); - - $data = $updateData['$set']; - })); - - $this->assertTrue($this->storage->write('foo', 'bar')); - - $this->assertEquals('bar', $data[$this->options['data_field']]->bin); - $that->assertInstanceOf('MongoDate', $data[$this->options['time_field']]); - $that->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]); + $this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]); + $this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec); } public function testReplaceSessionData() @@ -228,39 +193,13 @@ public function testGc() $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)); } - public function testGcWhenUsingExpiresField() - { - $this->options = array( - 'id_field' => '_id', - 'data_field' => 'data', - 'time_field' => 'time', - 'database' => 'sf2-test', - 'collection' => 'session-test', - 'expiry_field' => 'expiresAt', - ); - - $this->storage = new MongoDbSessionHandler($this->mongo, $this->options); - - $collection = $this->createMongoCollectionMock(); - - $this->mongo->expects($this->never()) - ->method('selectCollection'); - - $that = $this; - - $collection->expects($this->never()) - ->method('remove'); - - $this->assertTrue($this->storage->gc(1)); - } - public function testGetConnection() { $method = new \ReflectionMethod($this->storage, 'getMongo');