[ci skip] Fix #5703

5 files changed, 63 insertions, 83 deletions

diff --git a/system/libraries/Session/Session_driver.php b/system/libraries/Session/Session_driver.php
index 14ebdb09f..dbc833739 100644
--- a/system/libraries/Session/Session_driver.php
+++ b/system/libraries/Session/Session_driver.php
@@ -184,25 +184,4 @@ abstract class CI_Session_driver implements SessionHandlerInterface {
 
 		return TRUE;
 	}
-
-	// ------------------------------------------------------------------------
-
-	/**
-	 * Fail
-	 *
-	 * Drivers other than the 'files' one don't (need to) use the
-	 * session.save_path INI setting, but that leads to confusing
-	 * error messages emitted by PHP when open() or write() fail,
-	 * as the message contains session.save_path ...
-	 * To work around the problem, the drivers will call this method
-	 * so that the INI is set just in time for the error message to
-	 * be properly generated.
-	 *
-	 * @return	mixed
-	 */
-	protected function _fail()
-	{
-		ini_set('session.save_path', config_item('sess_save_path'));
-		return $this->_failure;
-	}
 }
diff --git a/system/libraries/Session/drivers/Session_database_driver.php b/system/libraries/Session/drivers/Session_database_driver.php
index 734fe624f..89afe3455 100644
--- a/system/libraries/Session/drivers/Session_database_driver.php
+++ b/system/libraries/Session/drivers/Session_database_driver.php
@@ -130,7 +130,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 	{
 		if (empty($this->_db->conn_id) && ! $this->_db->db_connect())
 		{
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		$this->php5_validate_id();
@@ -150,48 +150,47 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 	 */
 	public function read($session_id)
 	{
-		if ($this->_get_lock($session_id) !== FALSE)
+		if ($this->_get_lock($session_id) === FALSE)
 		{
-			// Prevent previous QB calls from messing with our queries
-			$this->_db->reset_query();
-
-			// Needed by write() to detect session_regenerate_id() calls
-			$this->_session_id = $session_id;
+			return $this->_failure;
+		}
 
-			$this->_db
-				->select('data')
-				->from($this->_config['save_path'])
-				->where('id', $session_id);
+		// Prevent previous QB calls from messing with our queries
+		$this->_db->reset_query();
 
-			if ($this->_config['match_ip'])
-			{
-				$this->_db->where('ip_address', $_SERVER['REMOTE_ADDR']);
-			}
+		// Needed by write() to detect session_regenerate_id() calls
+		$this->_session_id = $session_id;
 
-			if ( ! ($result = $this->_db->get()) OR ($result = $result->row()) === NULL)
-			{
-				// PHP7 will reuse the same SessionHandler object after
-				// ID regeneration, so we need to explicitly set this to
-				// FALSE instead of relying on the default ...
-				$this->_row_exists = FALSE;
-				$this->_fingerprint = md5('');
-				return '';
-			}
+		$this->_db
+			->select('data')
+			->from($this->_config['save_path'])
+			->where('id', $session_id);
 
-			// PostgreSQL's variant of a BLOB datatype is Bytea, which is a
-			// PITA to work with, so we use base64-encoded data in a TEXT
-			// field instead.
-			$result = ($this->_platform === 'postgre')
-				? base64_decode(rtrim($result->data))
-				: $result->data;
+		if ($this->_config['match_ip'])
+		{
+			$this->_db->where('ip_address', $_SERVER['REMOTE_ADDR']);
+		}
 
-			$this->_fingerprint = md5($result);
-			$this->_row_exists = TRUE;
-			return $result;
+		if ( ! ($result = $this->_db->get()) OR ($result = $result->row()) === NULL)
+		{
+			// PHP7 will reuse the same SessionHandler object after
+			// ID regeneration, so we need to explicitly set this to
+			// FALSE instead of relying on the default ...
+			$this->_row_exists = FALSE;
+			$this->_fingerprint = md5('');
+			return '';
 		}
 
-		$this->_fingerprint = md5('');
-		return '';
+		// PostgreSQL's variant of a BLOB datatype is Bytea, which is a
+		// PITA to work with, so we use base64-encoded data in a TEXT
+		// field instead.
+		$result = ($this->_platform === 'postgre')
+			? base64_decode(rtrim($result->data))
+			: $result->data;
+
+		$this->_fingerprint = md5($result);
+		$this->_row_exists = TRUE;
+		return $result;
 	}
 
 	// ------------------------------------------------------------------------
@@ -215,7 +214,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 		{
 			if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
 			{
-				return $this->_fail();
+				return $this->_failure;
 			}
 
 			$this->_row_exists = FALSE;
@@ -223,7 +222,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 		}
 		elseif ($this->_lock === FALSE)
 		{
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		if ($this->_row_exists === FALSE)
@@ -242,7 +241,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 				return $this->_success;
 			}
 
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		$this->_db->where('id', $session_id);
@@ -265,7 +264,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -280,7 +279,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 	public function close()
 	{
 		return ($this->_lock && ! $this->_release_lock())
-			? $this->_fail()
+			? $this->_failure
 			: $this->_success;
 	}
 
@@ -309,7 +308,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 
 			if ( ! $this->_db->delete($this->_config['save_path']))
 			{
-				return $this->_fail();
+				return $this->_failure;
 			}
 		}
 
@@ -319,7 +318,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -339,7 +338,7 @@ class CI_Session_database_driver extends CI_Session_driver implements SessionHan
 
 		return ($this->_db->delete($this->_config['save_path'], 'timestamp < '.(time() - $maxlifetime)))
 			? $this->_success
-			: $this->_fail();
+			: $this->_failure;
 	}
 
 	// --------------------------------------------------------------------
diff --git a/system/libraries/Session/drivers/Session_memcached_driver.php b/system/libraries/Session/drivers/Session_memcached_driver.php
index ab54f029f..854adf821 100644
--- a/system/libraries/Session/drivers/Session_memcached_driver.php
+++ b/system/libraries/Session/drivers/Session_memcached_driver.php
@@ -117,7 +117,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 		{
 			$this->_memcached = NULL;
 			log_message('error', 'Session: Invalid Memcached save path format: '.$this->_config['save_path']);
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		foreach ($matches as $match)
@@ -142,7 +142,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 		if (empty($server_list))
 		{
 			log_message('error', 'Session: Memcached server pool is empty.');
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		$this->php5_validate_id();
@@ -172,7 +172,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 			return $session_data;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -190,14 +190,14 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 	{
 		if ( ! isset($this->_memcached, $this->_lock_key))
 		{
-			return $this->_fail();
+			return $this->_failure;
 		}
 		// Was the ID regenerated?
 		elseif ($session_id !== $this->_session_id)
 		{
 			if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
 			{
-				return $this->_fail();
+				return $this->_failure;
 			}
 
 			$this->_fingerprint = md5('');
@@ -215,7 +215,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 				return $this->_success;
 			}
 
-			return $this->_fail();
+			return $this->_failure;
 		}
 		elseif (
 			$this->_memcached->touch($key, $this->_config['expiration'])
@@ -225,7 +225,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -244,14 +244,14 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 			$this->_release_lock();
 			if ( ! $this->_memcached->quit())
 			{
-				return $this->_fail();
+				return $this->_failure;
 			}
 
 			$this->_memcached = NULL;
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -273,7 +273,7 @@ class CI_Session_memcached_driver extends CI_Session_driver implements SessionHa
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
diff --git a/system/libraries/Session/drivers/Session_redis_driver.php b/system/libraries/Session/drivers/Session_redis_driver.php
index 25bf80706..d7777cdb3 100644
--- a/system/libraries/Session/drivers/Session_redis_driver.php
+++ b/system/libraries/Session/drivers/Session_redis_driver.php
@@ -131,7 +131,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 	{
 		if (empty($this->_config['save_path']))
 		{
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		$redis = new Redis();
@@ -155,7 +155,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 
 		$this->php5_validate_id();
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -185,7 +185,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 			return $session_data;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -203,14 +203,14 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 	{
 		if ( ! isset($this->_redis, $this->_lock_key))
 		{
-			return $this->_fail();
+			return $this->_failure;
 		}
 		// Was the ID regenerated?
 		elseif ($session_id !== $this->_session_id)
 		{
 			if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
 			{
-				return $this->_fail();
+				return $this->_failure;
 			}
 
 			$this->_key_exists = FALSE;
@@ -227,12 +227,12 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 				return $this->_success;
 			}
 
-			return $this->_fail();
+			return $this->_failure;
 		}
 
 		return ($this->_redis->setTimeout($this->_key_prefix.$session_id, $this->_config['expiration']))
 			? $this->_success
-			: $this->_fail();
+			: $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
@@ -254,7 +254,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 					$this->_release_lock();
 					if ($this->_redis->close() === FALSE)
 					{
-						return $this->_fail();
+						return $this->_failure;
 					}
 				}
 			}
@@ -293,7 +293,7 @@ class CI_Session_redis_driver extends CI_Session_driver implements SessionHandle
 			return $this->_success;
 		}
 
-		return $this->_fail();
+		return $this->_failure;
 	}
 
 	// ------------------------------------------------------------------------
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 6275ad7bc..0a05d6906 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -15,6 +15,8 @@ Bug fixes for 3.1.11
 -  Fixed a bug (#5692) - :doc:`Database Forge <database/forge>` didn't handle column nullability with the 'oci8', 'pdo/oci' drivers.
 -  Fixed a bug (#5701) - :doc:`Database <database/index>` driver 'pdo/pgsql' produced incorrect DSNs when constructing from a configuration array.
 -  Fixed a bug (#5708) - :doc:`Session Library <libraries/session>` 'redis' driver too often failed with locking-related errors that could've been avoided.
+-  Fixed a bug (#5703) - :doc:`Session Library <libraries/session>` triggered an ``E_WARNING`` message about changing ``session.save_path`` during an active session when it fails to obtain a lock.
+-  Fixed a bug where :doc:`Session Library <libraries/session>` 'database' driver didn't trigger a failure if it can't obtain a lock.
 
 Version 3.1.10
 ==============