diff options
| author | 2020-10-06 23:19:45 +0200 | |
|---|---|---|
| committer | 2020-10-06 23:19:45 +0200 | |
| commit | 0319cc9d234e107109d988f36f2361b25f9f0777 (patch) | |
| tree | e373d93694297e36056d9888141d3233d0686260 | |
| parent | 3aed0b95534c60b26254292e951c8a9c5badc786 (diff) | |
Minz allow parallel sessions (#3096)
* Minz allow parallel sessions
#fix https://github.com/FreshRSS/FreshRSS/issues/3093
* Array optimisation
* Array optimisation missing
* Reduce direct access to $_SESSION except in install process
* Fix session start headers warning
* Use cookie only the first time the session is started:
`PHP Warning: session_start(): Cannot start session when headers
already sent in /var/www/FreshRSS/lib/Minz/Session.php on line 39`
* New concept of volatile session for API calls
Optimisation: do not use cookies or local storage at all for API calls
without a Web session
Fix warning:
```
PHP Warning: session_destroy(): Trying to destroy uninitialized session
in Unknown on line 0
```
* Only call Minz_Session::init once in our index
It was called twice (once indirectly via FreshRSS->init())
* Whitespace
* Mutex for notifications
Implement mutex for notifications
https://github.com/FreshRSS/FreshRSS/pull/3208#discussion_r499509809
* Typo
* Install script is not ready for using Minz_Session
| -rw-r--r-- | app/Controllers/authController.php | 16 | ||||
| -rw-r--r-- | app/Controllers/errorController.php | 6 | ||||
| -rw-r--r-- | app/Controllers/userController.php | 8 | ||||
| -rw-r--r-- | app/Models/Auth.php | 34 | ||||
| -rw-r--r-- | app/Models/DatabaseDAO.php | 12 | ||||
| -rwxr-xr-x | app/actualize_script.php | 6 | ||||
| -rw-r--r-- | app/install.php | 11 | ||||
| -rw-r--r-- | cli/_cli.php | 1 | ||||
| -rwxr-xr-x | cli/do-install.php | 8 | ||||
| -rw-r--r-- | lib/Minz/Error.php | 6 | ||||
| -rw-r--r-- | lib/Minz/Request.php | 6 | ||||
| -rw-r--r-- | lib/Minz/Session.php | 60 | ||||
| -rw-r--r-- | p/api/fever.php | 4 | ||||
| -rw-r--r-- | p/api/greader.php | 4 | ||||
| -rwxr-xr-x | p/i/index.php | 5 |
15 files changed, 136 insertions, 51 deletions
diff --git a/app/Controllers/authController.php b/app/Controllers/authController.php index e7bff363e..342c577e2 100644 --- a/app/Controllers/authController.php +++ b/app/Controllers/authController.php @@ -141,9 +141,11 @@ class FreshRSS_auth_Controller extends Minz_ActionController { ); if ($ok) { // Set session parameter to give access to the user. - Minz_Session::_param('currentUser', $username); - Minz_Session::_param('passwordHash', $conf->passwordHash); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $username, + 'passwordHash' => $conf->passwordHash, + 'csrf' => false, + ]); FreshRSS_Auth::giveAccess(); // Set cookie parameter if nedded. @@ -190,9 +192,11 @@ class FreshRSS_auth_Controller extends Minz_ActionController { $ok = password_verify($password, $s); unset($password); if ($ok) { - Minz_Session::_param('currentUser', $username); - Minz_Session::_param('passwordHash', $s); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $username, + 'passwordHash' => $s, + 'csrf' => false, + ]); FreshRSS_Auth::giveAccess(); Minz_Translate::init($conf->language); diff --git a/app/Controllers/errorController.php b/app/Controllers/errorController.php index b0bafda72..a5f901fd4 100644 --- a/app/Controllers/errorController.php +++ b/app/Controllers/errorController.php @@ -16,8 +16,10 @@ class FreshRSS_error_Controller extends Minz_ActionController { public function indexAction() { $code_int = Minz_Session::param('error_code', 404); $error_logs = Minz_Session::param('error_logs', array()); - Minz_Session::_param('error_code'); - Minz_Session::_param('error_logs'); + Minz_Session::_params([ + 'error_code' => false, + 'error_logs' => false, + ]); switch ($code_int) { case 200 : diff --git a/app/Controllers/userController.php b/app/Controllers/userController.php index 4dfd11751..06c9ebc04 100644 --- a/app/Controllers/userController.php +++ b/app/Controllers/userController.php @@ -350,9 +350,11 @@ class FreshRSS_user_Controller extends Minz_ActionController { // get started immediately. if ($ok && !FreshRSS_Auth::hasAccess('admin')) { $user_conf = get_user_configuration($new_user_name); - Minz_Session::_param('currentUser', $new_user_name); - Minz_Session::_param('passwordHash', $user_conf->passwordHash); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $new_user_name, + 'passwordHash' => $user_conf->passwordHash, + 'csrf' => false, + ]); FreshRSS_Auth::giveAccess(); } diff --git a/app/Models/Auth.php b/app/Models/Auth.php index fcbf37fa3..77a244843 100644 --- a/app/Models/Auth.php +++ b/app/Models/Auth.php @@ -23,8 +23,10 @@ class FreshRSS_Auth { if ($current_user === '') { $conf = Minz_Configuration::get('system'); $current_user = $conf->default_user; - Minz_Session::_param('currentUser', $current_user); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $current_user, + 'csrf' => false, + ]); } if (self::$login_ok) { @@ -55,9 +57,11 @@ class FreshRSS_Auth { $current_user = ''; if (isset($credentials[1])) { $current_user = trim($credentials[0]); - Minz_Session::_param('currentUser', $current_user); - Minz_Session::_param('passwordHash', trim($credentials[1])); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $current_user, + 'passwordHash' => trim($credentials[1]), + 'csrf' => false, + ]); } return $current_user != ''; case 'http_auth': @@ -79,8 +83,10 @@ class FreshRSS_Auth { ]); } if ($login_ok) { - Minz_Session::_param('currentUser', $current_user); - Minz_Session::_param('csrf'); + Minz_Session::_params([ + 'currentUser' => $current_user, + 'csrf' => false, + ]); } return $login_ok; case 'none': @@ -118,8 +124,10 @@ class FreshRSS_Auth { self::$login_ok = false; } - Minz_Session::_param('loginOk', self::$login_ok); - Minz_Session::_param('REMOTE_USER', httpAuthUser()); + Minz_Session::_params([ + 'loginOk' => self::$login_ok, + 'REMOTE_USER' => httpAuthUser(), + ]); return self::$login_ok; } @@ -153,9 +161,11 @@ class FreshRSS_Auth { */ public static function removeAccess() { self::$login_ok = false; - Minz_Session::_param('loginOk'); - Minz_Session::_param('csrf'); - Minz_Session::_param('REMOTE_USER'); + Minz_Session::_params([ + 'loginOk' => false, + 'csrf' => false, + 'REMOTE_USER' => false, + ]); $system_conf = Minz_Configuration::get('system'); $username = ''; diff --git a/app/Models/DatabaseDAO.php b/app/Models/DatabaseDAO.php index 2e0ee25a0..9d762a615 100644 --- a/app/Models/DatabaseDAO.php +++ b/app/Models/DatabaseDAO.php @@ -20,11 +20,10 @@ class FreshRSS_DatabaseDAO extends Minz_ModelPdo { try { $sql = sprintf($SQL_CREATE_DB, empty($db['base']) ? '' : $db['base']); - return $this->pdo->exec($sql) !== false; + return $this->pdo->exec($sql) === false ? 'Error during CREATE DATABASE' : ''; } catch (Exception $e) { - $_SESSION['bd_error'] = $e->getMessage(); - syslog(LOG_DEBUG, __method__ . ' warning: ' . $e->getMessage()); - return false; + syslog(LOG_DEBUG, __method__ . ' notice: ' . $e->getMessage()); + return $e->getMessage(); } } @@ -33,11 +32,10 @@ class FreshRSS_DatabaseDAO extends Minz_ModelPdo { $sql = 'SELECT 1'; $stm = $this->pdo->query($sql); $res = $stm->fetchAll(PDO::FETCH_COLUMN, 0); - return $res != false; + return $res == false ? 'Error during SQL connection test!' : ''; } catch (Exception $e) { - $_SESSION['bd_error'] = $e->getMessage(); syslog(LOG_DEBUG, __method__ . ' warning: ' . $e->getMessage()); - return false; + return $e->getMessage(); } } diff --git a/app/actualize_script.php b/app/actualize_script.php index ffa6baedb..18b7f99f8 100755 --- a/app/actualize_script.php +++ b/app/actualize_script.php @@ -78,8 +78,10 @@ foreach ($users as $user) { } } - Minz_Session::_param('currentUser', '_'); - Minz_Session::_param('loginOk'); + Minz_Session::_params([ + 'currentUser' => '_', + 'loginOk' => false, + ]); gc_collect_cycles(); } diff --git a/app/install.php b/app/install.php index bed2a8383..5093b45a6 100644 --- a/app/install.php +++ b/app/install.php @@ -163,9 +163,14 @@ function saveStep2() { $ok = false; try { - Minz_Session::_param('currentUser', $config_array['default_user']); - $ok = initDb(); - Minz_Session::_param('currentUser'); + $_SESSION['currentUser'] = $config_array['default_user']; + $error = initDb(); + unset($_SESSION['currentUser']); + if ($error != '') { + $_SESSION['bd_error'] = $error; + } else { + $ok = true; + } } catch (Exception $ex) { $_SESSION['bd_error'] = $ex->getMessage(); $ok = false; diff --git a/cli/_cli.php b/cli/_cli.php index 4e1188428..68a0f8731 100644 --- a/cli/_cli.php +++ b/cli/_cli.php @@ -10,6 +10,7 @@ require(__DIR__ . '/../constants.php'); require(LIB_PATH . '/lib_rss.php'); //Includes class autoloader require(LIB_PATH . '/lib_install.php'); +Minz_Session::init('FreshRSS', true); Minz_Configuration::register('system', DATA_PATH . '/config.php', FRESHRSS_PATH . '/config.default.php'); diff --git a/cli/do-install.php b/cli/do-install.php index 6535cc2e6..3fa9cd63e 100755 --- a/cli/do-install.php +++ b/cli/do-install.php @@ -93,10 +93,14 @@ Minz_Session::_param('currentUser', '_'); //Default user $ok = false; try { - $ok = initDb(); + $error = initDb(); + if ($error != '') { + $_SESSION['bd_error'] = $error; + } else { + $ok = true; + } } catch (Exception $ex) { $_SESSION['bd_error'] = $ex->getMessage(); - $ok = false; } if (!$ok) { diff --git a/lib/Minz/Error.php b/lib/Minz/Error.php index 3e4a3e8f3..2fb5a4d5d 100644 --- a/lib/Minz/Error.php +++ b/lib/Minz/Error.php @@ -24,8 +24,10 @@ class Minz_Error { $error_filename = APP_PATH . '/Controllers/errorController.php'; if (file_exists ($error_filename)) { - Minz_Session::_param('error_code', $code); - Minz_Session::_param('error_logs', $logs); + Minz_Session::_params([ + 'error_code' => $code, + 'error_logs' => $logs, + ]); Minz_Request::forward (array ( 'c' => 'error' diff --git a/lib/Minz/Request.php b/lib/Minz/Request.php index fc333244f..271bcf738 100644 --- a/lib/Minz/Request.php +++ b/lib/Minz/Request.php @@ -269,13 +269,14 @@ class Minz_Request { } private static function setNotification($type, $content) { - //TODO: Will need to ensure non-concurrency when landing https://github.com/FreshRSS/FreshRSS/pull/3096 + Minz_Session::lock(); $requests = Minz_Session::param('requests', []); $requests[self::requestId()] = [ 'time' => time(), 'notification' => [ 'type' => $type, 'content' => $content ], ]; Minz_Session::_param('requests', $requests); + Minz_Session::unlock(); } public static function setGoodNotification($content) { @@ -288,7 +289,7 @@ class Minz_Request { public static function getNotification() { $notif = null; - //TODO: Will need to ensure non-concurrency when landing https://github.com/FreshRSS/FreshRSS/pull/3096 + Minz_Session::lock(); $requests = Minz_Session::param('requests'); if ($requests) { //Delete abandonned notifications @@ -301,6 +302,7 @@ class Minz_Request { } Minz_Session::_param('requests', $requests); } + Minz_Session::unlock(); return $notif; } diff --git a/lib/Minz/Session.php b/lib/Minz/Session.php index 97b15c4d0..cb0e5336e 100644 --- a/lib/Minz/Session.php +++ b/lib/Minz/Session.php @@ -4,18 +4,51 @@ * La classe Session gère la session utilisateur */ class Minz_Session { + private static $volatile = false; + + /** + * For mutual exclusion. + */ + private static $locked = false; + + public static function lock() { + if (!self::$volatile && !self::$locked && session_start()) { + self::$locked = true; + } + return self::$locked; + } + + public static function unlock() { + if (!self::$volatile && session_write_close()) { + self::$locked = false; + } + return self::$locked; + } + /** * Initialise la session, avec un nom * Le nom de session est utilisé comme nom pour les cookies et les URLs(i.e. PHPSESSID). * Il ne doit contenir que des caractères alphanumériques ; il doit être court et descriptif + * If the volatile parameter is true, then no cookie and not session storage are used. + * Volatile is especially useful for API calls without cookie / Web session. */ - public static function init($name) { + public static function init($name, $volatile = false) { + self::$volatile = $volatile; + if (self::$volatile) { + $_SESSION = []; + return; + } + $cookie = session_get_cookie_params(); self::keepCookie($cookie['lifetime']); // démarre la session session_name($name); + //When using cookies (default value), session_stars() sends HTTP headers session_start(); + session_write_close(); + //Use cookie only the first time the session is started to avoid resending HTTP headers + ini_set('session.use_cookies', '0'); } @@ -35,13 +68,34 @@ class Minz_Session { * @param $v la valeur à attribuer, false pour supprimer */ public static function _param($p, $v = false) { + if (!self::$volatile && !self::$locked) { + session_start(); + } if ($v === false) { unset($_SESSION[$p]); } else { $_SESSION[$p] = $v; } + if (!self::$volatile && !self::$locked) { + session_write_close(); + } } + public static function _params($keyValues) { + if (!self::$volatile && !self::$locked) { + session_start(); + } + foreach ($keyValues as $k => $v) { + if ($v === false) { + unset($_SESSION[$k]); + } else { + $_SESSION[$k] = $v; + } + } + if (!self::$volatile && !self::$locked) { + session_write_close(); + } + } /** * Permet d'effacer une session @@ -50,7 +104,9 @@ class Minz_Session { public static function unset_session($force = false) { $language = self::param('language'); - session_destroy(); + if (!self::$volatile) { + session_destroy(); + } $_SESSION = array(); if (!$force) { diff --git a/p/api/fever.php b/p/api/fever.php index 909e29d32..64e6dfd65 100644 --- a/p/api/fever.php +++ b/p/api/fever.php @@ -25,9 +25,7 @@ if (!FreshRSS_Context::$system_conf->api_enabled) { die('Service Unavailable!'); } -ini_set('session.use_cookies', '0'); -register_shutdown_function('session_destroy'); -Minz_Session::init('FreshRSS'); +Minz_Session::init('FreshRSS', true); // ================================================================================================ // <Debug> diff --git a/p/api/greader.php b/p/api/greader.php index 027662cc1..e93585275 100644 --- a/p/api/greader.php +++ b/p/api/greader.php @@ -935,9 +935,7 @@ if (!FreshRSS_Context::$system_conf->api_enabled) { checkCompatibility(); } -ini_set('session.use_cookies', '0'); -register_shutdown_function('session_destroy'); -Minz_Session::init('FreshRSS'); +Minz_Session::init('FreshRSS', true); $user = $pathInfos[1] === 'accounts' ? '' : authorizationToUser(); FreshRSS_Context::$user_conf = null; diff --git a/p/i/index.php b/p/i/index.php index 35e53ec5e..d1d56bbb9 100755 --- a/p/i/index.php +++ b/p/i/index.php @@ -27,8 +27,6 @@ if (file_exists(DATA_PATH . '/do-install.txt')) { require(APP_PATH . '/install.php'); } else { session_cache_limiter(''); - Minz_Session::init('FreshRSS'); - Minz_Session::_param('keepAlive', 1); //To prevent the PHP session from expiring if (!file_exists(DATA_PATH . '/no-cache.txt')) { require(LIB_PATH . '/http-conditional.php'); @@ -38,6 +36,8 @@ if (file_exists(DATA_PATH . '/do-install.txt')) { @filemtime(join_path(DATA_PATH, 'config.php')) ); if (httpConditional($dateLastModification, 0, 0, false, PHP_COMPRESSION, true)) { + Minz_Session::init('FreshRSS'); + Minz_Session::_param('keepAlive', 1); //To prevent the PHP session from expiring exit(); //No need to send anything } } @@ -65,6 +65,7 @@ if (file_exists(DATA_PATH . '/do-install.txt')) { if ($result === true) { $front_controller = new FreshRSS(); $front_controller->init(); + Minz_Session::_param('keepAlive', 1); //To prevent the PHP session from expiring $front_controller->run(); } else { $error = $result; |
