Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Hotfix/session mutability #3452

Merged
merged 4 commits into from

3 participants

@weierophinney

Fixes issues introduced earlier this week when we added the ability to call the session's writeClose() method on shutdown. Doing it in __destruct caused issues, so @mwillbanks has moved it to a register_shutdown_function, which ensures the original issue is fixed, while simultaneously fixing the regression.

In this PR, I also made it possible to inject the PRG plugin with a session manager, to make mocking and test isolation easier.

mwillbanks and others added some commits
@mwillbanks mwillbanks Updated session manager to register_shutdown_function rather than a d…
…estructor

- Fixed an issue where an exception was not properly defined in use
- Fixed a flash messenger test by clearing session storage on teardown
- Removed a no longer valid test on sessionmanagertest

Conflicts:
	library/Zend/Session/Storage/SessionArrayStorage.php
d7e8a58
@weierophinney weierophinney Allow injecting session manager into PRG plugin
- which allows creating the internally used Container with the specified
  session manager
- neither FM and PRG should instantiate a SessionManager directly; they
  should use the default manager from the Container class
5153e55
@weierophinney weierophinney CS cleanup of PRG plugin
- Created docblock for __invoke() method
- Removed an else condition that would not be hit if the if condition is
  evaluated.
5ad3c29
@weierophinney weierophinney Fix issue with imports
- An import of "use Zend\Session\Storage\StorageInterface as Storage" caused a
  subsequent, existing check of "instanceof Storage\StorageInterface" to thus
  fail. Removed all imports for classes in the same namespace to fix the issue.
46100cd
@weierophinney

To whomever reviews this: please merge against both master and develop, as the issue presents in both branches.

@Maks3w Maks3w merged commit 46100cd into from
@Maks3w Maks3w referenced this pull request from a commit in Maks3w/zf2
@Maks3w Maks3w Forward #3452
Conflicts:
	library/Zend/Mvc/Controller/Plugin/PostRedirectGet.php
d80d076
@weierophinney weierophinney deleted the branch
@ghost Unknown referenced this pull request from a commit
@Maks3w Maks3w Forward #3452
Conflicts:
	library/Zend/Mvc/Controller/Plugin/PostRedirectGet.php
ebfb1d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 16, 2013
  1. @mwillbanks @weierophinney

    Updated session manager to register_shutdown_function rather than a d…

    mwillbanks authored weierophinney committed
    …estructor
    
    - Fixed an issue where an exception was not properly defined in use
    - Fixed a flash messenger test by clearing session storage on teardown
    - Removed a no longer valid test on sessionmanagertest
    
    Conflicts:
    	library/Zend/Session/Storage/SessionArrayStorage.php
  2. @weierophinney

    Allow injecting session manager into PRG plugin

    weierophinney authored
    - which allows creating the internally used Container with the specified
      session manager
    - neither FM and PRG should instantiate a SessionManager directly; they
      should use the default manager from the Container class
  3. @weierophinney

    CS cleanup of PRG plugin

    weierophinney authored
    - Created docblock for __invoke() method
    - Removed an else condition that would not be hit if the if condition is
      evaluated.
  4. @weierophinney

    Fix issue with imports

    weierophinney authored
    - An import of "use Zend\Session\Storage\StorageInterface as Storage" caused a
      subsequent, existing check of "instanceof Storage\StorageInterface" to thus
      fail. Removed all imports for classes in the same namespace to fix the issue.
This page is out of date. Refresh to see the latest.
View
3  library/Zend/Mvc/Controller/Plugin/FlashMessenger.php
@@ -15,7 +15,6 @@
use IteratorAggregate;
use Zend\Session\Container;
use Zend\Session\ManagerInterface as Manager;
-use Zend\Session\SessionManager;
use Zend\Stdlib\SplQueue;
/**
@@ -79,7 +78,7 @@ public function setSessionManager(Manager $manager)
public function getSessionManager()
{
if (!$this->session instanceof Manager) {
- $this->setSessionManager(new SessionManager());
+ $this->setSessionManager(Container::getDefaultManager());
}
return $this->session;
}
View
67 library/Zend/Mvc/Controller/Plugin/PostRedirectGet.php
@@ -13,6 +13,7 @@
use Zend\Mvc\Exception\RuntimeException;
use Zend\Session\Container;
+use Zend\Session\ManagerInterface as Manager;
/**
* Plugin to help facilitate Post/Redirect/Get (http://en.wikipedia.org/wiki/Post/Redirect/Get)
@@ -23,6 +24,56 @@
*/
class PostRedirectGet extends AbstractPlugin
{
+ /**
+ * @var Manager
+ */
+ protected $session;
+
+ /**
+ * Set the session manager
+ *
+ * @param Manager $manager
+ * @return FlashMessenger
+ */
+ public function setSessionManager(Manager $manager)
+ {
+ $this->session = $manager;
+ return $this;
+ }
+
+ /**
+ * Retrieve the session manager
+ *
+ * If none composed, lazy-loads a SessionManager instance
+ *
+ * @return Manager
+ */
+ public function getSessionManager()
+ {
+ if (!$this->session instanceof Manager) {
+ $this->setSessionManager(Container::getDefaultManager());
+ }
+ return $this->session;
+ }
+
+ /**
+ * Perform PRG logic
+ *
+ * If a null value is present for the $redirect, the current route is
+ * retrieved and use to generate the URL for redirect.
+ *
+ * If the request method is POST, creates a session container set to expire
+ * after 1 hop containing the values of the POST. It then redirects to the
+ * specified URL using a status 303.
+ *
+ * If the request method is GET, checks to see if we have values in the
+ * session container, and, if so, returns them; otherwise, it returns a
+ * boolean false.
+ *
+ * @param null|string $redirect
+ * @param bool $redirectToUrl
+ * @return \Zend\Http\Response|array|Traversable|false
+ */
public function __invoke($redirect = null, $redirectToUrl = false)
{
$controller = $this->getController();
@@ -36,7 +87,7 @@ public function __invoke($redirect = null, $redirectToUrl = false)
$params = $routeMatch->getParams();
}
- $container = new Container('prg_post1');
+ $container = new Container('prg_post1', $this->getSessionManager());
if ($request->isPost()) {
$container->setExpirationHops(1, 'post');
@@ -67,14 +118,14 @@ public function __invoke($redirect = null, $redirectToUrl = false)
$response->setStatusCode(303);
return $response;
- } else {
- if ($container->post !== null) {
- $post = $container->post;
- unset($container->post);
- return $post;
- }
+ }
- return false;
+ if ($container->post !== null) {
+ $post = $container->post;
+ unset($container->post);
+ return $post;
}
+
+ return false;
}
}
View
16 library/Zend/Session/SessionManager.php
@@ -11,7 +11,6 @@
namespace Zend\Session;
use Zend\EventManager\EventManagerInterface;
-use Zend\Session\SaveHandler\SaveHandlerInterface;
/**
* Session ManagerInterface implementation utilizing ext/session
@@ -43,14 +42,17 @@ class SessionManager extends AbstractManager
protected $validatorChain;
/**
- * Destructor
- * Ensures that writeClose is called.
+ * Constructor
*
- * @return void
+ * @param Config\ConfigInterface|null $config
+ * @param Storage\StorageInterface|null $storage
+ * @param SaveHandler\SaveHandlerInterface|null $saveHandler
+ * @throws Exception\RuntimeException
*/
- public function __destruct()
+ public function __construct(Config\ConfigInterface $config = null, Storage\StorageInterface $storage = null, SaveHandler\SaveHandlerInterface $saveHandler = null)
{
- $this->writeClose();
+ parent::__construct($config, $storage, $saveHandler);
+ register_shutdown_function(array($this, 'writeClose'));
}
/**
@@ -89,7 +91,7 @@ public function start($preserveStorage = false)
}
$saveHandler = $this->getSaveHandler();
- if ($saveHandler instanceof SaveHandlerInterface) {
+ if ($saveHandler instanceof SaveHandler\SaveHandlerInterface) {
// register the session handler with ext/session
$this->registerSaveHandler($saveHandler);
}
View
5 tests/ZendTest/Mvc/Controller/Plugin/FlashMessengerTest.php
@@ -27,6 +27,11 @@ public function setUp()
$this->helper->setSessionManager($this->session);
}
+ public function tearDown()
+ {
+ $this->session->getStorage()->clear();
+ }
+
public function seedMessages()
{
$helper = new FlashMessenger();
View
4 tests/ZendTest/Mvc/Controller/Plugin/PostRedirectGetTest.php
@@ -65,7 +65,9 @@ public function setUp()
$this->sessionManager->destroy();
$this->controller->setEvent($this->event);
- $this->controller->flashMessenger()->setSessionManager($this->sessionManager);
+ $plugins = $this->controller->getPluginManager();
+ $plugins->get('flashmessenger')->setSessionManager($this->sessionManager);
+ $plugins->get('prg')->setSessionManager($this->sessionManager);
}
public function testReturnsFalseOnIntialGet()
View
12 tests/ZendTest/Session/SessionManagerTest.php
@@ -329,18 +329,6 @@ public function testCallingWriteCloseMarksStorageAsImmutable()
/**
* @runInSeparateProcess
*/
- public function testCallingDestructorMarksStorageAsImmutable()
- {
- $this->manager->start();
- $storage = $this->manager->getStorage();
- $storage['foo'] = 'bar';
- $this->manager = null;
- $this->assertTrue($storage->isImmutable());
- }
-
- /**
- * @runInSeparateProcess
- */
public function testCallingWriteCloseShouldNotAlterSessionExistsStatus()
{
$this->manager->start();
View
4 tests/ZendTest/Session/TestAsset/TestSaveHandlerWithValidator.php
@@ -15,7 +15,9 @@
class TestSaveHandlerWithValidator implements SaveHandler
{
public function open($save_path, $name)
- {return true;}
+ {
+ return true;
+ }
public function close()
{}
Something went wrong with that request. Please try again.