At this point i am interested to see other techniques on handling session timeouts.
What are some good improvements on this script to detect when a session is no longer valid?
Important definitions:
session.gc_maxlifetime - (default 1440 seconds) defines how long an unused PHP session will be kept alive. For example: A user logs in, browses through your application or web site, for hours, for days. No problem. As long as the time between his clicks never exceed 1440 seconds. It's a timeout value.
session.cookie_lifetime - *This value (default 0, which means until the browser's next restart) defines how long (in seconds) a session cookie will live. Sounds similar to session.gc_maxlifetime, but it's a completely different approach. This value indirectly defines the "absolute" maximum lifetime of a session, whether the user is active or not. If this value is set to 1*60*60, every session ends after an hour.*
Problems to address:
- Session timeout
- Cookie timeout
- Browser deleted cookies / cleared history
SESSION_handler.php
<?php
class SESSION_handler{
public static $gc_maxlifetime;
public static $cookie_lifetime;
function __construct(){
self::$gc_maxlifetime = ini_get('session.gc_maxlifetime');
self::$cookie_lifetime = ini_get('session.cookie_lifetime');
if(session_status() == PHP_SESSION_NONE) {
session_start();
}
}
public function get($session_key){
if(session_status() == PHP_SESSION_NONE) {
if(!empty($_SESSION[$session_key])){
$_SESSION['time_at_last_session'] = time();
return $_SESSION[$session_key];
}
}
return undefined;
}
public function set($session_key, $session_value){
if(session_status() == PHP_SESSION_NONE) {
$_SESSION[$session_key] = $session_value;
$_SESSION['time_at_last_session'] = time();
return $_SESSION[$session_key];
}
return undefined;
}
public function session_expired(){
if(
time() - ((!empty($_SESSION['time_at_last_session']))? $_SESSION['time_at_last_session'] : 0) >= self::$gc_maxlifetime ||
session_status() == PHP_SESSION_NONE ||
!isset($_COOKIE[session_name()])
){
return true;
}
return false;
}
}
?>
is_session_expired.php
<?php
$SESSION = new SESSION_handler();
if($SESSION->session_expired()){
echo "true";
}else{
echo "false";
}
?>
AJAX
<script type="text/javascript">
$(document).ready(function(){
(function(){
tmp_ = arguments.callee;
window.setTimeout(function(){
$.ajax({
url:'/php/is_session_expired.php',
success: function(session_expired){
if(session_expired == "true"){
alert("session has expired");
}else{
console.log('session has not expired');
}
}
});
});
})();
})
</script>
1 Answer 1
I don't believe the code works as expected.
One of us for sure made mistake with session_status() == PHP_SESSION_NONE
. If is me, comment so I can fix the answer.
Code style
You will have better readability, if you indent everything inside the class.
You need to work on if
statement in SESSION_handler::session_expired()
. I can not read it at all. I personally would do it on single row, but then many people will not like it at all.
Constructor
self::$cookie_lifetime
- I do not see where you need this?
You initializing the static fields every time.
self::$gc_maxlifetime = ini_get('session.gc_maxlifetime');
self::$cookie_lifetime = ini_get('session.cookie_lifetime');
I would suggest something like this:
if (self::$gc_maxlifetime = -1){
self::$gc_maxlifetime = ini_get('session.gc_maxlifetime');
self::$cookie_lifetime = ini_get('session.cookie_lifetime');
}
You also might decide to remove them and use ini_get
directly. Or you can do static method SESSION_handler::gc_maxlifetime()
.
Here you create session as well:
if(session_status() == PHP_SESSION_NONE) {
session_start();
}
Do you really need to check session_status()
? My experience is you can use session_start()
directly. If there is a session created, statement will do no harm.
I believe you need to initialize $_SESSION['time_at_last_session']
, but only if is not set already.
Something like:
if (!isset($_SESSION['time_at_last_session']))
$_SESSION['time_at_last_session'] = time();
SESSION_handler::get() and SESSION_handler::set()
Why you have this:
if(session_status() == PHP_SESSION_NONE) {
// ...
}
return undefined;
You probably want to check if everything with the session is correct? to do so, use:
if(session_status() != PHP_SESSION_NONE) {
// ...
}
You return undefined;
. This is first time I see this line in PHP. Just remove the if
statement. Session is already made in constructor. If is not undefined
will not help at all.
Actually, in constructor, you can save
the result of session_start();
into some class property and use it instead of session_status()
.
SESSION_handler::session_expired()
should be something like this?
public function session_expired(){
$time = isset($_SESSION['time_at_last_session']) ?
$_SESSION['time_at_last_session'] : 0;
// same, but ugly:
//$time = (int) @$_SESSION['time_at_last_session'];
if (session_status() == PHP_SESSION_NONE)
return true; // session problem
if (time() - $time > self::$gc_maxlifetime)
return true; // session expired
// fix $_COOKIE[session_name() thing
return false; // session is NOT expired
}
is_session_expired.php
All OK if you fix the constructor.
$this->time_at_last_session
initialized? I do not see where? \$\endgroup\$