From 49ac54b365c39c02023bed70628a7f2dacd3ba3f Mon Sep 17 00:00:00 2001 From: ralf Date: Tue, 27 Sep 2022 19:02:20 +0200 Subject: [PATCH] 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 fe4d0dbbe32a57b7f5d690a04770f4aea1b78fa0. --- api/src/Cache.php | 132 +++++---------------------------------- api/src/Egw.php | 3 - api/src/Framework.php | 1 - api/src/Json/Request.php | 7 +-- api/src/Session.php | 20 +++--- json.php | 2 +- 6 files changed, 26 insertions(+), 139 deletions(-) diff --git a/api/src/Cache.php b/api/src/Cache.php index 6493b3396a..426bf8f385 100644 --- a/api/src/Cache.php +++ b/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 ($_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] !== $data) + if (isset($_SESSION[Session::EGW_SESSION_ENCRYPTED])) { - $_SESSION[Session::EGW_APPSESSION_VAR][$app][$location] = $data; - self::checkSetClosedSession($app, $location, $data); + 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 } + $_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; } @@ -997,4 +897,4 @@ if (is_null(Cache::$default_provider)) 'EGroupware\Api\Cache\Files')); } -//error_log('Cache::$default_provider='.array2string(Cache::$default_provider)); \ No newline at end of file +//error_log('Cache::$default_provider='.array2string(Cache::$default_provider)); diff --git a/api/src/Egw.php b/api/src/Egw.php index 7ba918af0a..676ae161ce 100644 --- a/api/src/Egw.php +++ b/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()) { diff --git a/api/src/Framework.php b/api/src/Framework.php index 8eb0aef469..69e4bc5739 100644 --- a/api/src/Framework.php +++ b/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)) { diff --git a/api/src/Json/Request.php b/api/src/Json/Request.php index 4d222e5080..04ffbad92b 100644 --- a/api/src/Json/Request.php +++ b/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')) - { - notifications_push::get(); - } + if (class_exists('notifications_push')) notifications_push::get(); } /** @@ -204,4 +201,4 @@ class Request } -} \ No newline at end of file +} diff --git a/api/src/Session.php b/api/src/Session.php index e0215195e3..597c252553 100644 --- a/api/src/Session.php +++ b/api/src/Session.php @@ -298,15 +298,10 @@ class Session */ 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()); + if (self::ERROR_LOG_DEBUG) error_log(__METHOD__."() sessionid=$this->sessionid, _SESSION[".self::EGW_SESSION_VAR.']='.array2string($_SESSION[self::EGW_SESSION_VAR]).' '.function_backtrace()); + self::encrypt($this->kp3); - //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(); - } + session_write_close(); } /** @@ -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; @@ -2199,4 +2193,4 @@ class Session 'notification_heartbeat > '.self::heartbeat_limit(), ), __LINE__, __FILE__)->fetchColumn(); } -} \ No newline at end of file +} diff --git a/json.php b/json.php index c9f398c33b..2229aa645c 100644 --- a/json.php +++ b/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();