Skip to content

Commit a4d6be4

Browse files
fabpotfabpot
committed
bug #13911 [HttpFoundation] MongoDbSessionHandler::read() now checks for valid session age (bzikarsky)
This PR was squashed before being merged into the 2.3 branch (closes #13911). Discussion ---------- [HttpFoundation] MongoDbSessionHandler::read() now checks for valid session age This PR is a follow-up to #12516 and replaces the old one. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | no | Fixed tickets | - | License | MIT | Doc PR | TODO As discussed there: Sessions which are older than GC age should never be read. This PR adds the expiry-datetime on session-write and changes session-read and session-gc accordingly. We still need to update the documentation with some clarifications, as described here: - symfony/symfony#12516 (comment) - symfony/symfony#12516 (comment) My experience with the Symfony Docs from a developer perspective is very limited, so help would be very appreciated. Commits ------- 8289ec3 [HttpFoundation] MongoDbSessionHandler::read() now checks for valid session age
2 parents f68532c + 8289ec3 commit a4d6be4

File tree

2 files changed

+74
-19
lines changed

2 files changed

+74
-19
lines changed

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

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ class MongoDbSessionHandler implements \SessionHandlerInterface
4242
* * id_field: The field name for storing the session id [default: _id]
4343
* * data_field: The field name for storing the session data [default: data]
4444
* * time_field: The field name for storing the timestamp [default: time]
45+
* * expiry_field: The field name for storing the expiry-timestamp [default: expires_at]
46+
*
47+
* It is strongly recommended to put an index on the `expiry_field` for
48+
* garbage-collection. Alternatively it's possible to automatically expire
49+
* the sessions in the database as described below:
50+
*
51+
* A TTL collections can be used on MongoDB 2.2+ to cleanup expired sessions
52+
* automatically. Such an index can for example look like this:
53+
*
54+
* db.<session-collection>.ensureIndex(
55+
* { "<expiry-field>": 1 },
56+
* { "expireAfterSeconds": 0 }
57+
* )
58+
*
59+
* More details on: http://docs.mongodb.org/manual/tutorial/expire-data/
60+
*
61+
* If you use such an index, you can drop `gc_probability` to 0 since
62+
* no garbage-collection is required.
4563
*
4664
* @param \Mongo|\MongoClient $mongo A MongoClient or Mongo instance
4765
* @param array $options An associative array of field options
@@ -65,6 +83,7 @@ public function __construct($mongo, array $options)
6583
'id_field' => '_id',
6684
'data_field' => 'data',
6785
'time_field' => 'time',
86+
'expiry_field' => 'expires_at',
6887
), $options);
6988
}
7089

@@ -101,18 +120,8 @@ public function destroy($sessionId)
101120
*/
102121
public function gc($maxlifetime)
103122
{
104-
/* Note: MongoDB 2.2+ supports TTL collections, which may be used in
105-
* place of this method by indexing the "time_field" field with an
106-
* "expireAfterSeconds" option. Regardless of whether TTL collections
107-
* are used, consider indexing this field to make the remove query more
108-
* efficient.
109-
*
110-
* See: http://docs.mongodb.org/manual/tutorial/expire-data/
111-
*/
112-
$time = new \MongoDate(time() - $maxlifetime);
113-
114123
$this->getCollection()->remove(array(
115-
$this->options['time_field'] => array('$lt' => $time),
124+
$this->options['expiry_field'] => array('$lt' => new \MongoDate()),
116125
));
117126

118127
return true;
@@ -123,12 +132,17 @@ public function gc($maxlifetime)
123132
*/
124133
public function write($sessionId, $data)
125134
{
135+
$expiry = new \MongoDate(time() + (int) ini_get('session.gc_maxlifetime'));
136+
137+
$fields = array(
138+
$this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
139+
$this->options['time_field'] => new \MongoDate(),
140+
$this->options['expiry_field'] => $expiry,
141+
);
142+
126143
$this->getCollection()->update(
127144
array($this->options['id_field'] => $sessionId),
128-
array('$set' => array(
129-
$this->options['data_field'] => new \MongoBinData($data, \MongoBinData::BYTE_ARRAY),
130-
$this->options['time_field'] => new \MongoDate(),
131-
)),
145+
array('$set' => $fields),
132146
array('upsert' => true, 'multiple' => false)
133147
);
134148

@@ -141,7 +155,8 @@ public function write($sessionId, $data)
141155
public function read($sessionId)
142156
{
143157
$dbData = $this->getCollection()->findOne(array(
144-
$this->options['id_field'] => $sessionId,
158+
$this->options['id_field'] => $sessionId,
159+
$this->options['expiry_field'] => array('$gte' => new \MongoDate()),
145160
));
146161

147162
return null === $dbData ? '' : $dbData[$this->options['data_field']]->bin;

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

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ protected function setUp()
4040
'id_field' => '_id',
4141
'data_field' => 'data',
4242
'time_field' => 'time',
43+
'expiry_field' => 'expires_at',
4344
'database' => 'sf2-test',
4445
'collection' => 'session-test',
4546
);
@@ -73,6 +74,42 @@ public function testCloseMethodAlwaysReturnTrue()
7374
$this->assertTrue($this->storage->close(), 'The "close" method should always return true');
7475
}
7576

77+
public function testRead()
78+
{
79+
$collection = $this->createMongoCollectionMock();
80+
81+
$this->mongo->expects($this->once())
82+
->method('selectCollection')
83+
->with($this->options['database'], $this->options['collection'])
84+
->will($this->returnValue($collection));
85+
86+
$that = $this;
87+
88+
// defining the timeout before the actual method call
89+
// allows to test for "greater than" values in the $criteria
90+
$testTimeout = time();
91+
92+
$collection->expects($this->once())
93+
->method('findOne')
94+
->will($this->returnCallback(function ($criteria) use ($that, $testTimeout) {
95+
$that->assertArrayHasKey($that->options['id_field'], $criteria);
96+
$that->assertEquals($criteria[$that->options['id_field']], 'foo');
97+
98+
$that->assertArrayHasKey($that->options['expiry_field'], $criteria);
99+
$that->assertArrayHasKey('$gte', $criteria[$that->options['expiry_field']]);
100+
$that->assertInstanceOf('MongoDate', $criteria[$that->options['expiry_field']]['$gte']);
101+
$that->assertGreaterThanOrEqual($criteria[$that->options['expiry_field']]['$gte']->sec, $testTimeout);
102+
103+
return array(
104+
$that->options['id_field'] => 'foo',
105+
$that->options['data_field'] => new \MongoBinData('bar', \MongoBinData::BYTE_ARRAY),
106+
$that->options['id_field'] => new \MongoDate(),
107+
);
108+
}));
109+
110+
$this->assertEquals('bar', $this->storage->read('foo'));
111+
}
112+
76113
public function testWrite()
77114
{
78115
$collection = $this->createMongoCollectionMock();
@@ -94,10 +131,13 @@ public function testWrite()
94131
$data = $updateData['$set'];
95132
}));
96133

134+
$expectedExpiry = time() + (int) ini_get('session.gc_maxlifetime');
97135
$this->assertTrue($this->storage->write('foo', 'bar'));
98136

99137
$this->assertEquals('bar', $data[$this->options['data_field']]->bin);
100138
$that->assertInstanceOf('MongoDate', $data[$this->options['time_field']]);
139+
$this->assertInstanceOf('MongoDate', $data[$this->options['expiry_field']]);
140+
$this->assertGreaterThanOrEqual($expectedExpiry, $data[$this->options['expiry_field']]->sec);
101141
}
102142

103143
public function testReplaceSessionData()
@@ -153,11 +193,11 @@ public function testGc()
153193
$collection->expects($this->once())
154194
->method('remove')
155195
->will($this->returnCallback(function ($criteria) use ($that) {
156-
$that->assertInstanceOf('MongoDate', $criteria[$that->options['time_field']]['$lt']);
157-
$that->assertGreaterThanOrEqual(time() - -1, $criteria[$that->options['time_field']]['$lt']->sec);
196+
$that->assertInstanceOf('MongoDate', $criteria[$that->options['expiry_field']]['$lt']);
197+
$that->assertGreaterThanOrEqual(time() - 1, $criteria[$that->options['expiry_field']]['$lt']->sec);
158198
}));
159199

160-
$this->assertTrue($this->storage->gc(-1));
200+
$this->assertTrue($this->storage->gc(1));
161201
}
162202

163203
private function createMongoCollectionMock()

0 commit comments

Comments
 (0)