3
\$\begingroup\$

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 FALSE;
        }

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

        return TRUE;
    }

    function _destroy($sid) {
        try {
            $this->sdb['destroy']['query'] = $this->sdb['conn']->prepare('DELETE FROM sessions WHERE id = :sid');

            $this->sdb['destroy']['query']->bindParam(':sid', $sid, PDO::PARAM_STR);
            $this->sdb['destroy']['query']->execute();
        }
        catch( PDOException $exception ) {
            var_export($exception);

            return FALSE;
        }

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

        return TRUE;
    }

    function _clean() {
        try {
            $this->sdb['clean']['query'] = $this->sdb['conn']->prepare('DELETE FROM sessions WHERE access < :stale');

            $this->sdb['clean']['query']->bindParam(':stale', date("Y-m-d H:i:s", strtotime("-30 minutes")));
            $this->sdb['clean']['query']->execute();
        }
        catch( PDOException $exception ) {
            var_export($exception);

            return FALSE;
        }

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

        return TRUE;
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.