From b98c87a6c51baa6696ef9f077775f633c4c5ecd4 Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Wed, 19 Sep 2018 14:24:40 +1200
Subject: [PATCH] FIX: Ensure existing session can be accessed if
 headers_sent()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If a session already exists, and Session::start() isn’t called until
after a large enough block of content is output, then headers_sent()
will be false. The previous code prevented the session from being
started in this case. That might makes sense for the creation of a new
session, but it prevent legitimate access to an existing session.

This mostly manifested when running debugging tools such as showqueries,
which may output content before the session is started.
---
 src/Control/Session.php | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/Control/Session.php b/src/Control/Session.php
index f9f95d1cf..6739409ee 100644
--- a/src/Control/Session.php
+++ b/src/Control/Session.php
@@ -298,11 +298,20 @@ class Session
             }
         }
 
-        if (!session_id() && !headers_sent()) {
-            if ($domain) {
-                session_set_cookie_params($timeout, $path, $domain, $secure, true);
+        // If the session cookie is already set, then the session can be read even if headers_sent() = true
+        // This helps with edge-case such as debugging.
+        if (!session_id() && (!headers_sent() || !empty($_COOKIE[ini_get('session.name')]))) {
+            if (!headers_sent()) {
+                session_set_cookie_params($timeout, $path, $domain ?: null, $secure, true);
+
+                $limiter = $this->config()->get('sessionCacheLimiter');
+                if (isset($limiter)) {
+                    session_cache_limiter($limiter);
+                }
+
+            // If headers are sent then we can't have a session_cache_limiter otherwise we'll get a warning
             } else {
-                session_set_cookie_params($timeout, $path, null, $secure, true);
+                session_cache_limiter(null);
             }
 
             // Allow storing the session in a non standard location
@@ -311,16 +320,13 @@ class Session
             }
 
             // If we want a secure cookie for HTTPS, use a seperate session name. This lets us have a
-            // seperate (less secure) session for non-HTTPS requests
+            // seperate (less secure) session for non-HTTPS requests. Note that if this causes problems
+            // if headers_sent() is true then it's best to throw the resulting error rather than risk
+            // a security hole.
             if ($secure) {
                 session_name($this->config()->get('cookie_name_secure'));
             }
 
-            $limiter = $this->config()->get('sessionCacheLimiter');
-            if (isset($limiter)) {
-                session_cache_limiter($limiter);
-            }
-
             session_start();
 
             if (isset($_SESSION)) {