<?php $user = $_GET['usid']; $code = $_GET['pass']; $date = date("D M d, Y"); $time = date("G:i a"); if (Login is clicked) { $con = mysql_connect("localhost"); if (!$con) { die('CONNECTION UNAVAILABLE ' . mysql_error()); } else { mysql_select_db("Attendance", $con); $pass = mysql_query("Select PW FROM Employees WHERE EmpID == $user"); if ( $pass == $code ) { mysql_query("INSERT INTO Attendance (CDate, TimeIn) VALUES ($date, $time) WHERE (Id==$user && pass==$code); echo "You have logged in as: " . $user . " at " . date("D M d, Y G:i a"); mysql_close($con); } else { echo "Wrong password or username, try again." } else { $con = mysql_connect("localhost"); if (!$con) { die('CONNECTION UNAVAILABLE ' . mysql_error()); } else { mysql_select_db("Attendance", $con); $pass = mysql_query("Select PW FROM Employees WHERE EmpID == $user"); if ( $pass == $code ) { mysql_query("INSERT INTO Attendance (CDate, TimeOut) VALUES ($date $time)") WHERE (Id==$user && pass==$code); echo "You have logged out at " . date("D M d, Y G:i a"); mysql_close($con); } else { echo "Wrong password or username, try again." } } ?> This is my code, im trying to input the login time of a user when he clicks on the login button, and to input the logout time when he logs off. it's not working please help
Where to start.... 1) you are passing GET requests directly into an SQL statement... bad move. You are now prone to SQL injection attacks. 2) you don't need to setup variables to handle the current date and time, you can use SQL syntax to do this job. 3) I hope that the string for if (Login is clicked) is just an example here and not real code... 4) you don't close your SQL query on line 30. Use "); 5) You don't close your statement on line 39. Use ; 6) You don't close your statement on line 69. Use ; 7) Your if else structure is wrong, check it! I won't post here all the fixes, you should check this yourself. 8) Your SQL query syntax on line 60 is completely wrong! I may have even missed some errors there were that many. Get yourself a proper editor, not just notepad, that highlights syntax and you will see for yourself.
I agree with Oxi. Your code is a bit scruffy and is vulnerable to both SQL injection and Cross Site Scripting attacks. I mean no offence, it's better to be completely honest about these things. The main problem is that you perform SQL queries without actually retrieving the results. Here's what I mean: $pass = mysql_query("Select PW FROM Employees WHERE EmpID == $user"); Code (markup): $pass is not the password. It's a database resource, you still need to use a mysql_fetch_* function on it. Couple of other tips: Use single quotes in combination with the concatenation operator (.) instead of double quotes. Example: 'Hello ' . $username instead of "Hell $username". It's faster. Store your passwords in encrypted form instead of plaintext, lookup MD5 hashing. Here's my code (incomplete, but you get the idea): <?php try { if (!isset($_GET['usid'])) throw new Exception('No username entered'); if (!isset($_GET['pass'])) throw new Exception('No password entered'); if (!isset($_GET['method'])) throw new Exception('Method not set'); // prevent SQL injection attacks $username = mysql_escape_string($_GET['usid']); $password = mysql_escape_string($_GET['pass']); $method = mysql_escape_string($_GET['method']); // connect to and select database mysql_connect('localhost') or die('CONNECTION UNAVAILABLE ' . mysql_error()); mysql_select_db('Attendance'); if ($method == 'login') { $query = mysql_query('SELECT PW FROM Employees WHERE EmpID == ' . $username); // check results if (!$query) throw new Exception('Could not run query' . mysql_error()); if (mysql_num_rows($query) == 0) throw new Exception('User not found'); // grab first row $row = mysql_fetch_assoc($query); if ($password == $row['PW']) { // start a session $date = date('D M d, Y'); $time = date('G:i a'); mysql_query('INSERT INTO Attendance(CDate, TimeIn) VALUES('.$date.','.$time.') WHERE (Id == '.$username.' && '.'pass == "'.$password.'")'); echo 'You have logged in as: ' . htmlentities($username) . ' at ' . date('D M d, Y G:i a'); } else { throw new Exception('Wrong username or password, please try again.'); } } else if ($method == 'logout') { // check for session, if not set - don't bother with logout crap mysql_query('INSERT INTO Attendance(CDate, TimeOut) VALUES('.$date.','.$time.') WHERE (Id=='.$user.' && pass=="'.$code.'")'); echo 'You have logged out at ' . date('D M d, Y G:i a'); } else { throw new Exception('Are you logging in or logging out?'); } } catch (Exception $e) { echo 'Error: ' . $e->getMessage() . "\n"; } mysql_close($con); ?> PHP:
Agree with the additional tips above from Deacalion except on the concatenation operator. I see no reason why variables cannot be used directly in an SQL statement, its actually quicker that way and even shows up in a text editor differently so can still be seen as a variable whilst you are working It may just be your preferred method of working (saw it in the provided code).
I admit it's more my preferred style of coding. However, it is also faster using concatenation and single quotes. By about 14% - nowhere near noticeable in real world use though .
Benchmark proof: <?php define(ITERATIONS, 100000); $t = microtime(true); for ($i = 0; $i < ITERATIONS; $i++) { $j = $i . ' hmm...'; } $first = (microtime(true) - $t); $t = microtime(true); for ($i = 0; $i < ITERATIONS; $i++) { $j = "$i hmm..."; } $second = (microtime(true) - $t); echo number_format((($second-$first)/$first)*100, 2) . '% Faster'; ?> PHP:
I meant in the purpose of SQL query statements not string separation. I agree with you on that e.g. You use $query = mysql_query('SELECT PW FROM Employees WHERE EmpID == ' . $username); PHP: instead of $query = mysql_query('SELECT PW FROM Employees WHERE EmpID == $username'); PHP:
It's so the script can process both logins and logouts without the need of having two seperate scripts. For example, if you wanted to login: <form action="/script.php" method="get"> <input type="text" name="username" /> <input type="text" name="password" /> <input type="hidden" name="method" value="login" /> </form> HTML: And to logout, you could just have a link: <a href="/script.php?method=logout">Logout</a> HTML: Also, might want to change the first three lines after 'try' in my script to: if (isset($_GET['method']) && $_GET['method'] == 'logout') { $method = mysql_escape_string($_GET['method']); } else { if (!isset($_GET['usid'])) throw new Exception('No username entered'); if (!isset($_GET['pass'])) throw new Exception('No password entered'); if (!isset($_GET['method'])) throw new Exception('Method not set'); // prevent SQL injection attacks $username = mysql_escape_string($_GET['usid']); $password = mysql_escape_string($_GET['pass']); $method = mysql_escape_string($_GET['method']); } PHP:
Heres my input (untested); PS: You can use the following, to check if someone is logged in you can check if (isset($_SESSION['user'])) ...and you can also extract info by using that within the WHERE clause within your querie(s). <?php session_start(); $type = strtolower(@$_GET['type']); /* -To login -> ?type=login -TO logout -> ?type=logout */ $date = date('D M d, Y'); $time = date('G:i a'); if ($type == 'login' && !isset($_SESSION['user'])) { ?> <form method="get"> Username: <input type="text" name="user" /><br /> Password: <input type="password" name="pass" /><br /> <input type="hidden" name="type" value="Login" /> </form> <?php //validate input... if (strlen(@$_GET['user']) > 0 && strlen(@$_GET['pass']) > 0) { //sanitize input to prevent sql injection... $input_user = mysql_real_escape_string($_GET['user']); $input_pass = mysql_real_escape_string($_GET['pass']); $con = mysql_connect("localhost") or trigger_error("Unable to connect ".mysql_error(), E_USER_ERROR); mysql_select_db("Attendance", $con) or trigger_error("Unable to connect to database ".mysql_error(), E_USER_ERROR); $result = mysql_query("SELECT PW FROM Employees WHERE EmpID == '{$input_user}'"); $pass = mysql_result($result, 0, 0); if ($pass == $input_pass) { mysql_query("INSERT INTO Attendance (CDate, TimeIn) VALUES ($date, $time) WHERE (Id == '{$input_user}' && pass == '{$input_pass}'"); //create session... $_SESSION['user'] = $input_user; //sanitize input to prevent xss... $input_user = htmlspecialchars($input_user); echo "You have logged in as: {$input_user} at " . date('D M d, Y G:i a'); mysql_close($con); } else { echo "Wrong password or username, try again."; } } } elseif ($type == 'logout' && isset($_SESSION['user'])) { mysql_query("INSERT INTO Attendance(CDate, TimeOut) VALUES($date, $time) WHERE (Id == '{$_SESSION['user']}'"); //delete session(s)... $_SESSION = array(); session_destroy(); echo "You have logged out at " . date('D M d, Y G:i a'); } ?> PHP: