Issue #2347877 by znerol, Berdir: Move DrupalKernel::initializeCookieGlobals()...

Issue #2347877 by znerol, Berdir: Move DrupalKernel::initializeCookieGlobals() into a SessionConfiguration service

core/core.services.yml

 parameters:
+  session.storage.options: {}
   twig.config: {}
   factory.keyvalue:
     default: keyvalue.database
@@ -116,6 +117,7 @@ services:
     arguments: [discovery]
   page_cache_request_policy:
     class: Drupal\Core\PageCache\DefaultRequestPolicy
+    arguments: ['@session_configuration']
     tags:
       - { name: service_collector, tag: page_cache_request_policy, call: addPolicy}
   page_cache_response_policy:
@@ -1044,9 +1046,12 @@ services:
   current_user:
     class: Drupal\Core\Session\AccountProxy
     arguments: ['@authentication', '@request_stack']
+  session_configuration:
+    class: Drupal\Core\Session\SessionConfiguration
+    arguments: ['%session.storage.options%']
   session_manager:
     class: Drupal\Core\Session\SessionManager
-    arguments: ['@request_stack', '@database', '@session_manager.metadata_bag', '@settings']
+    arguments: ['@request_stack', '@database', '@session_manager.metadata_bag', '@session_configuration']
     tags:
       - { name: backend_overridable }
   session_manager.metadata_bag:

core/lib/Drupal/Core/DrupalKernel.php

@@ -158,13 +158,6 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
    */
   protected $serviceProviders;
 
-  /**
-   * Whether the request globals have been initialized.
-   *
-   * @var bool
-   */
-  protected static $isRequestInitialized = FALSE;
-
   /**
    * Whether the PHP environment has been initialized.
    *
@@ -451,9 +444,6 @@ public function preHandle(Request $request) {
     // Initialize legacy request globals.
     $this->initializeRequestGlobals($request);
 
-    // Initialize cookie globals.
-    $this->initializeCookieGlobals($request);
-
     // Put the request on the stack.
     $this->container->get('request_stack')->push($request);
 
@@ -479,7 +469,6 @@ public function preHandle(Request $request) {
    */
   public function handlePageCache(Request $request) {
     $this->boot();
-    $this->initializeCookieGlobals($request);
 
     // Check for a cache mode force from settings.php.
     if (Settings::get('page_cache_without_database')) {
@@ -892,64 +881,6 @@ protected function initializeRequestGlobals(Request $request) {
 
   }
 
-  /**
-   * Initialize cookie settings.
-   *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The current request.
-   *
-   * @todo D8: Eliminate this entirely in favor of a session object.
-   */
-  protected function initializeCookieGlobals(Request $request) {
-    // If we do this more then once per page request we are likely to cause
-    // errors.
-    if (static::$isRequestInitialized) {
-      return;
-    }
-    global $cookie_domain;
-
-    if ($cookie_domain) {
-      // If the user specifies the cookie domain, also use it for session name.
-      $session_name = $cookie_domain;
-    }
-    else {
-      // Otherwise use $base_url as session name, without the protocol
-      // to use the same session identifiers across HTTP and HTTPS.
-      $session_name = $request->getHost() . $request->getBasePath();
-      // Replace "core" out of session_name so core scripts redirect properly,
-      // specifically install.php.
-      $session_name = preg_replace('/\/core$/', '', $session_name);
-      if ($cookie_domain = $request->getHost()) {
-        // Strip leading periods and www. from cookie domain.
-        $cookie_domain = ltrim($cookie_domain, '.');
-        if (strpos($cookie_domain, 'www.') === 0) {
-          $cookie_domain = substr($cookie_domain, 4);
-        }
-        $cookie_domain = '.' . $cookie_domain;
-      }
-    }
-    // Per RFC 2109, cookie domains must contain at least one dot other than the
-    // first. For hosts such as 'localhost' or IP Addresses we don't set a
-    // cookie domain.
-    if (count(explode('.', $cookie_domain)) > 2 && !is_numeric(str_replace('.', '', $cookie_domain))) {
-      ini_set('session.cookie_domain', $cookie_domain);
-    }
-    // To prevent session cookies from being hijacked, a user can configure the
-    // SSL version of their website to only transfer session cookies via SSL by
-    // using PHP's session.cookie_secure setting. The browser will then use two
-    // separate session cookies for the HTTPS and HTTP versions of the site. So
-    // we must use different session identifiers for HTTPS and HTTP to prevent a
-    // cookie collision.
-    if ($request->isSecure()) {
-      ini_set('session.cookie_secure', TRUE);
-    }
-    $prefix = ini_get('session.cookie_secure') ? 'SSESS' : 'SESS';
-
-    session_name($prefix . substr(hash('sha256', $session_name), 0, 32));
-
-    static::$isRequestInitialized = TRUE;
-  }
-
   /**
    * Returns service instances to persist from an old container to a new one.
    */

core/lib/Drupal/Core/PageCache/DefaultRequestPolicy.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\Core\PageCache;
 
+use Drupal\Core\Session\SessionConfigurationInterface;
+
 /**
  * The default page cache request policy.
  *
@@ -18,10 +20,13 @@ class DefaultRequestPolicy extends ChainRequestPolicy {
 
   /**
    * Constructs the default page cache request policy.
+   *
+   * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration
+   *   The session configuration.
    */
-  public function __construct() {
+  public function __construct(SessionConfigurationInterface $session_configuration) {
     $this->addPolicy(new RequestPolicy\CommandLineOrUnsafeMethod());
-    $this->addPolicy(new RequestPolicy\NoSessionOpen());
+    $this->addPolicy(new RequestPolicy\NoSessionOpen($session_configuration));
   }
 
 }

core/lib/Drupal/Core/PageCache/RequestPolicy/NoSessionOpen.php

@@ -8,6 +8,7 @@
 namespace Drupal\Core\PageCache\RequestPolicy;
 
 use Drupal\Core\PageCache\RequestPolicyInterface;
+use Drupal\Core\Session\SessionConfigurationInterface;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -21,27 +22,27 @@
 class NoSessionOpen implements RequestPolicyInterface {
 
   /**
-   * The name of the session cookie.
+   * The session configuration.
    *
-   * @var string
+   * @var \Drupal\Core\Session\SessionConfigurationInterface
    */
-  protected $sessionCookieName;
+  protected $sessionConfiguration;
 
   /**
    * Constructs a new page cache session policy.
    *
-   * @param string $session_cookie_name
-   *   (optional) The name of the session cookie. Defaults to session_name().
+   * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration
+   *   The session configuration.
    */
-  public function __construct($session_cookie_name = NULL) {
-    $this->sessionCookieName = $session_cookie_name ?: session_name();
+  public function __construct(SessionConfigurationInterface $session_configuration) {
+    $this->sessionConfiguration = $session_configuration;
   }
 
   /**
    * {@inheritdoc}
    */
   public function check(Request $request) {
-    if (!$request->cookies->has($this->sessionCookieName)) {
+    if (!$this->sessionConfiguration->hasSession($request)) {
       return static::ALLOW;
     }
   }

core/lib/Drupal/Core/Session/SessionConfiguration.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Session\SessionConfiguration
+ */
+
+namespace Drupal\Core\Session;
+
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Defines the default session configuration generator.
+ */
+class SessionConfiguration implements SessionConfigurationInterface {
+
+  /**
+   * An associative array of session ini settings.
+   */
+  protected $options;
+
+  /**
+   * Constructs a new session configuration instance.
+   *
+   * @param array $options
+   *   An associative array of session ini settings.
+   *
+   * @see \Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage::__construct()
+   * @see http://php.net/manual/session.configuration.php
+   */
+  public function __construct($options = []) {
+    $this->options = $options;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function hasSession(Request $request) {
+    return $request->cookies->has($this->getName($request));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getOptions(Request $request) {
+    $options = $this->options;
+
+    // Generate / validate the cookie domain.
+    $options['cookie_domain'] = $this->getCookieDomain($request) ?: '';
+
+    // If the site is accessed via SSL, ensure that the session cookie is
+    // issued with the secure flag.
+    $options['cookie_secure'] = $request->isSecure();
+
+    // Set the session cookie name.
+    $options['name'] = $this->getName($request);
+
+    return $options;
+  }
+
+  /**
+   * Returns the session cookie name.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @return string
+   *   The name of the session cookie.
+   */
+  protected function getName(Request $request) {
+    // To prevent session cookies from being hijacked, a user can configure the
+    // SSL version of their website to only transfer session cookies via SSL by
+    // using PHP's session.cookie_secure setting. The browser will then use two
+    // separate session cookies for the HTTPS and HTTP versions of the site. So
+    // we must use different session identifiers for HTTPS and HTTP to prevent a
+    // cookie collision.
+    $prefix = $request->isSecure() ? 'SSESS' : 'SESS';
+    return $prefix . $this->getUnprefixedName($request);
+  }
+
+  /**
+   * Returns the session cookie name without the secure/insecure prefix.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @returns string
+   *   The session name without the prefix (SESS/SSESS).
+   */
+  protected function getUnprefixedName(Request $request) {
+    if ($test_prefix = $this->drupalValidTestUa()) {
+      $session_name = $test_prefix;
+    }
+    elseif (isset($this->options['cookie_domain'])) {
+      // If the user specifies the cookie domain, also use it for session name.
+      $session_name = $this->options['cookie_domain'];
+    }
+    else {
+      // Otherwise use $base_url as session name, without the protocol
+      // to use the same session identifiers across HTTP and HTTPS.
+      $session_name = $request->getHost() . $request->getBasePath();
+      // Replace "core" out of session_name so core scripts redirect properly,
+      // specifically install.php.
+      $session_name = preg_replace('#/core$#', '', $session_name);
+    }
+
+    return substr(hash('sha256', $session_name), 0, 32);
+  }
+
+  /**
+   * Return the session cookie domain.
+   *
+   * The Set-Cookie response header and its domain attribute are defined in RFC
+   * 2109, RFC 2965 and RFC 6265 each one superseeding the previous version.
+   *
+   * @see http://tools.ietf.org/html/rfc2109
+   * @see http://tools.ietf.org/html/rfc2965
+   * @see http://tools.ietf.org/html/rfc6265
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @returns string
+   *   The session cookie domain.
+   */
+  protected function getCookieDomain(Request $request) {
+    if (isset($this->options['cookie_domain'])) {
+      $cookie_domain = $this->options['cookie_domain'];
+    }
+    else {
+      $host = $request->getHost();
+
+      // Strip www. from hostname.
+      if (strpos($host, 'www.') === 0) {
+        $host = substr($host, 4);
+      }
+
+      // To maximize compatibility and normalize the behavior across user
+      // agents, the cookie domain should start with a dot.
+      $cookie_domain = '.' . $host;
+    }
+
+    // Cookies for domains without an embedded dot will be rejected by user
+    // agents in order to defeat malicious websites attempting to set cookies
+    // for top-level domains. Also IP addresses may not be used in the domain
+    // attribute of a Set-Cookie header.
+    if (count(explode('.', $cookie_domain)) > 2 && !is_numeric(str_replace('.', '', $cookie_domain))) {
+      return $cookie_domain;
+    }
+  }
+
+  /**
+   * Wraps drupal_valid_test_ua().
+   *
+   * @return string|FALSE
+   *   Either the simpletest prefix (the string "simpletest" followed by any
+   *   number of digits) or FALSE if the user agent does not contain a valid
+   *   HMAC and timestamp.
+   */
+  protected function drupalValidTestUa() {
+    return drupal_valid_test_ua();
+  }
+
+}

core/lib/Drupal/Core/Session/SessionConfigurationInterface.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Session\SessionConfigurationInterface
+ */
+
+namespace Drupal\Core\Session;
+
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Defines an interface for session configuration generators.
+ */
+interface SessionConfigurationInterface {
+
+  /**
+   * Determines whether a session identifier is on the request.
+   *
+   * This method detects whether a session was started during one of the
+   * previous requests from the same user agent. Session identifiers are
+   * normally passed along using cookies and hence a typical implementation
+   * checks whether the session cookie is on the request.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @return bool
+   *   TRUE if there is a session identifier on the request.
+   */
+  public function hasSession(Request $request);
+
+  /**
+   * Returns a list of options suitable for passing to the session storage.
+   *
+   * @see \Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage::__construct()
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @return array
+   *   An associative array of session ini settings.
+   */
+  public function getOptions(Request $request);
+
+}

core/lib/Drupal/Core/Session/SessionManager.php

@@ -11,7 +11,6 @@
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Session\AnonymousUserSession;
 use Drupal\Core\Session\SessionHandler;
-use Drupal\Core\Site\Settings;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler;
 use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;
@@ -49,6 +48,13 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
    */
   protected $connection;
 
+  /**
+   * The session configuration.
+   *
+   * @var \Drupal\Core\Session\SessionConfigurationInterface
+   */
+  protected $sessionConfiguration;
+
   /**
    * Whether a lazy session has been started.
    *
@@ -76,11 +82,12 @@ class SessionManager extends NativeSessionStorage implements SessionManagerInter
    *   The database connection.
    * @param \Drupal\Core\Session\MetadataBag $metadata_bag
    *   The session metadata bag.
-   * @param \Drupal\Core\Site\Settings $settings
-   *   The settings instance.
+   * @param \Drupal\Core\Session\SessionConfigurationInterface $session_configuration
+   *   The session configuration interface.
    */
-  public function __construct(RequestStack $request_stack, Connection $connection, MetadataBag $metadata_bag, Settings $settings) {
+  public function __construct(RequestStack $request_stack, Connection $connection, MetadataBag $metadata_bag, SessionConfigurationInterface $session_configuration) {
     $options = array();
+    $this->sessionConfiguration = $session_configuration;
     $this->requestStack = $request_stack;
     $this->connection = $connection;
 
@@ -111,8 +118,10 @@ public function start() {
       return $this->started;
     }
 
-    $cookies = $this->requestStack->getCurrentRequest()->cookies;
-    if ($cookies->get($this->getName())) {
+    $request = $this->requestStack->getCurrentRequest();
+    $this->setOptions($this->sessionConfiguration->getOptions($request));
+
+    if ($this->sessionConfiguration->hasSession($request)) {
       // If a session cookie exists, initialize the session. Otherwise the
       // session is only started on demand in save(), making
       // anonymous users not use a session cookie unless something is stored in

core/lib/Drupal/Core/Site/Settings.php

@@ -111,7 +111,7 @@ public static function getAll() {
    */
   public static function initialize($app_root, $site_path, &$class_loader) {
     // Export these settings.php variables to the global namespace.
-    global $base_url, $cookie_domain, $config_directories, $config;
+    global $base_url, $config_directories, $config;
     $settings = array();
     $config = array();
     $databases = array();

core/modules/simpletest/src/WebTestBase.php

@@ -778,11 +778,10 @@ protected function setUp() {
       'pass_raw' => $this->randomMachineName(),
     ));
 
-    // Some tests (SessionTest and SessionHttpsTest) need to examine whether the
-    // proper session cookies were set on a response. Because the child site
-    // uses the same session name as the test runner, it is necessary to make
-    // that available to test-methods.
-    $this->sessionName = $this->originalSessionName;
+    // The child site derives its session name from the database prefix when
+    // running web tests.
+    $prefix = (Request::createFromGlobals()->isSecure() ? 'SSESS' : 'SESS');
+    $this->sessionName = $prefix . substr(hash('sha256', $this->databasePrefix), 0, 32);
 
     // Reset the static batch to remove Simpletest's batch operations.
     $batch = &batch_get();

core/tests/Drupal/Tests/Core/PageCache/NoSessionOpenTest.php

@@ -19,11 +19,11 @@
 class NoSessionOpenTest extends UnitTestCase {
 
   /**
-   * The session cookie name.
+   * The session configuration.
    *
-   * @var string
+   * @var \Drupal\Core\Session\SessionConfigurationInterface|\PHPUnit_Framework_MockObject_MockObject
    */
-  protected $sessionCookieName;
+  protected $sessionConfiguration;
 
   /**
    * The request policy under test.
@@ -33,8 +33,8 @@ class NoSessionOpenTest extends UnitTestCase {
   protected $policy;
 
   public function setUp() {
-    $this->sessionCookieName = 'B1ESkdf3V4F8u27myaSAShuuHc';
-    $this->policy = new RequestPolicy\NoSessionOpen($this->sessionCookieName);
+    $this->sessionConfiguration = $this->getMock('Drupal\Core\Session\SessionConfigurationInterface');
+    $this->policy = new RequestPolicy\NoSessionOpen($this->sessionConfiguration);
   }
 
   /**
@@ -43,12 +43,23 @@ public function setUp() {
    * @covers ::check
    */
   public function testNoAllowUnlessSessionCookiePresent() {
-    $request = new Request();
-    $result = $this->policy->check($request);
+    $request_without_session = new Request();
+    $request_with_session = Request::create('/', 'GET', [], ['some-session-name' => 'some-session-id']);
+
+    $this->sessionConfiguration->expects($this->at(0))
+      ->method('hasSession')
+      ->with($request_without_session)
+      ->will($this->returnValue(FALSE));
+
+    $this->sessionConfiguration->expects($this->at(1))
+      ->method('hasSession')
+      ->with($request_with_session)
+      ->will($this->returnValue(TRUE));
+
+    $result = $this->policy->check($request_without_session);
     $this->assertSame(RequestPolicyInterface::ALLOW, $result);
 
-    $request = Request::create('/', 'GET', [], [$this->sessionCookieName => 'some-session-id']);
-    $result = $this->policy->check($request);
+    $result = $this->policy->check($request_with_session);
     $this->assertSame(NULL, $result);
   }
 }

core/tests/Drupal/Tests/Core/Session/SessionConfigurationTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Session\SessionConfigurationTest.
+ */
+
+namespace Drupal\Tests\Core\Session;
+
+use Drupal\Tests\UnitTestCase;
+use Drupal\Core\Session\SessionConfiguration;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Session\SessionConfiguration
+ * @group Session
+ */
+class SessionConfigurationTest extends UnitTestCase {
+
+  /**
+   * Constructs a partially mocked SUT.
+   *
+   * @returns \Drupal\Core\Session\SessionConfiguration|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected function createSessionConfiguration($options = []) {
+    return $this->getMock('Drupal\Core\Session\SessionConfiguration', ['drupalValidTestUa'], [$options]);
+  }
+
+  /**
+   * Tests whether the session.cookie_domain ini settings is computed correctly.
+   *
+   * @covers ::getOptions()
+   *
+   * @dataProvider providerTestGeneratedCookieDomain
+   */
+  public function testGeneratedCookieDomain($uri, $expected_domain) {
+    $config = $this->createSessionConfiguration();
+
+    $request = Request::create($uri);
+    $options = $config->getOptions($request);
+
+    $this->assertEquals($expected_domain, $options['cookie_domain']);
+  }
+
+  /**
+   * Data provider for the cookie domain test.
+   *
+   * @returns array
+   *   Test data
+   */
+  public function providerTestGeneratedCookieDomain() {
+    return [
+      ['http://example.com/path/index.php', '.example.com'],
+      ['http://www.example.com/path/index.php', '.example.com'],
+      ['http://subdomain.example.com/path/index.php', '.subdomain.example.com'],
+      ['http://example.com:8080/path/index.php', '.example.com'],
+      ['https://example.com/path/index.php', '.example.com'],
+      ['http://localhost/path/index.php', ''],
+      ['http://127.0.0.1/path/index.php', ''],
+      ['http://127.0.0.1:8888/path/index.php', ''],
+      ['http://1.1.1.1/path/index.php', ''],
+      ['http://[::1]/path/index.php', ''],
+      ['http://[::1]:8888/path/index.php', ''],
+    ];
+  }
+
+  /**
+   * Tests the constructor injected session.cookie_domain ini setting.
+   *
+   * @covers ::__construct()
+   * @covers ::getOptions()
+   *
+   * @dataProvider providerTestEnforcedCookieDomain
+   */
+  public function testEnforcedCookieDomain($uri, $expected_domain) {
+    $config = $this->createSessionConfiguration(['cookie_domain' => '.example.com']);
+
+    $request = Request::create($uri);
+    $options = $config->getOptions($request);
+
+    $this->assertEquals($expected_domain, $options['cookie_domain']);
+  }
+
+  /**
+   * Data provider for the cookie domain test.
+   *
+   * @returns array
+   *   Test data
+   */
+  public function providerTestEnforcedCookieDomain() {
+    return [
+      ['http://example.com/path/index.php', '.example.com'],
+      ['http://www.example.com/path/index.php', '.example.com'],