Custom Session Handler

[vivehosting.com]

Hi, I just finished my custom session handler and I was wondering what you guys thought of it. Any suggestions, criticisms, whatever are welcome .

The handler is basically encapsulated in a class and starts the session automatically. It reads/writes to any database supported by my database abstraction classes (mysql, pgsql, sqlite), automatically generates a new id on each use to prevent session fixation, and has some security options to distinguish users (browser check, referer check, ip check). So, tell me what you think.

class session
{
    var $connection;
    var $formersid;
    var $user_agent;
    var $ip_address;
    var $referring;
    
    function session($conn = NULL, $browser = '', $ip = '', $refer = '') {
        $this->connection = $conn;
        $this->user_agent = $this->connection->filter($browser);
        $this->ip_address = $this->connection->filter($ip);
        $this->referring  = $this->connection->filter($refer);
        
        session_set_save_handler(
            array(&$this, 'open'),
            array(&$this, 'close'),
            array(&$this, 'read'),
            array(&$this, 'write'),
            array(&$this, 'destroy'),
            array(&$this, 'gc')
        );
        
        $this->set();
        $this->start();
        $this->noFixation();
    }
    function set() {
        ini_set('session.name', 's');
        ini_set('session.use_cookies', 1);
        ini_set('session.use_only_cookies', 0);
        ini_set('session.gc_probability', 1);
        ini_set('session.gc_maxlifetime', 3600);
    }
    function start() {
        if(!isset($_SESSION)) {
            session_start();
        } else {
            session_destroy();
            session_start();
            $this->_msg('session', 'should not start a session outside the session object');
        }
    }
    function noFixation() {
        $this->formersid = session_id();
        session_regenerate_id();
    }
    function open($save, $session) {
        if(is_object($this->connection)) {
            return true;
        } else {
            $this->_msg('session', 'connection is not an object');
            return false;
        }
        if(is_resource($this->connection->getConn())) {
            return true;
        } else {
            $this->_msg('session', 'connection resource is invalid');
            return false;
        }
    }
    function close() {
        $this->connection->query("DELETE FROM sessions WHERE sesskey = '".$this->formersid."'", false);
        return $this->connection->disconnect();
    }
    function read($id) {
        $result = $this->connection->query("SELECT * FROM sessions WHERE sesskey = '".$this->connection->filter($id)."' AND sessvalid = 'yes'");
        $row = $result->getCurrent();
        
        if($result->getRowCount()) {
            if($this->user_agent && $this->user_agent != $row['sessagent']) {
                $this->_msg('session', 'there is a problem identifying you with the current session, please login again');
                return '';
            }
            if($this->referring && $this->referring != $row['sessrefer']) {
                $this->_msg('session', 'an error in page traversal has occurred');
                return '';
            }
            if($this->ip_address) {
                $this->ip_address = str_replace('.*', '', $this->ip_address);

                if($this->ip_address == substr($row['sessip'], 0, strlen($this->ip_address))) return $row['sessdata'];

                $this->_msg('session', 'there is a problem identifying you with the current session, please login again');
                return '';
            }

            return $row['sessdata'];
        }
        
        return '';
    }
    function write($id, $data) {
        $result = $this->connection->query("SELECT * FROM sessions WHERE sesskey = '$id' AND sessvalid = 'yes'");
        $row = $result->getCurrent();

        if($result->getRowCount()) {
            if($this->user_agent && $this->user_agent != $row['sessagent']) {
                $this->_msg('session', 'there is a problem identifying you with the current session, please login again');
                return false;
            }
            if($this->referring && $this->referring != $row['sessrefer']) {
                $this->_msg('session', 'an error in page traversal has occurred');
                return false;
            }
            if($this->ip_address) {
                $this->ip_address = str_replace('.*', '', $this->ip_address);

                if($this->ip_address == substr($row['sessip'], 0, strlen($this->ip_address))) return true;

                $this->_msg('session', 'there is a problem identifying you with the current session, please login again');
                return false;
            }
        }
        
        $result = $this->connection->query("INSERT INTO sessions (sessexp, sessvalid, sesskey, sessdata, sessagent, sessip, sessrefer) VALUES (".
                                           time().", 'yes', '".$this->connection->filter($id)."', '".$this->connection->filter($data).
                                           "', '$this->user_agent', '$this->ip_address', '$this->referring')");
        return $result->isSuccess();
    }
    function destroy($id) {
        $result = $this->connection->query("UPDATE sessions SET sessvalid = 'no' WHERE sesskey = '".$this->connection->filter($id)."'");
        return $result->isSuccess();
    }
    function gc($lifetime) {
        $result = $this->connection->query("UPDATE sessions SET sessvalid = 'no' WHERE sessvalid = 'yes' AND sessexp < ".time() - $lifetime);
        return $result->isSuccess();
    }
    function _msg($type, $msg) {
        trigger_error($type.'|'.$msg);
    }
} 

[vivehosting.com]

noone has anything bad to say? hehe.

[Widow Maker]

Okay, since no one has said nothing, here is some bad news

...
 if($this->referring && $this->referring != $row['sessrefer']) { 
                $this->_msg('session', 'an error in page traversal has occurred'); 
                return false; 
            } 
            if($this->ip_address) { 
                $this->ip_address = str_replace('.*', '', $this->ip_address); 

                if($this->ip_address == substr($row['sessip'], 0, strlen($this->ip_address))) return true; 

                $this->_msg('session', 'there is a problem identifying you with the current session, please login again'); 
                return false; 
            }
... 

You need to seperate a few things, namely your checks for a REFERER and your IP ADDRESS ?

Also, if this is a session handler, it should only be handling session related tasks, ie those you map from the default PHP handler such as open(), write(), and read().

The database side of things should be encapsulated to a lower layer, whereby you check your database related errors.

Also, I don't believe that you should have this part in your handler either, for the given reason I have given above,

...
if(!isset($_SESSION)) { 
            session_start(); 
        } else { 
            session_destroy(); 
            session_start(); 
            $this->_msg('session', 'should not start a session outside the session object'); 
        } 
... 

Anyway, the good news is that it is fixable

[vivehosting.com]

What do you mean I need to seperate the checks for a referer and ip?

[Widow Maker]

Even though you may store the IP ADDRESS(es) in a SESSION variable, actual logic in using and verifying an IP ADDRESS has nothing to do with a session handler.

The sole purpose of a session handler is just that, and nothing more. So you'll need to trim the fat in your class(es) much like you do with the fat on a bit of steak really

[vivehosting.com]

I guess we have different point of views here . I prefer to have this automation optional rather than remove it altogether. You are a firm believer of keeping things as abstract as possible. Both aren't wrong, but we can't all agree .

[maetl]

Heres another basic idea for a session class, that I've found to be quite effective on smaller projects, really all it does is wrap the builtin functions in PHP with dataspace methods. retrieve() and store() are a complete waste of time now I think btw...

IMHO, I think functionally, what your class does is good, but it's not really about managing the session itself, seems, more about handling the way data is stored, which I think was Widow Maker's point there...

I would use the session object to delegate the actual storage/manipulation of data to a specialised handler object - that way the session object (from the perspective of other parts of the app) just provides methods to pass values in and pull them out when needed, regardless of the way data is actually stored...

 
 class session {
 
     var $values;
 
     function session() {
         session_start();
         if (defined('SESSION_NAME')) {
             session_name(SESSION_NAME);
         }
         foreach ($_SESSION as $ss => $s) {
             $this->values[$ss] = $s;
         }
     }
  
     /**
     * set a new property to the session 
     */
     function set($var,$val) {
         $_SESSION[$var] = $val;
         $this->values[$var] = $val;
     }
  
     /**
     * get the value of a property
     */
     function get($var) {
         return $this->values[$var];
     }
 
     /**
     * get the current session id 
     */    
     function getId() {
         return session_id();
     }
  
     /**
     * retrieve a serialized object 
     */
     function retrieve($var) {
         return unserialize($this->values[$var]);
     }
 
     /**
      * store a serialized object 
     */
      function store($var,$obj) {
          $this->set($var,serialize($obj));
      }
  
     /*
     * delete a property from the session
     */
     function delete($var) {
         unset($_SESSION[$var]);
         unset($this->values[$var]);
     }
  
     /**
     * erase the entire session 
     */
     function erase() {
         unset($this->values);
         session_unset();
         session_destroy();
     } 
  
 }