Error — session_destroy() — Trying to destroy uninitialized session

Question

I'm getting an error using session_destroy() in my PHP code.

The following script is on every page and if a user is signed in, it checks if the session is valid or not, killing the session if it's not.

session_start();

// check for users already signed in and check session
if (isset($_SESSION['user_id'])) {
    $uid = $_SESSION['user_id'];

    // check user_id is a valid id
    if (!is_numeric($uid) || $uid < 0) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }

    // if user agent is different, kill session
    if ($_SESSION['user_agent'] != $_SERVER['HTTP_USER_AGENT']) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }

    // if user's last login record fails to match session_id, kill session
    $SQL = "SELECT user_session FROM users_logins ";
    $SQL .= "WHERE user_id = :user_id ";
    $SQL .= "ORDER BY time_in DESC LIMIT 1;";
    $STH = $DBH_P->prepare($SQL);
    $STH->bindParam(':user_id', $uid);
    $STH->execute();
    $row = $STH->fetch();
    if ($STH->rowCount() > 0) {
        $db_sid = $row['user_session'];
    }
    if ($db_sid !== session_id()) {
        session_unset();
        session_destroy();
        session_regenerate_id(true);
    }
}

The error I receive indicates the failure is coming from the last session_destroy() call.

Am I using session_destroy() correctly or not? I have read other questions on here but most answers advise that session_start() must be used before destroying it, but I have started the session at the top, before the check begins.

_Why_ are you using session_unset and session_destroy? regenerating the id should be enough. And why are you checking the user id in the session to be numeric? If that should be necessary at all – do it before you put it into the session. — CBroe, Aug 31 '13 at 17:47
@CBroe I'm using session_unset and session_destroy to log the user out if any issues found, so when they go to visit another page they have to login again. I'm checking that the user_id is numeric because of session hijacking. Just because I checked this before I put it in the session in the first place, does not mean it's going to valid on this page if somebody has played with it. — TheCarver, Aug 31 '13 at 17:49
I now track that `session_unset` myth since about yesterday here on SO. It's regardless what you tell users, somehow the rumors are there that the more functions you see fit to throw into your script the better it would be. Actually the opposite is the case. — hakre, Aug 31 '13 at 18:03
That you verify / filter session values is good btw, consider session data as user-data that is injected into your script because as your write correctly, it actually is. Same applies to the data that is *returned* from your database btw. — hakre, Aug 31 '13 at 18:05

score 1 · Accepted Answer · answered Aug 31 '13 at 18:01

1

You do some crazy stuff there (but you need to negotiate that with your own, I don't cover it in my answer), the reason why you see the error message is quite simple:

 session_regenerate_id(true);

is commanding PHP to destroy the old session. Problem is, you already did that, one line earlier:

 session_destroy();
 session_regenerate_id(true);

So just take a view from above. There is no reason in an OCD manner to throw as many functions as you see fit (but actually don't understand/know well) onto your session processing. Instead take the one function that is intended to do the job and actually process it's return value if you want to put some safety net in there actually. That would be more helpful.

answered Aug 31 '13 at 18:01

hakre

193,403
52
435
836

Thanks for explaining, nicely. I suppose I got a bit carried away with checking my sessions. I came from Classic ASP not that long ago, where managing sessions was a bit simpler (for a reason, because it was rubbish). Maybe I'll just use `session_regenerate_id(true);` if any conditions are true, redirect the user to the logout page where all the session data gets cleared anyway, and then bounce them back to the page they were on. – TheCarver Aug 31 '13 at 18:22
1

Yes, that's what I wanted to say, that function alone does it (with `true` as parameter). – hakre Aug 31 '13 at 18:35

Error — session_destroy() — Trying to destroy uninitialized session

1 Answers1

Linked

Related