PHP Session handling class

Question

I've written a custom PHP session class for handling sessions across the web app. Please review the code and point out mistakes and suggest better handling techniques.

require_once('config.php');

class Sessions {

protected $sessionID;

public function __construct(){
    if( !isset($_SESSION) ){
        $this->init_session();
    }
    //session_start();
    //$this->sessionID = session_id();
}

public function init_session(){
    session_start();
}

public function set_session_id(){
    //$this->start_session();
    $this->sessionID = session_id();
}

public function get_session_id(){
    return $this->sessionID;
}

public function session_exist( $session_name ){
    if( isset($_SESSION[$session_name]) ){
        return true;
    }
    else{
        return false;
    }
}

public function create_session( $session_name , $is_array = false ){
    if( !isset($_SESSION[$session_name])  ){
        if( $is_array == true ){
            $_SESSION[$session_name] = array();
        }
        else{
            $_SESSION[$session_name] = '';
        }
    }
}

public function insert_value( $session_name , array $data ){
    if( is_array($_SESSION[$session_name]) ){
        array_push( $_SESSION[$session_name], $data );
    }
}

public function display_session( $session_name ){
    echo '<pre>';print_r($_SESSION[$session_name]);echo '</pre>';
}

public function remove_session( $session_name = '' ){
    if( !empty($session_name) ){
        unset( $_SESSION[$session_name] );
    }
    else{
        unset($_SESSION);
        //session_unset();
        //session_destroy();
    }
}

public function get_session_data( $session_name ){
    return $_SESSION[$session_name];
}

public function set_session_data( $session_name , $data ){
    $_SESSION[$session_name] = $data;
}

}

Answer 1

Commented out Code

You should remove code that is commented out. If you think that you might need it in the future, think about using version control.

set_session_id()

This is only called in your commented out code. Does the user have to call it manually? If they don't, get_session_id() will return a wrong result. Maybe rewrite it like this (you don't seem to be using the field $sessionID, so you might as well get rid of it):

public function get_session_id(){
    return session_id();
}

if-else and session_exist

If you have code like this:

if( isset($_SESSION[$session_name]) ){
    return true;
}
else{
    return false;
}

you can rewrite it like this:

return isset($_SESSION[$session_name]);

Also, when you define a function like this, then use it. Instead of

if( !isset($_SESSION[$session_name])  ){

write

if(!session_exist($session_name]) ){

XSS

Using Session for XSS might be a possibility, depending on how your code is used. So in display_session I would clean the session with htmlentities to prevent XSS attacks.

Answer 2

To be honest, it looks like a mess to me and it's difficult to read. Your naming convention is also off. They don't appear to do what you (or any other programmer) would think it does by looking at the name of your methods (not to mention your variables too).

• Method session_exist() is not what it seems. It's not checking if a session exist, but it's rather checking if a session parameter exist. I would rename that to parameterExist().

• Method create_session() does not create sessions. It sets session parameters. I would call it setParameter() instead.

• Method insert_value(), what does it even do? :|

  So you're taking in a session name (?) and an array of data. Then you're checking if the session name is an array? And if it is you push the data array that's passed in to it? Is there any difference beween create_session() and insert_value()? I'm confused.

• Why does your class contain a sessionID property?

  It looks like your using it to eh, differentiate between session objects? If that's the case the object itself can serve for that. Unless you have some other intentions for it I would just get rid of it and it's setter and getter.

I could go on and on, but this should be enough to let you realize other obvious problems that are an issue with your code.

Answer 3

I'd like to make naming suggestion to your class. Since the class is called Sessions, the object will likely be called $session or similar. Thus, make the word session in the method names superfluous. Further, since we're dealing with objects and data all the time in programming, I try to be very selective when to include them, as they often time aren't needed.

Apply these changes would result in the following:

 $session->get_session_data() 

Would be become:

$session->get_session();

Also,

$session->session_init();

would become:

$session->init();