I'm posting my PHP code for sessions using MySQL. Please tell me if there are any errors, if it can be optimized, or if security can be enhanced.
- Host: localhost
- User: root
- Password: ""
- Database: pro
Table structure:
SessionID [pk] - Data - DateTouched
<?php
function open($sess_id, $sess_name) {
return true;
}
function close() {
return true;
}
function read($sess_id) {
$con = mysqli_connect("localhost", "root", "","pro");
$stmt = mysqli_prepare($con,"SELECT Data FROM sessions WHERE SessionID = ?");
mysqli_stmt_bind_param($stmt,"s",$sess_id);
mysqli_stmt_execute($stmt);
mysqli_stmt_bind_result($stmt,$data);
mysqli_stmt_fetch($stmt);
mysqli_stmt_close($stmt);
$CurrentTime = date('Y-m-d H:i:s');
if (!isset($data))
{
$stmt = mysqli_prepare($con,"INSERT INTO sessions (SessionID, DateTouched) VALUES (?,?)");
mysqli_stmt_bind_param($stmt,"ss",$sess_id,$CurrentTime);
mysqli_stmt_execute($stmt);
mysqli_stmt_close($stmt);
return false;
}
else
{
$stmt = mysqli_prepare($con,"UPDATE sessions SET DateTouched = ? WHERE SessionID = ?");
mysqli_stmt_bind_param($stmt,"ss",$CurrentTime,$sess_id);
mysqli_stmt_execute($stmt);
mysqli_stmt_close($stmt);
return $data;
}
}
function write($sess_id, $data) {
$con = mysqli_connect("localhost", "root", "","pro");
$CurrentTime = date('Y-m-d H:i:s');
$stmt = mysqli_prepare($con,"UPDATE sessions SET Data= ?,DateTouched=? WHERE SessionID=?");
mysqli_stmt_bind_param($stmt,"sss",$data,$CurrentTime,$sess_id);
mysqli_stmt_execute($stmt);
return true;
}
function destroy($sess_id) {
$con = mysqli_connect("localhost", "root", "","pro");
$stmt = mysqli_prepare($con,"DELETE FROM sessions WHERE SessionID = ?");
mysqli_stmt_bind_param($stmt,"s",$sess_id);
mysqli_stmt_execute($stmt);
return true;
}
function gc($sess_maxlifetime)
{
$con = mysqli_connect("localhost", "root", "","pro");
$stmt = mysqli_prepare($con,"Delete from sessions where TIMESTAMPDIFF (Second ,DateTouched,now())>=?");
mysqli_stmt_bind_param($stmt,"s",$sess_maxlifetime);
mysqli_stmt_execute($stmt);
return true;
}
session_set_save_handler("open", "close", "read", "write", "destroy", "gc");
session_name('Session');
session_start();
?>
2 Answers 2
Your open
and close
functions don't do anything. You could use them to connect & disconnect from the database. That way, an error in the connection won't be a read or write error, but will be an error in open
. That may later help diagnosing such errors. It also means less duplicated code and it may mean fewer connections to the database (which might speed up things a bit).
-
\$\begingroup\$ good idea ronald, anything else you will suggest ? \$\endgroup\$Sourav– Sourav2011年05月08日 17:59:07 +00:00Commented May 8, 2011 at 17:59
-
1\$\begingroup\$ @Sourav Well, personally I'd put all of it in a class so I could carry the connection around in an object... \$\endgroup\$rlc– rlc2011年05月09日 14:21:00 +00:00Commented May 9, 2011 at 14:21
-
\$\begingroup\$ anything for security or increasing performance ? \$\endgroup\$Sourav– Sourav2011年05月09日 14:23:12 +00:00Commented May 9, 2011 at 14:23
Especially in ajaxy environments, a lot of different request can be opened to your server. In their current implementation there is no check or lock for that, which means that setting a session variable somewhere can be undone by a session save in another request.
- Request A open
- Request B open
- A changes data
- A exist & saves
- B exists & saves stale data
I'd recommand either doing an SELECT ..... FOR UPDATE
or checking last-opened-time with last-changed-time in the database.
-
\$\begingroup\$ plz explain a little bit more ! \$\endgroup\$Sourav– Sourav2011年06月08日 02:30:24 +00:00Commented Jun 8, 2011 at 2:30
-
\$\begingroup\$ What do you need more explanation of? You can see PHP's file-based session solution in action by creating a file with
<?php session_start();sleep(60);?>
, and while that is loading, try to open any other page with asession_start()
in it: it won't load any further beyond the blockingsession_start()
until the other request is finished. Doing aSELECT .... FOR UPDATE
is pretty much the same thing, only it locks a row in the database (i.e. it cannot be selected until the connection having the lock either releases it or exits). \$\endgroup\$Wrikken– Wrikken2011年06月08日 18:04:37 +00:00Commented Jun 8, 2011 at 18:04 -
\$\begingroup\$ If you do so, make the
gc
a cronjob rather then in the code of a request itself: thegc
handler will also block until the lock on that row is released. To avoid something like that, you could store a token (not a timestamp, microseconds do count) in the row in the database and only allow writing if the token has stayed the same, but that would mean 2 simultaneous requests (that are not blocked in this manner) could want to write different changes to the session in the database. You'll have to think of something clever in your application to handle that if you support concurrent requests. \$\endgroup\$Wrikken– Wrikken2011年06月08日 18:09:17 +00:00Commented Jun 8, 2011 at 18:09 -
\$\begingroup\$ but InnoDB only the row gets locked not the table (as in MyISAM), and as a user should only have a single session then i dont think it's problem. \$\endgroup\$Sourav– Sourav2011年06月09日 02:55:58 +00:00Commented Jun 9, 2011 at 2:55
-
\$\begingroup\$ Yes, but the
gc
needs access to all rows afaik... \$\endgroup\$Wrikken– Wrikken2011年06月09日 08:28:26 +00:00Commented Jun 9, 2011 at 8:28