Flying Circus,
thanks for the detailed reply. I shall take your backtick comment on board. Perhaps I over de-bugged and forgot to try the new script with backticks.
You've replied just as I've finished something which I was hoping you would look at. In a previous post (which you have replied to) I mentioned that I was powering the user authentication within the session class. Someone, it may have been you, advised me that this was a bad idea as I might run into problems in the future. On the back of that advise, I've written a User class which corresponds with the SessionHandler class we've just been discussing.
It works (or seems to) but I was hoping you might glance over it, purely for advice on how you would tighten it up.
Here's my php front end (note that I reference our beloved SessionHandler):
Code: Select all
<?php
/*
* Session Handler
*/
require_once ('SessionHandler2.php');
$sess = new Session();
session_start();
require_once('User.php');
$_SESSION['cart'] = 'cart';
$_SESSION['location'] = 'Ukraine';
if($_POST['usr']){
$usr_mail = $_POST['usr'];
$usr_pwd = $_POST['pwd'];
}else{
$usr_mail = $_SESSION['usr_mail'];
$usr_pwd = NULL;
}
$usr = new User($sess->getSess(), $usr_mail, $usr_pwd);
echo($_SESSION['usr_mail']);
?>
<form method="POST" action='session2.php'>
<input name='usr' type='text'>
<input name='pwd' type='password'>
<button type='submit'>login</button>
</form>
Her is the User class (note that I'm using your sprintf() function):
Code: Select all
<?php
define('DB_HOST', 'localhost');
define('DB_USER', 'root');
define('DB_PASS', '');
define('DB_DB', 'dev');
class User{
# Variables
private $connection_usr;
private $usr_update_fields = array(
array('detail_country', 'location'),
array('usr_cart', 'cart')
);
# Methods
public function __construct($sess_id, $usr_mail, $usr_pwd = 0){
if($usr_pwd){
if($success = $this->auth_usr_sess($sess_id, $usr_mail, $usr_pwd)){
$_SESSION['usr_mail'] = $usr_mail;
$this->update_usr_sess_data($sess_id, $usr_mail);
}else{
$_SESSION['usr_mail'] = "guest";
}
}else{
if($success = $this->auth_usr($sess_id, $usr_mail)){
$_SESSION['usr_mail'] = $usr_mail;
$this->update_usr_sess_data($sess_id, $usr_mail);
}else{
$_SESSION['usr_mail'] = "guest";
}
}
}
private function auth_usr_sess($sess_id, $usr_mail, $usr_pwd){
# establish connection
$this->establish_connection_usr();
# prepare password
$usr_pwd = md5($usr_pwd);
$result = mysql_query(
sprintf(
"SELECT usr_mail FROM users WHERE usr_mail = '%s' AND usr_pwd = '%s' LIMIT 1;",
mysql_real_escape_string($usr_mail, $this->connection_usr),
mysql_real_escape_string($usr_pwd, $this->connection_usr)
),
$this->connection_usr
) or die(mysql_error());
# if a user exists, update usr_session where the usr_mail and usr_pwd match and return
if(mysql_num_rows($result)){
mysql_query(
sprintf(
"UPDATE users SET usr_session = '%s' WHERE usr_mail = '%s' AND usr_pwd = '%s' LIMIT 1;",
mysql_real_escape_string($sess_id, $this->connection_usr),
mysql_real_escape_string($usr_mail, $this->connection_usr),
mysql_real_escape_string($usr_pwd, $this->connection_usr)
),
$this->connection_usr
) or die(mysql_error());
return 1;
}else{
return 0;
}
}
private function auth_usr($sess_id, $usr_mail){
# establish connection
$this->establish_connection_usr();
# update usr_session where the usr_mail and usr_pwd match and return
$result = mysql_query(
sprintf(
"SELECT usr_mail FROM users WHERE usr_session = '%s' AND usr_mail = '%s' LIMIT 1;",
mysql_real_escape_string($sess_id, $this->connection_usr),
mysql_real_escape_string($usr_mail, $this->connection_usr)
),
$this->connection_usr
) or die(mysql_error());
return mysql_num_rows($result);
}
private function update_usr_sess_data($sess_id, $usr_mail){
# start query string
$query = "UPDATE users SET ";
# iterate through the usr_update_fields and add to the query string respectively
for($i = 0; $i<sizeof($this->usr_update_fields); $i++){
$query .= sprintf(
"%s = '%s' ",
mysql_real_escape_string($this->usr_update_fields[$i]['0'], $this->connection_usr),
mysql_real_escape_string($_SESSION[$this->usr_update_fields[$i]['1']], $this->connection_usr)
) or die(mysql_error());
if($i < (sizeof($this->usr_update_fields)-1)){
$query .= ", ";
}
}
# finish the query string with the conditionals set by the users mail and session id
$query .= sprintf("WHERE usr_session = '%s' AND usr_mail = '%s' LIMIT 1;", $sess_id, $usr_mail);
# query database and return true or false
return mysql_query($query, $this->connection_usr);
}
public function destroy_usr_sess($sess_id, $usr_mail){
# nullifies the users session
$this->establish_connection_usr();
# update the usr_session to NULL
mysql_query(
"UPDATE users SET usr_session = NULL WHERE usr_mail = '".mysql_real_escape_string($usr_mail)."' AND usr_session = '".mysql_real_escape_string($sess_id)."' LIMIT 1;",
$this->connection_usr
) or die(mysql_error());
# close connection
$this->close_connection_usr();
}
private function establish_connection_usr(){
# establish a connection for querying users
$this->connection_usr = mysql_connect(DB_HOST, DB_USER, DB_PWD);
# select database for users
mysql_select_db("DB_DB", $this->connection_usr);
}
private function close_connection_usr(){
# close users connection
mysql_close($this->connection_usr);
}
}
?>
The users tables is created with the following mysql query:
CREATE TABLE IF NOT EXISTS `users` (
`usr_mail` varchar(320) NOT NULL,
`usr_pwd` varchar(50) NOT NULL,
`usr_session` varchar(50) NOT NULL DEFAULT '',
`detail_country` varchar(20) DEFAULT NULL,
`detail_birthday` varchar(10) DEFAULT NULL,
`detail_heard_about` varchar(50) DEFAULT NULL,
`usr_cart` text,
PRIMARY KEY (`usr_mail`,`usr_session`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
Some places where I feel you might help:
+ Where I initiate authentication based on a user, session AND password (auth_usr_sess()), I'm worried about having to use a SELECT query and an UPDATE query. I'm being forced to use a SELECT query so I can get the appropriate feedback from the database. Is there a way to get feedback in an UPDATE, even if the UPDATE conditions are true but no update is made due to an already populated cell?
+ The different functions are called based on some very messy looking if statements at the __construct stage. Is there a neater way of doing this?
I completely understand if you want to back out at this stage, your help and advice has already been paramount. Truth be told, I need an inspiring coding mentor, and I suppose I like the way that you've explained things so far. I've got a pile of O'Reilly books. They're good, but they don't answer those little questions that inspire you to keep going.
drayfuss