diff --git a/lib/session_db.php b/lib/session_db.php index 81d0398cb1..10eeea149d 100644 --- a/lib/session_db.php +++ b/lib/session_db.php @@ -28,6 +28,8 @@ class Hm_DB_Session extends Hm_PHP_Session { */ private $lock_timeout = 10; + private $version = 1; + /** * Create a new session * @return boolean|integer|array @@ -77,17 +79,12 @@ public function start_new($request) { */ public function start_existing($key) { $this->session_key = $key; - if (!$this->acquire_lock($key)) { - Hm_Debug::add('DB SESSION: Failed to acquire lock'); - return; - } $data = $this->get_session_data($key); if (is_array($data)) { Hm_Debug::add('LOGGED IN'); $this->active = true; $this->data = $data; } - $this->release_lock($key); } /** @@ -96,9 +93,16 @@ public function start_existing($key) { * @return mixed array results or false on failure */ public function get_session_data($key) { - $results = Hm_DB::execute($this->dbh, 'select data from hm_user_session where hm_id=?', [$key]); - if (is_array($results) && array_key_exists('data', $results)) { - return $this->plaintext($results['data']); + $results = Hm_DB::execute($this->dbh, 'select data, hm_version from hm_user_session where hm_id=?', [$key]); + if (is_array($results)) { + if (array_key_exists('data', $results) && array_key_exists('hm_version', $results)) { + $this->version = $results['hm_version']; + $data = $results['data']; + if (is_resource($data)) { + $data = stream_get_contents($data); + } + return $this->plaintext($data); + } } Hm_Debug::add('DB SESSION failed to read session data'); return false; @@ -141,9 +145,25 @@ public function upsert($type) { $res = false; $params = [':key' => $this->session_key, ':data' => $this->ciphertext($this->data)]; if ($type == 'update') { - $res = Hm_DB::execute($this->dbh, 'update hm_user_session set data=:data where hm_id=:key', $params); + if ($this->version === null) { + Hm_Debug::add('DB SESSION: Missing hm_version for session key ' . $this->session_key); + return false; + } + $params[':hm_version'] = $this->version; + if (!$this->acquire_lock($this->session_key)) { + Hm_Debug::add('Failed to acquire lock on session'); + return false; + } + $res = Hm_DB::execute($this->dbh, 'update hm_user_session set data=:data, hm_version=hm_version+1 where hm_id=:key and hm_version=:hm_version', $params); + if ($res === 0) { + Hm_Debug::add('Optimistic Locking: hm_version mismatch, session data not updated'); + $this->release_lock($this->session_key); + return false; + } + $this->release_lock($this->session_key); } elseif ($type == 'insert') { - $res = Hm_DB::execute($this->dbh, 'insert into hm_user_session values(:key, :data, current_date)', $params); + $res = Hm_DB::execute($this->dbh, 'insert into hm_user_session (hm_id, data, hm_version, date) values(:key, :data, 1, current_date)', $params); + Hm_Debug::add('Session insert params: ' . json_encode($params)); } if (!$res) { Hm_Debug::add('DB SESSION failed to write session data'); @@ -192,44 +212,55 @@ public function db_start($request) { * @return bool true if lock acquired, false otherwise */ private function acquire_lock($key) { - $lock_name = 'session_lock_' . substr(hash('sha256', $key), 0, 51); + $lock_name = 'session_lock_' . substr(hash('sha256', $key), 0, 51); + // Polling parameters + $max_attempts = 5; + $retry_interval = 500000; + $attempts = 0; $query = ''; $params = []; - - switch ($this->db_driver) { - case 'mysql': - $query = 'SELECT GET_LOCK(:lock_name, :timeout)'; - $params = [':lock_name' => $lock_name, ':timeout' => $this->lock_timeout]; - break; - - case 'pgsql': - $query = 'SELECT pg_try_advisory_lock(:hash_key)'; - $params = [':hash_key' => crc32($lock_name)]; - break; - - case 'sqlite': - $query = 'UPDATE hm_user_session SET lock=1 WHERE hm_id=? AND lock=0'; - $params = [$key]; - break; - - default: - Hm_Debug::add('DB SESSION: Unsupported db_driver for locking: ' . $this->db_driver); - return false; - } - - $result = Hm_DB::execute($this->dbh, $query, $params); - if ($this->db_driver == 'mysql') { - return isset($result['GET_LOCK(?, ?)']) && $result['GET_LOCK(?, ?)'] == 1; - } - if ($this->db_driver == 'pgsql') { - return isset($result['pg_try_advisory_lock']) && $result['pg_try_advisory_lock'] === true; - } - - if ($this->db_driver == 'sqlite') { - return isset($result[0]) && $result[0] == 1; + while ($attempts < $max_attempts) { + switch ($this->db_driver) { + case 'mysql': + $query = 'SELECT GET_LOCK(:lock_name, :timeout)'; + $params = [':lock_name' => $lock_name, ':timeout' => $this->lock_timeout]; + break; + case 'pgsql': + $query = 'SELECT pg_try_advisory_lock(:hash_key)'; + $params = [':hash_key' => crc32($lock_name)]; + break; + case 'sqlite': + $query = 'UPDATE hm_user_session SET lock=1 WHERE hm_id=? AND lock=0'; + $params = [$key]; + break; + default: + Hm_Debug::add('DB SESSION: Unsupported db_driver for locking: ' . $this->db_driver); + return false; + } + $result = Hm_DB::execute($this->dbh, $query, $params); + if ($this->db_driver == 'mysql') { + if (isset($result['GET_LOCK(?, ?)']) && $result['GET_LOCK(?, ?)'] == 1) { + return true; + } + } + if ($this->db_driver == 'pgsql') { + if (isset($result['pg_try_advisory_lock']) && $result['pg_try_advisory_lock'] === true) { + return true; + } + } + if ($this->db_driver == 'sqlite') { + if (isset($result[0]) && $result[0] == 1) { + return true; + } + } + $attempts++; + if ($attempts < $max_attempts) { + usleep($retry_interval); + } } + Hm_Debug::add('DB SESSION: Failed to acquire lock after ' . $max_attempts . ' attempts.'); return false; - } + } /** * Release a lock for the session (unified for all DB types) @@ -239,24 +270,20 @@ private function acquire_lock($key) { private function release_lock($key) { $query = ''; $params = []; - $lock_name = "session_lock_" . substr(hash('sha256', $key), 0, 51); switch ($this->db_driver) { case 'mysql': $query = 'SELECT RELEASE_LOCK(:lock_name)'; $params = [':lock_name' => $lock_name]; break; - case 'pgsql': $query = 'SELECT pg_advisory_unlock(:hash_key)'; $params = [':hash_key' => crc32($lock_name)]; break; - case 'sqlite': $query = 'UPDATE hm_user_session SET lock=0 WHERE hm_id=?'; $params = [$key]; break; - default: Hm_Debug::add('DB SESSION: Unsupported db_driver for unlocking: ' . $this->db_driver); return false; @@ -268,7 +295,6 @@ private function release_lock($key) { if ($this->db_driver == 'pgsql') { return isset($result['pg_advisory_unlock']) && $result['pg_advisory_unlock'] === true; } - if ($this->db_driver == 'sqlite') { return isset($result[0]) && $result[0] == 1; } diff --git a/scripts/setup_database.php b/scripts/setup_database.php index 2ffceb02bf..19ac21c7b0 100755 --- a/scripts/setup_database.php +++ b/scripts/setup_database.php @@ -132,19 +132,22 @@ function add_missing_columns($conn, $table_name, $required_columns, $db_driver) $tables = [ 'hm_user_session' => [ 'mysql' => [ - 'hm_id' => 'varchar(255) PRIMARY KEY', - 'data' => 'longblob', - 'date' => 'timestamp', + 'hm_id' => 'varchar(255) PRIMARY KEY', + 'data' => 'longblob', + 'hm_version' => 'INT DEFAULT 1', + 'date' => 'timestamp', ], 'sqlite' => [ 'hm_id' => 'varchar(255) PRIMARY KEY', 'data' => 'longblob', 'lock' => 'INTEGER DEFAULT 0', + 'hm_version' => 'INT DEFAULT 1', 'date' => 'timestamp', ], 'pgsql' => [ 'hm_id' => 'varchar(255) PRIMARY KEY', 'data' => 'text', + 'hm_version' => 'INT DEFAULT 1', 'date' => 'timestamp', ], ], diff --git a/tests/phpunit/data/schema.sql b/tests/phpunit/data/schema.sql index 75ad172e7e..e756a18797 100644 --- a/tests/phpunit/data/schema.sql +++ b/tests/phpunit/data/schema.sql @@ -1,6 +1,12 @@ +DROP TABLE IF EXISTS hm_user; + +DROP TABLE IF EXISTS hm_user_session; + +DROP TABLE IF EXISTS hm_user_settings; + CREATE TABLE IF NOT EXISTS hm_user (username varchar(255), hash varchar(255), primary key (username)); -CREATE TABLE IF NOT EXISTS hm_user_session (hm_id varchar(255), data longblob, date timestamp, primary key (hm_id)); +CREATE TABLE IF NOT EXISTS hm_user_session (hm_id varchar(255), data longblob, date timestamp, hm_version int default 1, primary key (hm_id)); CREATE TABLE IF NOT EXISTS hm_user_settings(username varchar(255), settings longblob, primary key (username)); diff --git a/tests/phpunit/data/schema_postgres.sql b/tests/phpunit/data/schema_postgres.sql new file mode 100644 index 0000000000..8870a97645 --- /dev/null +++ b/tests/phpunit/data/schema_postgres.sql @@ -0,0 +1,12 @@ + +DROP TABLE IF EXISTS hm_user; + +DROP TABLE IF EXISTS hm_user_session; + +DROP TABLE IF EXISTS hm_user_settings; + +CREATE TABLE IF NOT EXISTS hm_user (username varchar(255), hash varchar(255), primary key (username)); + +CREATE TABLE IF NOT EXISTS hm_user_session (hm_id varchar(255), data BYTEA, date timestamp, hm_version int default 1, primary key (hm_id)); + +CREATE TABLE IF NOT EXISTS hm_user_settings(username varchar(255), settings BYTEA, primary key (username)); diff --git a/tests/phpunit/data/schema_sqlite.sql b/tests/phpunit/data/schema_sqlite.sql new file mode 100644 index 0000000000..0b6ea73dd8 --- /dev/null +++ b/tests/phpunit/data/schema_sqlite.sql @@ -0,0 +1,11 @@ +DROP TABLE IF EXISTS hm_user; + +DROP TABLE IF EXISTS hm_user_session; + +DROP TABLE IF EXISTS hm_user_settings; + +CREATE TABLE IF NOT EXISTS hm_user (username varchar(255), hash varchar(255), primary key (username)); + +CREATE TABLE IF NOT EXISTS hm_user_session (hm_id varchar(255), data longblob, date timestamp, lock int default 0, hm_version int default 1, primary key (hm_id)); + +CREATE TABLE IF NOT EXISTS hm_user_settings(username varchar(255), settings longblob, primary key (username)); diff --git a/tests/phpunit/run.sh b/tests/phpunit/run.sh index b2436a4989..502865c1e6 100755 --- a/tests/phpunit/run.sh +++ b/tests/phpunit/run.sh @@ -20,7 +20,7 @@ if [ "$DB" = "sqlite" ]; then export DB_CONNECTION_TYPE=socket export DB_SOCKET=${FILE} - cat ${SCRIPT_DIR}/data/schema.sql | sqlite3 ${FILE} + cat ${SCRIPT_DIR}/data/schema_sqlite.sql | sqlite3 ${FILE} cat ${SCRIPT_DIR}/data/seed.sql | sqlite3 ${FILE} elif [ "$DB" = "mysql" ]; then @@ -33,7 +33,7 @@ elif [ "$DB" = "mysql" ]; then elif [ "$DB" = "postgres" ]; then export DB_DRIVER=pgsql # Load schema.sql - PGPASSWORD=cypht_test psql -h 127.0.0.1 -U cypht_test -d cypht_test -f ${SCRIPT_DIR}/data/schema.sql + PGPASSWORD=cypht_test psql -h 127.0.0.1 -U cypht_test -d cypht_test -f ${SCRIPT_DIR}/data/schema_postgres.sql # Load seed.sql PGPASSWORD=cypht_test psql -h 127.0.0.1 -U cypht_test -d cypht_test -f ${SCRIPT_DIR}/data/seed_postgres.sql