[FrameworkBundle] Inform the user when save_path will be ignored

gnat42 · fabpot · commit a0901294d437 · 2019-07-08T14:54:05.000+02:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -470,6 +470,12 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
         $rootNode
             ->children()
                 ->arrayNode('session')
+                    ->validate()
+                        ->ifTrue(function ($v) {
+                            return empty($v['handler_id']) && !empty($v['save_path']);
+                        })
+                        ->thenInvalid('Session save path is ignored without a handler service')
+                    ->end()
                     ->info('session configuration')
                     ->canBeEnabled()
                     ->children()
@@ -498,7 +504,7 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
                             ->defaultTrue()
                             ->setDeprecated('The "%path%.%node%" option is enabled by default and deprecated since Symfony 3.4. It will be always enabled in 4.0.')
                         ->end()
-                        ->scalarNode('save_path')->defaultValue('%kernel.cache_dir%/sessions')->end()
+                        ->scalarNode('save_path')->end()
                         ->integerNode('metadata_update_threshold')
                             ->defaultValue('0')
                             ->info('seconds to wait between 2 session metadata updates')
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -873,13 +873,22 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
 
         // session handler (the internal callback registered with PHP session management)
         if (null === $config['handler_id']) {
+            // If the user set a save_path without using a non-default \SessionHandler, it will silently be ignored
+            if (isset($config['save_path'])) {
+                throw new LogicException('Session save path is ignored without a handler service');
+            }
+
             // Set the handler class to be null
             $container->getDefinition('session.storage.native')->replaceArgument(1, null);
             $container->getDefinition('session.storage.php_bridge')->replaceArgument(0, null);
         } else {
             $container->setAlias('session.handler', $config['handler_id'])->setPrivate(true);
         }
 
+        if (!isset($config['save_path'])) {
+            $config['save_path'] = ini_get('session.save_path');
+        }
+
         $container->setParameter('session.save_path', $config['save_path']);
 
         if (\PHP_VERSION_ID < 70000) {
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
@@ -378,7 +378,6 @@ protected static function getBundleDefaultConfig()
                 'handler_id' => 'session.handler.native_file',
                 'cookie_httponly' => true,
                 'gc_probability' => 1,
-                'save_path' => '%kernel.cache_dir%/sessions',
                 'metadata_update_threshold' => '0',
                 'use_strict_mode' => true,
             ],
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/session_savepath.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/session_savepath.php
@@ -0,0 +1,8 @@
+<?php
+
+$container->loadFromExtension('framework', [
+    'session' => [
+        'handler_id' => null,
+        'save_path' => '/some/path',
+    ],
+]);
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/session_savepath.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/session_savepath.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:framework="http://symfony.com/schema/dic/symfony"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+                        http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
+
+    <framework:config>
+        <framework:session handler-id="null" save-path="/some/path"/>
+    </framework:config>
+</container>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/session_savepath.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/session_savepath.yml
@@ -0,0 +1,4 @@
+framework:
+    session:
+        handler_id: null
+        save_path: /some/path
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -24,6 +24,7 @@
 use Symfony\Component\Cache\Adapter\FilesystemAdapter;
 use Symfony\Component\Cache\Adapter\ProxyAdapter;
 use Symfony\Component\Cache\Adapter\RedisAdapter;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -474,6 +475,12 @@ public function testNullSessionHandler()
         $this->assertNull($container->getDefinition('session.storage.php_bridge')->getArgument(0));
     }
 
+    public function testNullSessionHandlerWithSavePath()
+    {
+        $this->expectException(InvalidConfigurationException::class);
+        $this->createContainerFromFile('session_savepath');
+    }
+
     public function testRequest()
     {
         $container = $this->createContainerFromFile('full');
