merged branch drak/session_on_demand (PR #4264)

Commits ------- 911db69 [FrameworkBundle] Typo fix 19eeac8 [HttpFoundation] Removed erroneous reliance on session.auto_start dcac5d7 [HttpFoundation] Corrected docblocks and properties. 1fd66f3 [FrameworkBundle] Remove 'auto_start' configuration parameter. Discussion ---------- [HttpFoundation] Remove session start on demand Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: - License of the code: MIT This removes false reliance on ini directive `session.auto_start` to allow a session to start when session bags are accessed before the `SessionStorageInterface` is started. Sessions must be explicitly started in all circumstances. --------------------------------------------------------------------------- by stloyd at 2012-06-13T07:22:24Z @drak Shouldn't you add note about this change in upgrade file ? --------------------------------------------------------------------------- by drak at 2012-06-13T15:13:37Z It's a development version change, so not really. But saying that, I have a bunch of documentation to amend when this gets merged and at that time I'll make sure the changelogs and upgrading are up to date as part of that. --------------------------------------------------------------------------- by dlsniper at 2012-06-13T21:57:28Z @drak If this change will kick in what does one user of Symfony 2 Standard must do in order to keep compat with this merge? I see that you said you'll update the docs but until that happens some might upgrade their app directly to master :) Thanks. --------------------------------------------------------------------------- by drak at 2012-06-14T01:36:04Z @dlsniper - nothing. This corrects a bug and inconsistency. --------------------------------------------------------------------------- by travisbot at 2012-06-29T17:48:42Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1739033) (merged 19eeac88 into 62100f1a). --------------------------------------------------------------------------- by drak at 2012-06-29T17:55:13Z @fabpot ping. The failing Travis is nothing to do with this PR (see the travis logs). --------------------------------------------------------------------------- by travisbot at 2012-06-29T20:39:43Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1739805) (merged 911db69b into 62100f1a).
2012-06-30 09:39:50 +02:00 · 2012-06-30 09:39:50 +02:00 · fe527d00c1
commit fe527d00c1
parent 62100f1a18 911db69bfe
16 changed files with 20 additions and 24 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
+++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@ -32,3 +32,5 @@ CHANGELOG
  `gc_probability`/`gc_divisor` chance of being run. The `gc_maxlifetime` defines
   how long a session can idle for. It is different from cookie lifetime which
   declares how long a cookie can be stored on the remote client.
+ * Removed 'auto_start' configuration parameter from session config. The session will
+   start on demand.
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@ -176,7 +176,7 @@ class Configuration implements ConfigurationInterface
                    ->info('session configuration')
                    ->canBeUnset()
                    ->children()
-                        ->booleanNode('auto_start')->defaultFalse()->end()
+                        ->booleanNode('auto_start')->info('DEPRECATED! Session starts on demand')->end()
                        ->scalarNode('storage_id')->defaultValue('session.storage.native')->end()
                        ->scalarNode('handler_id')->defaultValue('session.handler.native_file')->end()
                        ->scalarNode('name')->end()
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@ -291,13 +291,10 @@ class FrameworkExtension extends Extension
    {
        $loader->load('session.xml');

-        // session
-        $container->getDefinition('session_listener')->addArgument($config['auto_start']);
-
        // session storage
        $container->setAlias('session.storage', $config['storage_id']);
        $options = array();
-        foreach (array('name', 'cookie_lifetime', 'cookie_path', 'cookie_domain', 'cookie_secure', 'cookie_httponly', 'auto_start', 'gc_maxlifetime', 'gc_probability', 'gc_divisor') as $key) {
+        foreach (array('name', 'cookie_lifetime', 'cookie_path', 'cookie_domain', 'cookie_secure', 'cookie_httponly', 'gc_maxlifetime', 'gc_probability', 'gc_divisor') as $key) {
            if (isset($config[$key])) {
                $options[$key] = $config[$key];
            }
--- a/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php
+++ b/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php
@ -41,7 +41,6 @@ class SessionListener implements EventSubscriberInterface
    public function __construct(ContainerInterface $container, $autoStart = false)
    {
        $this->container = $container;
-        $this->autoStart = $autoStart;
    }

    public function onKernelRequest(GetResponseEvent $event)
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/deprecated_merge_full.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/deprecated_merge_full.php
@ -3,7 +3,6 @@
 $container->loadFromExtension('framework', array(
    'secret' => 's3cr3t',
    'session' => array(
-        'auto_start'        => true,
        'storage_id'        => 'session.storage.native',
        'handler_id'        => 'session.handler.native_file',
        'name'              => '_SYMFONY',
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/deprecated_merge_partial.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/deprecated_merge_partial.php
@ -3,7 +3,6 @@
 $container->loadFromExtension('framework', array(
    'secret' => 's3cr3t',
    'session' => array(
-        'auto_start'        => true,
        'storage_id'        => 'session.storage.native',
        'handler_id'        => 'session.handler.native_file',
        'name'              => '_SYMFONY',
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php
@ -19,7 +19,6 @@ $container->loadFromExtension('framework', array(
        'type'         => 'xml',
    ),
    'session' => array(
-        'auto_start'     => true,
        'storage_id'     => 'session.storage.native',
        'handler_id'     => 'session.handler.native_file',
        'name'           => '_SYMFONY',
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/deprecated_merge_full.yml
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/deprecated_merge_full.yml
@ -1,7 +1,6 @@
 framework:
    secret: s3cr3t
    session:
-        auto_start:       true
        storage_id:       session.storage.native
        handler_id:       session.handler.native_file
        name:             _SYMFONY
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/deprecated_merge_partial.yml
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/deprecated_merge_partial.yml
@ -1,7 +1,6 @@
 framework:
    secret: s3cr3t
    session:
-        auto_start:       true
        storage_id:       session.storage.native
        handler_id:       session.handler.native_file
        name:             _SYMFONY
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml
@ -13,7 +13,6 @@ framework:
        resource:     %kernel.root_dir%/config/routing.xml
        type:         xml
    session:
-        auto_start:     true
        storage_id:     session.storage.native
        handler_id:     session.handler.native_file
        name:           _SYMFONY
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@ -77,7 +77,6 @@ abstract class FrameworkExtensionTest extends TestCase

        $this->assertTrue($container->hasDefinition('session'), '->registerSessionConfiguration() loads session.xml');
        $this->assertEquals('fr', $container->getParameter('kernel.default_locale'));
-        $this->assertTrue($container->getDefinition('session_listener')->getArgument(1));
        $this->assertEquals('session.storage.native', (string) $container->getAlias('session.storage'));
        $this->assertEquals('session.handler.native_file', (string) $container->getAlias('session.handler'));

--- a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/SessionHelperTest.php
@ -25,6 +25,7 @@ class SessionHelperTest extends \PHPUnit_Framework_TestCase
        $this->request = new Request();

        $session = new Session(new MockArraySessionStorage());
+        $session->start();
        $session->set('foobar', 'bar');
        $session->getFlashBag()->set('notice', 'bar');

--- a/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php
+++ b/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php
@ -58,6 +58,11 @@ class MockArraySessionStorage implements SessionStorageInterface
     */
    protected $metadataBag;

+    /**
+     * @var array
+     */
+    protected $bags;
+
    /**
     * Constructor.
     *
--- a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
+++ b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
@ -61,7 +61,9 @@ class NativeSessionStorage implements SessionStorageInterface
     * @see http://php.net/session.configuration for options
     * but we omit 'session.' from the beginning of the keys for convenience.
     *
-     * auto_start, "0"
+     * ("auto_start", is not supported as it tells PHP to start a session before
+     * PHP starts to execute user-land code. Setting during runtime has no effect).
+     *
     * cache_limiter, "nocache" (use "0" to prevent headers from being sent entirely).
     * cookie_domain, ""
     * cookie_httponly, ""
@ -95,8 +97,6 @@ class NativeSessionStorage implements SessionStorageInterface
     */
    public function __construct(array $options = array(), $handler = null, MetadataBag $metaBag = null)
    {
-        // sensible defaults
-        ini_set('session.auto_start', 0); // by default we prefer to explicitly start the session using the class.
        ini_set('session.cache_limiter', ''); // disable by default because it's managed by HeaderBag (if used)
        ini_set('session.use_cookies', 1);

@ -256,10 +256,10 @@ class NativeSessionStorage implements SessionStorageInterface
            throw new \InvalidArgumentException(sprintf('The SessionBagInterface %s is not registered.', $name));
        }

-        if (ini_get('session.auto_start') && !$this->started) {
-            $this->start();
-        } elseif ($this->saveHandler->isActive() && !$this->started) {
+        if ($this->saveHandler->isActive() && !$this->started) {
            $this->loadSession();
+        } elseif (!$this->started) {
+            $this->start();
        }

        return $this->bags[$name];
@ -302,7 +302,7 @@ class NativeSessionStorage implements SessionStorageInterface
    public function setOptions(array $options)
    {
        $validOptions = array_flip(array(
-            'auto_start', 'cache_limiter', 'cookie_domain', 'cookie_httponly',
+            'cache_limiter', 'cookie_domain', 'cookie_httponly',
            'cookie_lifetime', 'cookie_path', 'cookie_secure',
            'entropy_file', 'entropy_length', 'gc_divisor',
            'gc_maxlifetime', 'gc_probability', 'hash_bits_per_character',
--- a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php
+++ b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php
@ -12,7 +12,6 @@
 namespace Symfony\Component\HttpFoundation\Tests\Session;

 use Symfony\Component\HttpFoundation\Session\Session;
-use Symfony\Component\HttpFoundation\Session\MetadataBag;
 use Symfony\Component\HttpFoundation\Session\Flash\FlashBag;
 use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag;
 use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
@ -260,7 +259,7 @@ class SessionTest extends \PHPUnit_Framework_TestCase
    }

    /**
-     * @covers Symfony\Component\HttpFoundation\Session\Session::count
+     * @covers \Symfony\Component\HttpFoundation\Session\Session::count
     */
    public function testGetCount()
    {
--- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php
+++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php
@ -28,12 +28,12 @@ class MockArraySessionStorageTest extends \PHPUnit_Framework_TestCase
    private $storage;

    /**
-     * @var array
+     * @var AttributeBag
     */
    private $attributes;

    /**
-     * @var array
+     * @var FlashBag
     */
    private $flashes;