From eb335a7db6e002925b6c45f0332b21dabedffe60 Mon Sep 17 00:00:00 2001 From: Benjamin Zikarsky Date: Wed, 19 Nov 2014 13:44:15 +0100 Subject: [PATCH] [HttpFoundation] MongoDbSessionHandler::read() now checks for valid session age --- .../Storage/Handler/MongoDbSessionHandler.php | 47 ++++++++++++------- .../Handler/MongoDbSessionHandlerTest.php | 46 ++++++++++++++++-- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/Session/Storage/Handler/MongoDbSessionHandler.php b/Session/Storage/Handler/MongoDbSessionHandler.php index 04d76b5d9..3876434eb 100644 --- a/Session/Storage/Handler/MongoDbSessionHandler.php +++ b/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,6 +83,7 @@ public function __construct($mongo, array $options) 'id_field' => '_id', 'data_field' => 'data', 'time_field' => 'time', + 'expiry_field' => 'expires_at', ), $options); } @@ -101,18 +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/ - */ - $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 @@ 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, + ); + $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 @@ public function write($sessionId, $data) 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; diff --git a/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php b/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php index d5db694c0..c4031d8c1 100644 --- a/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php +++ b/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', ); @@ -73,6 +74,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(); + + $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 @@ 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']]); + $this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]); + $this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec); } public function testReplaceSessionData() @@ -153,11 +193,11 @@ 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)); + $this->assertTrue($this->storage->gc(1)); } private function createMongoCollectionMock()