From 83e025eede225579ce3120d3620f812fb037b3b2 Mon Sep 17 00:00:00 2001
From: Thomas Steur <tsteur@users.noreply.github.com>
Date: Mon, 23 Sep 2019 09:01:09 +1200
Subject: If session was already started, do not start it again (#14896)

* If session was already started, do not start it again

refs https://github.com/matomo-org/matomo/issues/12963
refs https://forum.matomo.org/t/an-error-occurred/32500

* Update Session.php

* Update bootstrap.php

* add test
---
 core/Session.php                          |  7 ++++++-
 core/bootstrap.php                        |  4 +++-
 tests/PHPUnit/Integration/SessionTest.php | 30 +++++++++++++++++++++++++++++
 tests/resources/sessionStarter.php        | 32 +++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 tests/PHPUnit/Integration/SessionTest.php
 create mode 100644 tests/resources/sessionStarter.php

diff --git a/core/Session.php b/core/Session.php
index c916893706..024c5246d8 100644
--- a/core/Session.php
+++ b/core/Session.php
@@ -50,6 +50,7 @@ class Session extends Zend_Session
         if (headers_sent()
             || self::$sessionStarted
             || (defined('PIWIK_ENABLE_SESSION_START') && !PIWIK_ENABLE_SESSION_START)
+            || session_status() == PHP_SESSION_ACTIVE
         ) {
             return;
         }
@@ -173,7 +174,11 @@ class Session extends Zend_Session
 
     public static function close()
     {
-        parent::writeClose();
+        if (self::isSessionStarted()) {
+            // only write/close session if the session was actually started by us
+            // otherwise we will set the session values to base64 encoded and whoever the session started might not expect the values in that way
+            parent::writeClose();
+        }
     }
 
     public static function isSessionStarted()
diff --git a/core/bootstrap.php b/core/bootstrap.php
index 57c3f95f8d..04d161a57b 100644
--- a/core/bootstrap.php
+++ b/core/bootstrap.php
@@ -29,7 +29,9 @@ if (!defined('PIWIK_VENDOR_PATH')) {
 // NOTE: the code above must be PHP 4 compatible
 require_once PIWIK_INCLUDE_PATH . '/core/testMinimumPhpVersion.php';
 
-session_cache_limiter('nocache');
+if (session_status() !== PHP_SESSION_ACTIVE) {
+    session_cache_limiter('nocache');
+}
 
 define('PIWIK_DEFAULT_TIMEZONE', @date_default_timezone_get());
 @date_default_timezone_set('UTC');
diff --git a/tests/PHPUnit/Integration/SessionTest.php b/tests/PHPUnit/Integration/SessionTest.php
new file mode 100644
index 0000000000..8c6eade11c
--- /dev/null
+++ b/tests/PHPUnit/Integration/SessionTest.php
@@ -0,0 +1,30 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Tests\Integration;
+
+use Piwik\Http;
+use Piwik\Piwik;
+use Piwik\Session;
+use Piwik\Tests\Framework\Fixture;
+
+/**
+ * @group Core
+ * @group Session
+ * @group SessionTest
+ */
+class SessionTest extends \PHPUnit_Framework_TestCase
+{
+	public function test_session_should_not_be_started_if_it_was_already_started()
+	{
+        $url = Fixture::getRootUrl() . '/tests/resources/sessionStarter.php';
+	    $result = Http::sendHttpRequest($url, 5);
+	    $this->assertSame('ok', trim($result));
+	}
+
+}
diff --git a/tests/resources/sessionStarter.php b/tests/resources/sessionStarter.php
new file mode 100644
index 0000000000..48a4954934
--- /dev/null
+++ b/tests/resources/sessionStarter.php
@@ -0,0 +1,32 @@
+<?php
+/**
+ * This php file is used to unit test Piwik::serverStaticFile()
+ * To make a comprehensive test suit for Piwik::serverStaticFile() (ie. being able to test combinations of request
+ * headers, being able to test response headers and so on) we need to simulate static file requests in a controlled
+ * environment
+ * The php code which simulates requests using Piwik::serverStaticFile() is provided in the same file (ie. this one)
+ * as the unit testing code for Piwik::serverStaticFile()
+ * This decision has a structural impact on the usual unit test file structure
+ * serverStaticFile.test.php has been created to avoid making too many modifications to /tests/core/Piwik.test.php
+ */
+use Piwik\FrontController;
+session_start(); // matomo should not fail if session was started by someone else
+define('PIWIK_DOCUMENT_ROOT', dirname(__FILE__).'/../../');
+if(file_exists(PIWIK_DOCUMENT_ROOT . '/bootstrap.php')) {
+    require_once PIWIK_DOCUMENT_ROOT . '/bootstrap.php';
+}
+if (!defined('PIWIK_INCLUDE_PATH')) {
+    define('PIWIK_INCLUDE_PATH', PIWIK_DOCUMENT_ROOT);
+}
+
+require_once PIWIK_INCLUDE_PATH . '/core/bootstrap.php';
+
+$environment = new \Piwik\Application\Environment(null);
+try {
+    $environment->init();
+} catch(\Piwik\Exception\NotYetInstalledException $e) {
+    die($e->getMessage());
+}
+FrontController::getInstance()->init();
+
+echo 'ok';
\ No newline at end of file
-- 
cgit v1.2.3