Revert "json requests now close the PHP session immediately again and reopen it, if there was an update to the session" as it breaks Collabora editing

This reverts commit fe4d0dbbe3.

api/src/Cache.php

@@ -483,98 +483,6 @@ class Cache
 	 */
 	const SESSION_EXPIRATION_PREFIX = '*expiration*';
 
-	protected static $closed_session_sets = [];
-
-	/**
-	 * If session is already closed, record value(s) and reopen session to store them on shutdown
-	 *
-	 * @param string $app
-	 * @param string $location
-	 * @param mixed $data =null null: unset
-	 * @param int $expiration
-	 * @return void
-	 */
-	protected static function checkSetClosedSession($app, $location, $data=null, $expiration=0)
-	{
-		if (isset($_SESSION) && session_status() !== PHP_SESSION_ACTIVE)
-		{
-			self::$closed_session_sets[$app][$location] = $data;
-			//error_log(__METHOD__."(".json_encode(func_get_args()).") session_status()=closed");
-		}
-	}
-
-	/**
-	 * Called on shutdown to re-open session and store values set since session was closed
-	 *
-	 * It is called in Egw::onShutdown(), before any output is sent, as we can NOT reopen the session after!
-	 */
-	public static function onShutdown()
-	{
-		// first run closures used to check references returned by getSession() are used for updates
-		foreach(self::$closed_session_sets as $app => $app_data)
-		{
-			foreach($app_data as $location => $func)
-			{
-				if (is_callable($func))
-				{
-					$data = $func();
-					if (isset($data))
-					{
-						self::$closed_session_sets[$app][$location] = $data;
-					}
-					else
-					{
-						unset(self::$closed_session_sets[$app][$location]);
-						if (!self::$closed_session_sets[$app])
-						{
-							unset(self::$closed_session_sets[$app]);
-						}
-					}
-				}
-			}
-		}
-		// if we really have to (un)set something, re-open the session and do so
-		if (self::$closed_session_sets)
-		{
-			if (empty($GLOBALS['egw']->session) || empty($GLOBALS['egw']->session->sessionid))
-			{
-				// no session, eg. async service
-				return;
-			}
-			if (headers_sent())
-			{
-				error_log(__METHOD__."() headers_sent() --> can not re-open session of $_SERVER[REQUEST_URI] to set: ".json_encode(self::$closed_session_sets));
-				return;
-			}
-			if (!session_start())
-			{
-				error_log(__METHOD__."() could NOT reopen session of $_SERVER[REQUEST_URI] to set: ".json_encode(self::$closed_session_sets));
-				return;
-			}
-			foreach(self::$closed_session_sets as $app => $app_data)
-			{
-				foreach($app_data as $location => $data)
-				{
-					if (is_callable($data))
-					{
-						$data();
-					}
-					elseif (isset($data))
-					{
-						self::setSession($app, $location, $data);
-					}
-					else
-					{
-						self::unsetSession($app, $location);
-					}
-				}
-			}
-			error_log(__METHOD__."() updated session of $_SERVER[REQUEST_URI] with ".json_encode(self::$closed_session_sets));
-			$GLOBALS['egw']->session->commit_session();
-			self::$closed_session_sets = [];
-		}
-	}
-
 	/**
 	 * Set some data in the cache for the whole source tree (all instances)
 	 *
@@ -586,17 +494,16 @@ class Cache
 	 */
 	public static function setSession($app,$location,$data,$expiration=0)
 	{
-		// only update, if there is a change, no need to reopen session otherwise
+		if (isset($_SESSION[Session::EGW_SESSION_ENCRYPTED]))
-		if ($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] !== $data)
 		{
-			$_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] = $data;
+			if (Session::ERROR_LOG_DEBUG) error_log(__METHOD__.' called after session was encrypted --> ignored!');
-			self::checkSetClosedSession($app, $location, $data);
+			return false;	// can no longer store something in the session, eg. because commit_session() was called
 		}
+		$_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] = $data;
 
 		if ($expiration > 0)
 		{
 			$_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location] = time()+$expiration;
-			self::checkSetClosedSession(self::SESSION_EXPIRATION_PREFIX.$app, $location, time()+$expiration);
 		}
 
 		return true;
@@ -607,10 +514,6 @@ class Cache
 	 *
 	 * Returns a reference to the var in the session!
 	 *
-	 * Better would be two separate functions one returning a value and the other a reference,
-	 * as we now need to check via a closure, if the reference was used for an update,
-	 * to reopen the session and run the update (use "=\s*&.*Cache::getSession" to find references).
-	 *
 	 * @param string $app application storing data
 	 * @param string $location location name for data
 	 * @param callback $callback =null callback to get/create the value, if it's not cache
@@ -620,6 +523,12 @@ class Cache
 	 */
 	public static function &getSession($app, $location, $callback=null, array $callback_params=array(), $expiration=0)
 	{
+		if (isset($_SESSION[Session::EGW_SESSION_ENCRYPTED]))
+		{
+			if (Session::ERROR_LOG_DEBUG) error_log(__METHOD__.' called after session was encrypted --> ignored!');
+			$ret = null;	// can no longer store something in the session, eg. because commit_session() was called
+			return $ret;
+		}
 		// check if entry is expired and clean it up in that case
 		if (isset($_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location]) &&
 			$_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location] < time())
@@ -630,18 +539,7 @@ class Cache
 		if (!isset($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location]) && !is_null($callback))
 		{
 			$_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] = call_user_func_array($callback,$callback_params);
-			self::checkSetClosedSession($app, $location, $_SESSION[Session::EGW_APPSESSION_VAR][$app][$location]);
 		}
-		// as getSession returns a reference, which can be used to update the session, we store a function to check,
-		// if the value was actually changed and only in that case return it
-		$data = $_SESSION[Session::EGW_APPSESSION_VAR][$app][$location];
-		self::checkSetClosedSession($app, $location, function() use ($app, $location, $data)
-		{
-			if ($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] !== $data)
-			{
-				return $_SESSION[Session::EGW_APPSESSION_VAR][$app][$location];
-			}
-		});
 		return $_SESSION[Session::EGW_APPSESSION_VAR][$app][$location];
 	}
 
@@ -654,21 +552,23 @@ class Cache
 	 */
 	public static function unsetSession($app,$location)
 	{
+		if (isset($_SESSION[Session::EGW_SESSION_ENCRYPTED]))
+		{
+			if (Session::ERROR_LOG_DEBUG) error_log(__METHOD__.' called after session was encrypted --> ignored!');
+			return false;	// can no longer store something in the session, eg. because commit_session() was called
+		}
 		// check if entry is expired and clean it up in that case
 		if (isset($_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location]) &&
 			$_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location] < time())
 		{
 			unset($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location],
 				$_SESSION[Session::EGW_APPSESSION_VAR][self::SESSION_EXPIRATION_PREFIX.$app][$location]);
-			self::checkSetClosedSession($app, $location, null);
-			self::checkSetClosedSession(self::SESSION_EXPIRATION_PREFIX.$app, $location, null);
 		}
 		if (!isset($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location]))
 		{
 			return false;
 		}
 		unset($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location]);
-		self::checkSetClosedSession($app, $location, null);
 
 		return true;
 	}

api/src/Egw.php

@@ -565,9 +565,6 @@ class Egw extends Egw\Base
 		{
 			define('EGW_SHUTDOWN',True);
 
-			// must be before headers are sent, as we somehow can NOT reopen the session (to update session vars) after
-			Cache::onShutdown();
-
 			// send json response BEFORE flushing output
 			if (Json\Request::isJSONRequest())
 			{

api/src/Framework.php

@@ -1616,7 +1616,6 @@ abstract class Framework extends Framework\Extra
 	{
 		// dont block session, while we read preferences, they are not supposed to change something in the session
 		$GLOBALS['egw']->session->commit_session();
-		Session::cache_control(true);   // allow browser to cache
 
 		if (preg_match('/^[a-z0-9_]+$/i', $app))
 		{

api/src/Json/Request.php

@@ -104,10 +104,7 @@ class Request
 			$this->handleRequest($menuaction, $parameters);
 		}
 		// check if we have push notifications, if notifications app available
-		if (Push::onlyFallback() && class_exists('notifications_push'))
+		if (class_exists('notifications_push')) notifications_push::get();
-		{
-			notifications_push::get();
-		}
 	}
 
 	/**

api/src/Session.php

@@ -297,17 +297,12 @@ class Session
 	 * It's necessary to use this function instead of session_write_close() direct, as otherwise the session is not encrypted!
 	 */
 	function commit_session()
-	{
-		if (session_status() == PHP_SESSION_ACTIVE)
 	{
 		if (self::ERROR_LOG_DEBUG) error_log(__METHOD__."() sessionid=$this->sessionid, _SESSION[".self::EGW_SESSION_VAR.']='.array2string($_SESSION[self::EGW_SESSION_VAR]).' '.function_backtrace());
-
-			//error_log(__METHOD__."() $_SERVER[REQUEST_URI] closing session after ".number_format(microtime(true)-$_SERVER['REQUEST_TIME_FLOAT'], 3));
 		self::encrypt($this->kp3);
 
 		session_write_close();
 	}
-	}
 
 	/**
 	 * Keys of session variables which get encrypted
@@ -1279,11 +1274,10 @@ class Session
 			case PHP_SESSION_DISABLED:
 				throw new ErrorException('EGroupware requires the PHP session extension!');
 			case PHP_SESSION_NONE:
-				if (isset($_SESSION)) break;  // session already started, but closed again
 				session_name(self::EGW_SESSION_NAME);
 				session_id($this->sessionid);
 				self::cache_control();
-				session_start(empty($GLOBALS['egw_info']['flags']['close_session']) ? [] : ['read_and_close' => true]);
+				session_start();
 				break;
 			case PHP_SESSION_ACTIVE:
 				// session already started eg. by managementserver_client
@@ -2015,14 +2009,14 @@ class Session
 			case PHP_SESSION_DISABLED:
 				throw new \ErrorException('EGroupware requires PHP session extension!');
 			case PHP_SESSION_NONE:
-				if (isset($_SESSION)) return true;  // session already started, but closed again
 				if (headers_sent()) return false;	// only gives warnings
+				ini_set('session.use_cookies',0);	// disable the automatic use of cookies, as it uses the path / by default
 				session_name(self::EGW_SESSION_NAME);
 				if (isset($sessionid) || ($sessionid = self::get_sessionid()))
 				{
 					session_id($sessionid);
 					self::cache_control();
-					$ok = session_start(empty($GLOBALS['egw_info']['flags']['close_session']) ? [] : ['read_and_close' => true]);
+					$ok = session_start();
 					self::decrypt();
 					if (self::ERROR_LOG_DEBUG) error_log(__METHOD__."() sessionid=$sessionid, _SESSION[".self::EGW_SESSION_VAR.']='.array2string($_SESSION[self::EGW_SESSION_VAR]));
 					return $ok;

json.php

@@ -110,11 +110,11 @@ try {
 			// only log ajax requests which represent former GET requests or submits
 			// cuts down updates to egw_access_log table
 			'no_dla_update' => !preg_match('/(Etemplate::ajax_process_content|\.jdots_framework\.ajax_exec\.template)/', $_GET['menuaction']),
-			'close_session' => true,
 		)
 	);
 	include_once('./header.inc.php');
 
+
 	//Create a new json handler
 	$json = new Json\Request();