HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Custom session handler

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customhandlersession

Problem

I'm new to PDO and haven't been coding in a while. Is the following custom session handler industry-acceptable?

```
class Session {
protected $sdb;

function __construct() {
session_set_save_handler(array($this, '_open'), array($this, '_close'), array($this, '_read'), array($this, '_write'), array($this, '_destroy'), array($this, '_clean'));
register_shutdown_function('session_write_close');
}

function _open() {
$this->sdb['conn'] = new PDO('mysql:host=' . DBHOST . ';dbname=' . DBNAME . ';charset=utf8', DBUSER, DBPASS);
$this->sdb['conn']->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

return TRUE;
}

function _close() {
$this->sdb['conn'] = NULL;

return TRUE;
}

function _read($sid) {
try {
$this->sdb['read']['query'] = $this->sdb['conn']->prepare('SELECT data FROM sessions WHERE sid = :sid LIMIT 1')) {
$this->sdb['read']['query']->bindParam(':sid', $sid, PDO::PARAM_STR);
$this->sdb['read']['query']->execute();
$this->sdb['read']['result'] = $this->sdb['read']['query']->fetch(PDO::FETCH_ASSOC);
}
catch( PDOException $exception ) {
var_export($exception);

return FALSE;
}

$this->sdb['read']['query']->closeCursor();

return $this->sdb['read']['result']['data'];
}

function _write($sid, $data) {
try {
$this->sdb['write']['query'] = $this->sdb['conn']->prepare('REPLACE INTO sessions VALUES (:sid, :time, :data)');

$this->sdb['write']['query']->bindParam(':sid', $sid, PDO::PARAM_STR);
$this->sdb['write']['query']->bindParam(':time', date('Y-m-d H:i:s'));
$this->sdb['write']['query']->bindParam(':data', $data, PDO::PARAM_STR);
$this->sdb['write']['query']->execute();
}
catch( PDOException $exception ) {
var_export($exception);

return FAL

Solution

It would be better to implement SessionHandlerInterface.
Your code is tightly coupled, move session_set_save_handler out from constructor to some bootstrap file. Also this class shouldn't create db connection, it is better to pass it as a constructor parameter:

$conn = new PDO('mysql:host=' . DBHOST . ';dbname=' . DBNAME . ';charset=utf8', DBUSER, DBPASS);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$handler = new MySessionHandler($conn);
session_set_save_handler($handler, true);


Constructor for the session handler:

class Session implements \SessionHandlerInterface
{
    protected $conn;

    public function __construct(\PDO $conn)
    {
        $this->conn = $conn;
    }

    // ....
}


This gives you more freedom, in case you decide to extend PDO you can pass it there. Also you don't need to create separate connection for application and session.
Implementing Interface ensures that you fulfill all requirements.

What I find odd is that you put every query to handler property, they should be local variables, you create them on every method call anyway.

Code Snippets

$conn = new PDO('mysql:host=' . DBHOST . ';dbname=' . DBNAME . ';charset=utf8', DBUSER, DBPASS);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$handler = new MySessionHandler($conn);
session_set_save_handler($handler, true);
class Session implements \SessionHandlerInterface
{
    protected $conn;

    public function __construct(\PDO $conn)
    {
        $this->conn = $conn;
    }

    // ....
}

Context

StackExchange Code Review Q#80728, answer score: 2

Revisions (0)

No revisions yet.