chriscloyd Posted October 5, 2006 Share Posted October 5, 2006 here is the error i have[quote]Warning: mysql_fetch_array(): supplied argument is not a valid MySQL result resource in /home/chris/public_html/login.php on line 11Warning: mysql_fetch_array(): supplied argument is not a valid MySQL result resource in /home/chris/public_html/login.php on line 12Warning: Cannot modify header information - headers already sent by (output started at /home/chris/public_html/login.php:11) in /home/chris/public_html/login.php on line 28[/quote]and this is the code that is there[code]<?phpsession_start();include("db.php");if(!$email || !$password){$error = "Please Fill Out Both Fields!";header("Location: index.php?error=".$error."");} else {$getinfo = mysql_query("SELECT password FROM users WHERE email = '$email'");$check_admin = mysql_query("SELECT password FROM admins WHERE email = '$email'");$admin = mysql_fetch_array($check_admin);$user = mysql_fetch_array($getinfo); if($admin == 1){ if(md5($password) == $admin['password']){ $_SESSION['cssadmin'] = $email; } } if($user == 1){ if(md5($password) == $user['password']){ $_SESSION['css'] = $email; header("Location: index.php"); } else { $error = "The Password You Entered Was Incorrect!"; header("Location: index.php?error=".$error.""); } } else { $error = "Their Is No Such Email Registered Here!"; header("Location: index.php?error=".$error.""); }}?>[/code] Quote Link to comment Share on other sites More sharing options...
Orio Posted October 5, 2006 Share Posted October 5, 2006 Can you show the code of db.php?Orio. Quote Link to comment Share on other sites More sharing options...
chriscloyd Posted October 5, 2006 Author Share Posted October 5, 2006 <?php$dbhost = 'localhost';$dbuser = '***********';$dbpass = '***********f';$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Error connecting to mysql');$dbname = '**********';mysql_select_db($dbname);?> Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted October 5, 2006 Share Posted October 5, 2006 I'm not sure what's causing your errors, but just a design suggestion. I notice you're returning your error string as part of the URL; this is going to make it difficult to provide information about multiple errors. Not to mention your URLs are going to get very long on pages that cause errors. You're using sessions, so why not have a $_SESSION['Errors'] array? Each time a page loads, it will check for error messages in that array, display them, and then clear the array.[code]<?php // Login.phpsession_start();$HadErrors = false;if(!$email){ $_SESSION['Errors'][] = 'Missing email address.'; $HadErrors = true;}if(!$password){ $_SESSION['Errors'][] = 'Missing password.'; $HadErrors = true;}if($HadErrors){ header( 'Location: index.php' ); exit();}// Rest of code below?>[/code][code]<?php // index.phpsession_start();// First check for errorsif(is_array($_SESSION['Errors']) && count($_SESSION['Errors'])){ $out = 'The following errors occured:<ul>'; foreach($_SESSION['Errors'] as $Error){ $out .= '<li>' . $Error . '</li>'; } $out .= '</ul>';}$_SESSION['Errors'] = Array(); // Clear any previous errors// Rest of page follows?>[/code] Quote Link to comment Share on other sites More sharing options...
chriscloyd Posted October 5, 2006 Author Share Posted October 5, 2006 i changed my code but i still get those three errors[quote]Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/chris/public_html/login.php on line 17Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /home/chris/public_html/login.php on line 22Warning: Cannot modify header information - headers already sent by (output started at /home/chris/public_html/login.php:17) in /home/chris/public_html/login.php on line 36[/quote]heres my new code [code]<?phpsession_start();include("db.php");$HadErrors = false;if(!$email){ $_SESSION['Errors'] = 'Missing email address.'; $HadErrors = true;}if(!$password){ $_SESSION['Errors'] = 'Missing password.'; $HadErrors = true;}if($HadErrors){ header( 'Location: index.php' );}$check_admin = mysql_query("SELECT * FROM admins WHERE email = '$email'");$admin = mysql_num_rows($check_admin);if($admin['email'] == $email){ $_SESSION['cssadmin'] = $email;}$check_user = mysql_query("SELECT * FROM users WHERE email = '$email'");$user = mysql_num_rows($check_user);if($user == '1'){ $password = md5($password); if($user['password'] == $password){ $_SESSION['css'] = $email; } else { $_SESSION['Errors'] = 'Password was incorrect.'; $HadErrors = true; }} else {$_SESSION['Errors'] = 'Email address was not found in database.';$HadErrors = true;}if($HadErrors == true){header( 'Location: index.php' );}?>[/code] Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted October 5, 2006 Share Posted October 5, 2006 I made a typo in my post, after calling header("Location: <some_url>"); you should make a call to exit(), otherwise your script will keep running.As I said in my original post, I'm not sure what's causing your errors specifically; it all looked fine to me as far as mysql is concerned. Quote Link to comment Share on other sites More sharing options...
chriscloyd Posted October 5, 2006 Author Share Posted October 5, 2006 thanks for that session i cant tell if it works yet because it wont let me run the login script its weird i ahve done millions of login scripts and this is the only problem one i have ever made :( Quote Link to comment Share on other sites More sharing options...
Orio Posted October 5, 2006 Share Posted October 5, 2006 Try changing:[code]$getinfo = mysql_query("SELECT password FROM users WHERE email = '$email'");$check_admin = mysql_query("SELECT password FROM admins WHERE email = '$email'");[/code]To:[code]$query1="SELECT password FROM users WHERE email = ".$email;$query2="SELECT password FROM admins WHERE email = ".$email;$getinfo = mysql_query($query1) or die("Problem with Query1:<br>".mysql_error()."<br><br>");$check_admin = mysql_query($query2) or die("Problem with Query2:<br>".mysql_error());[/code]Orio. Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted October 5, 2006 Share Posted October 5, 2006 Also, you need to check if the result of mysql_query is a valid resource or not before using it.http://www.php.net/mysql_queryLook at the section [b]Return Values[/b] and not that FALSE indicates a bad query.You'll want to do something like:[code]<?php$check_admin = mysql_query("SELECT * FROM admins WHERE email = '$email'");if($check_admin){ $admin = mysql_num_rows($check_admin);}else{ $admin = 0;}?>[/code]Since all you're doing is a simple assignment, you can shorten it up a bit into:[code]<?php$check_admin = mysql_query("SELECT * FROM admins WHERE email = '$email'");$admin = $check_admin ? mysql_num_rows($check_admin) : 0;?>[/code] Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted October 5, 2006 Share Posted October 5, 2006 Another error:[code]<?php$admin = mysql_num_rows($check_admin);if($admin['email'] == $email){?>[/code]mysql_num_rows returns an integer but you're using it as an array.Also, I'm not sure you why'd type:[code]if($admin['email'] == $email){[/code]in the first place since the query is already checking if it exists.Basically what you've done is:1) "Hey, MySQL, can you find all rows where email is equal to $email"2) "Thanks MySQL, now can you give me the number of rows that were returned."3) "Now I want to know if the row returned has the same email as what's in $email" - [b]but you already did that in step 1![/b]At the point of that if statement, $admin will contain the number of rows returned, either 0 or greater than 0. If you want to know if the user is an admin, just do[code]if($admin){ // greater than zero evaluates as true // do admin stuff}[/code] Quote Link to comment Share on other sites More sharing options...
chriscloyd Posted October 5, 2006 Author Share Posted October 5, 2006 still aint working Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted October 5, 2006 Share Posted October 5, 2006 Can you post the current code and current set of errors? Quote Link to comment Share on other sites More sharing options...
alpine Posted October 5, 2006 Share Posted October 5, 2006 Now - i would agree on some rewriting on this, but i've re-arranged a bit on your original postYou probably are using POST on this too, so defining variables as POST....[code]<?phpsession_start();include("db.php");if(!empty($_POST['email'] || !empty($_POST['password'] ){$error = "Please Fill Out Both Fields!";header("Location: index.php?error=".$error."");exit();}else{$email = mysql_real_escape_string($_POST['email']);$password = mysql_real_escape_string($_POST['password']);$getinfo = mysql_query("SELECT password FROM users WHERE email = '$email'") or die(mysql_error());if(mysql_num_rows($getinfo) == "1"){$user = mysql_fetch_array($getinfo);if(md5($password) == $user['password']){$_SESSION['css'] = $email;header("Location: index.php");exit();}else{$error = "The Password You Entered Was Incorrect!";header("Location: index.php?error=".$error."");exit();}}else{$check_admin = mysql_query("SELECT password FROM admins WHERE email = '$email'") or die(mysql_error());if(mysql_num_rows($check_admin) == "1"){$admin = mysql_fetch_array($check_admin);if(md5($password) == $admin['password']){$_SESSION['cssadmin'] = $email;}else{$error = "The Password You Entered Was Incorrect!";header("Location: index.php?error=".$error."");exit();}}else{$error = "Their Is No Such Email Registered Here!";header("Location: index.php?error=".$error."");exit();}} }?>[/code]But instead of running for only matches on email and then extracting password from db, it would be better to simply run"SELECT id FROM users WHERE password = '$password' AND email = '$email'"I see you are comparing to see if the user have the identical password as the one actually stored in db on THAT username, and giving an error msg if the password is incorrect. In my point of view you should not give that kind of message because then you have already revealed a valid username with only a wrong password. One barrier less to break. Nevertheless, this is quite common afterall - I prefer to keep login usernames just as secret as login passwords and just give a common "no found" message on failure. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.