8000 merged branch baldurrensch/testlistener_fix (PR #6362) · symfony/symfony@c67ddb2 · GitHub
[go: up one dir, main page]
Skip to content

Commit c67ddb2

Browse files
fabpotfabpot
committed
merged branch baldurrensch/testlistener_fix (PR #6362)
This PR was merged into the 2.1 branch. Commits ------- 098b593 [Session] Added exception to save method 6b9ee87 [Session] Fixed a bug with the TestListener Discussion ---------- [Session] Fixed bug with TestListener Fixed a bug where an unstarted mock session would be emptied with a save. Here are the steps to reproduce: Use the test client from Symfony\Bundle\FrameworkBundle\Test\WebTestCase::createClient(), and add something to its session. (I actually had it authenticate against a firewall). Take the cookies of this first test client and add them to a second test client Have the second test client request a URL that results in a 404 Since the 404 does not need to start the session, hence when save is called (automatically), the mock session is overwritten with an empty array. This does not happen with the other session handlers. The added unit test in this PR shows this problem. If this PR gets accepted, will it also get merged into the 2.1.x-dev branch? Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes (The broken test seems to be unrelated to this change) Fixes the following tickets: - Todo: - License of the code: MIT Documentation PR: - This is a follow up PR since my original one (#6342) was against the wrong upstream branch.
2 parents 563b208 + 098b593 commit c67ddb2

File tree

8 files changed

+54
-1
lines changed

8 files changed

+54
-1
lines changed

‎src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ public function onKernelResponse(FilterResponseEvent $event)
6868
}
6969

7070
if ($session = $event->getRequest()->getSession()) {
71-
$session->save();
71+
if ($session->isStarted()) {
72+
$session->save();
73+
}
7274

7375
$params = session_get_cookie_params();
7476

‎src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ protected function tearDown()
4343

4444
public function testShouldSaveMasterRequestSession()
4545
{
46+
$this->sessionHasBeenStarted();
4647
$this->sessionMustBeSaved();
4748

4849
$this->filterResponse(new Request());
@@ -66,6 +67,14 @@ public function testDoesNotDeleteCookieIfUsingSessionLifetime()
6667
$this->assertEquals(0, reset($cookies)->getExpiresTime());
6768
}
6869

70+
public function testUnstartedSessionIsNotSave()
71+
{
72+
$this->sessionHasNotBeenStarted();
73+
$this->sessionMustNotBeSaved();
74+
75+
$this->filterResponse(new Request());
76+
}
77+
6978
private function filterResponse(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
7079
{
7180
$request->setSession($this->session);
@@ -92,6 +101,20 @@ private function sessionMustBeSaved()
92101
->method('save');
93102
}
94103

104+
private function sessionHasBeenStarted()
105+
{
106+
$this->session->expects($this->once())
107+
->method('isStarted')
108+
->will($this->returnValue(true));
109+
}
110+
111+
private function sessionHasNotBeenStarted()
112+
{
113+
$this->session->expects($this->once())
114+
->method('isStarted')
115+
->will($this->returnValue(false));
116+
}
117+
95118
private function getSession()
96119
{
97120
$mock = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session')

‎src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ public function setName($name)
159159
*/
160160
public function save()
161161
{
162+
if (!$this->started || $this->closed) {
163+
throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
164+
}
162165
// nothing to do since we don't persist the session data
163166
$this->closed = false;
164167
}

‎src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ public function regenerate($destroy = false, $lifetime = null)
9797
*/
9898
public function save()
9999
{
100+
if (!$this->started) {
101+
throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
102+
}
103+
100104
file_put_contents($this->getFilePath(), serialize($this->data));
101105

102106
// this is needed for Silex, where the session object is re-used across requests

‎src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ public function regenerate($destroy = false, $lifetime = null);
110110
* used for a storage object design for unit or functional testing where
111111
* a real PHP session would interfere with testing, in whic A935 h case it
112112
* it should actually persist the session data if required.
113+
*
114+
* @throws \RuntimeException If the session is saved without being started, or if the session
115+
* is already closed.
113116
*/
114117
public function save();
115118

‎src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ public function testMigrateDestroy()
151151

152152
public function testSave()
153153
{
154+
$this->session->start();
154155
$this->session->save();
155156
}
156157

‎src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,12 @@ public function testGetId()
9595
$this->storage->start();
9696
$this->assertNotEquals('', $this->storage->getId());
9797
}
98+
99+
/**
100+
* @expectedException RuntimeException
101+
*/
102+
public function testUnstartedSave()
103+
{
104+
$this->storage->save();
105+
}
98106
}

‎src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,15 @@ public function testMultipleInstances()
106106
$this->assertEquals('bar', $storage2->getBag('attributes')->get('foo'), 'values persist between instances');
107107
}
108108

109+
/**
110+
* @expectedException RuntimeException
111+
*/
112+
public function testSaveWithoutStart()
113+
{
114+
$storage1 = $this->getStorage();
115+
$storage1->save();
116+
}
117+
109118
private function getStorage()
110119
{
111120
$storage = new MockFileSessionStorage($this->sessionDir);

0 commit comments

Comments
 (0)
0