thminco Posted October 24, 2011 Share Posted October 24, 2011 This is my first attemp at a log in system for a website. Everything seems to work fine until the "successful" IF function near the end. All I get it an output of "?>" instead of a redirect to the file "login_success.php". Any help would be GREATLY appreciated!! Tom <?php // Connect to server and select databse. mysql_connect("localhost", "scripts3_public", "sfj123!")or die("cannot connect"); mysql_select_db("scripts3_sfj")or die("cannot select DB"); // username and password sent from form $fusername=$_POST['fusername']; $fpassword=$_POST['fpassword']; // To protect MySQL injection (more detail about MySQL injection) $fusername = stripslashes($fusername); $fpassword = stripslashes($fpassword); $fusername = mysql_real_escape_string($fusername); $fpassword = mysql_real_escape_string($fpassword); $sql="SELECT * FROM `users` WHERE `User name` = '$fusername' AND `Password` = '$fpassword'"; $result=mysql_query($sql); if(!mysql_num_rows($result)) {echo "No results returned.";} // Mysql_num_row is counting table row $count=mysql_num_rows($result); // If result matched $fusername and $fpassword, table row must be 1 row if($count==1){ // Register $fusername, $fpassword and redirect to file "login_success.php" session_register("fusername"); session_register("fpassword"); header("location:login_success.php"); } else { echo "Wrong Username or Password"; } ?> Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 I don't see it but maybe you have it and are not showing it, but you should have session_start(); at the top of any page that will use sessions. You are also missing $_ on both your session lines and you are not setting them to a value. Should be. // Register $fusername, $fpassword and redirect to file "login_success.php" $_session['fusername']=$fusername; $_session['fpassword]'=$fpassword; header("location:login_success.php"); } Quote Link to comment Share on other sites More sharing options...
ManiacDan Posted October 24, 2011 Share Posted October 24, 2011 1) $_SESSION is in all caps. 2) Session_register (and related functions) are deprecated and should not be used. Read the manual page on sessions for modern syntax 3) Don't post your database password in public. 4) Your use of stripslashes leads me to believe you're using magic_quotes. This is also deprecated and should be removed immediately. 5) you must die() immediately after header calls. 6) There is a space after the colon in a header redirect, and Location is capitalized. 7) View the SOURCE of a page to see the complete output. Always store passwords encrypted with a salted one-way hash in the database. Never store them "plain" like this. 9) This is clearly copied and pasted from a tutorial. Stop using that tutorial right now, it appears to be 5-6 years old. -Dan Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted October 24, 2011 Share Posted October 24, 2011 All I get it an output of "?>" Are you sure php is installed on your server and you browsed to the URL of your page so that php code would be parsed/executed on the server? Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 Man I hate when I make mistakes. ManiacDan is of course correct on every point and my posted code should have been. // Register $fusername, $fpassword and redirect to file "login_success.php" $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header("location: login_success.php"); die(); } Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 OK, here is my updated code thanks to all of the wonderful corrections (Thank you all SO MUCH!!) The script is still not doing the re-direct for some reason?? Yes, I was indeed getting help from an (old) online tutorial as I am very new to the log in work. Dan...I don't even know what magic quotes are?? Should I still not use the stripslashes?? Also included below is the landing file code. if($count==1){ // Register $fusername, $fpassword and redirect to file "login_success.php" $_SESSION["fusername"]=$fusername; $_SESSION["fpassword"]=$password; header("Location: login_success.php"); die(); } else { echo "Wrong Username or Password"; } ob_end_flush(); ?> LANDING FILE CODE: <? session_start(); if(!session_is_registered(fusername)){ header("location:sfjogin.php"); } ?> <html> <body> Login Successful </body> </html> Quote Link to comment Share on other sites More sharing options...
Far Cry Posted October 24, 2011 Share Posted October 24, 2011 OK, here is my updated code thanks to all of the wonderful corrections (Thank you all SO MUCH!!) The script is still not doing the re-direct for some reason?? Yes, I was indeed getting help from an (old) online tutorial as I am very new to the log in work. Dan...I don't even know what magic quotes are?? Should I still not use the stripslashes?? Also included below is the landing file code. <? if($count==1){ // Register $fusername, $fpassword and redirect to file "login_success.php" $_SESSION["fusername"]=$fusername; $_SESSION["fpassword"]=$password; header("Location: login_success.php"); die(); } else { echo "Wrong Username or Password"; } ob_end_flush(); ?> LANDING FILE CODE: <? session_start(); if(!session_is_registered(fusername)){ header("location:sfjogin.php"); } ?> <html> <body> Login Successful </body> </html> 1). Magic qoutes was used to escape strings to prevent MySQL injection, just use mysql_real_escape_string();. 2). <? session_start(); if(!session_is_registered(fusername)){ header("location:sfjogin.php"); } ?> Should be... <?php session_start(); if(!$_SESSION['fusername']){ header("Location:sfjogin.php"); die(); } ?> 3). Never use shorthand (<? ?>) PHP. Bad practice. Quote Link to comment Share on other sites More sharing options...
ManiacDan Posted October 24, 2011 Share Posted October 24, 2011 1) Magic quotes is a php.ini setting that randomly sticks backslashes into your strings if they have single quotes or other special characters. It's a VERY old method of SORT OF protecting against SQL injection. If your php.ini has it on, turn it off. 2) Your login page snippet is now correct, though there's no reason to be using the output buffering code probably. ob_end_clean is not usually necessary, but I don't know what the rest of your code is using. 3) Your landing page suffers from the same problems as your first page did. session_is_registered(fusername) should be replaced with isset( $_SESSION['fusername'] ). Die after a header. Etc. 4) Still, view your source to see the full output. It's possible your server doesn't even know what PHP is and all of this is a wasted exercise. Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 Thanks again everyone. I feel like I should be paying you all for this help!! I corrected all the landing file errors but don't think I am even getting to that page (file) because the url doesn't change in my browser address box. I have 5 sites that are all using php flawlessly (with I'm sure lots of newbie coding flaws) at my host. (They are using php5.2... something) I had some little echo statements in the if execution at the end of my code to make sure I was getting that far and the echos worked fine, so I'm pretty sure its not an exersize in futility. UPDATED CODE FROM INITIAL SCRIPT... if($count==1){ // Register $fusername, $fpassword and redirect to file "login_success.php" $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header('Location: http://www.***.com/login_success.php'); die(); } else { echo "Wrong Username or Password"; } ?> UPDATED CODE FROM LANDING FILE PAGE (login_success.php)... <?php session_start(); if(!$_SESSION['fusername']){ header("Location: sfjlogin.php"); die(); } ?> <html> <body> Login Successful </body> </html> Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 BTW Dan, when I view source from the browser at sfjlogin.php I don't see anything (because it is all php on the server?) I don't get redirected anywhere from there. Thanks very much again!!! Tom Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 Can you echo $count to see if that value is as expected and the enclosed code is being executed? Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 Yes, if I add the 2 echo statements below, I get an output as follows: 1thomas This is what I would expect and shows that I have made it into the "IF" function successfully. if($count==1){ echo $count; echo $fusername; // Register $fusername, $fpassword and redirect to file "login_success.php" $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header('Location: http://www.***.com/login_success.php'); die(); } else { echo "Wrong Username or Password"; } ?> Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 I usually quote my header location. header("Location: http://www.***.com/login_success.php"); Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 Thanks drummin. I changed to the double quotes but get the same result...an output of the $fusername and $count variables but no url redirection. Quote Link to comment Share on other sites More sharing options...
xyph Posted October 24, 2011 Share Posted October 24, 2011 Single quotes are better because it saves PHP from parsing a string it doesn't have to. To the OP, you should be programming with errors visible. You're getting a header error that isn't being displayed. Check out header in the manual. You can't output before calling it. Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 Sorry gang, I double posted! Man, it hurts being such a newbie! Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 Thanks xyph...I put that code in and got this error message: Warning: session_start() [function.session-start]: Cannot send session cache limiter - headers already sent (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 7 1thomas Warning: Cannot modify header information - headers already sent by (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 43 It seems I have something wrong with the way I am using "session_start()" on line 7?? I am headed back to the manual, but unfortunately it is a little bit greek for me! Thanks to everyone for the help!! BTW...thanks Dan for the tip on using encryption!! Very cool!! Also, it seems like I really don't need the escaping for sql injection since I am not inserting or appending to the sql database in this script?? Here is my entire code at this point: <?php ini_set('display_errors',1); error_reporting(-1); session_start(); // IMPORT FORM VARIABLES $fusername=$_POST['fusername']; $fpassword=$_POST['fpassword']; // CONNECT TO SERVER AND SELECT DATABASE mysql_connect("localhost", "scripts3_public", "******")or die("cannot connect"); mysql_select_db("scripts3_sfj")or die("cannot select DB"); // PROTECT AGAINST MYSQL INJECTION mysql_real_escape_string($fusername); mysql_real_escape_string($fpassword); // ENCRYPT FORM PASSWORD TO COMPARE WITH DATABASE ENCRYPTED PASSWORD $encrypt_fpassword=md5($fpassword); // LOOKUP USERNAME AND PASSWORD IN DATABASE COMPARED TO FORM ENTRIES $sql="SELECT * FROM `users` WHERE `User name` = '$fusername' AND `Password` = '$encrypt_fpassword'"; $result=mysql_query($sql); if(!mysql_num_rows($result)) {echo "No results returned.";} // COUNT RESULTS $count=mysql_num_rows($result); // IF COUNT IS 1,REGISTER USERNAME AND PASSWORD AND REDIRECT if($count==1){ echo $count; echo $fusername; $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header("Location: http://www.******.com/login_success.php"); die(); } else { echo "Wrong Username or Password"; } ?> Quote Link to comment Share on other sites More sharing options...
xyph Posted October 24, 2011 Share Posted October 24, 2011 Even SELECTing you should prevent injection. Imagine a query like SELECT * FROM `users` WHERE `user`='username' AND `pass`='password' OR 1=1 session_start() and header() must be called before any output to the browser. Quote Link to comment Share on other sites More sharing options...
litebearer Posted October 24, 2011 Share Posted October 24, 2011 my 2 cents... 1) if you haven't previously stored the password in a hashed form, then hashing the password for use in the query will not work 2) this echo $count; echo $fusername; $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header("Location: http://www.******.com/login_success.php"); die(); will result in a header issued problem Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 Obviously my post aren't worth much but get rid of the echo lines sending something to the browser or at least move them to after setting the session values, but as you're redirecting you don't need them. if($count==1){ $_SESSION['fusername']=$fusername; $_SESSION['fpassword']=$fpassword; header("Location: http://www.******.com/login_success.php"); die(); } Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 I can't figure out what this means: "session_start() and header() must be called before any output to the browser." What is meant by output to the browser? The calling of the header function? I do call "session_start();" at the start of my code and per the manual am supposed to also call "session_name" before "session_start" and before any other code, thus I now have my code beginning as follows: <?php session_name("sfjloginsession"); session_start(); // TURN ON ERROR REPORTING ini_set('display_errors',1); error_reporting(-1); My latest error gives this: Warning: Cannot modify header information - headers already sent by (output started at /home2/scripts3/public_html/sfjlogin.php:2) in /home2/scripts3/public_html/sfjlogin.php on line 46 Is this maybe because I have some session info already stored and need to clear it?? Quote Link to comment Share on other sites More sharing options...
Drummin Posted October 24, 2011 Share Posted October 24, 2011 session_start(); needs to be before using or calling any session and before anything is sent to the browser. Assuming "sfjloginsession" is supposed to be a session value. <?PHP session_start(); $sfjloginsession=$_SESSION['sfjloginsession']; // TURN ON ERROR REPORTING ini_set('display_errors',1); error_reporting(-1); Quote Link to comment Share on other sites More sharing options...
ManiacDan Posted October 24, 2011 Share Posted October 24, 2011 Echo, print, printf, and various other output functions count as "output." You also cannot have anything outside of PHP tags. If your page begins with a single space before the <?php tag, then it will fail to use a header. Similarly, if you include a file with a single space after the closing ?>, it will fail. The error messages you're pasting contain a file and line number. That's VERY easy to figure out. Go to that file. Go to that line. Fix that. You cannot output anything on any page if you're going to be using a header redirect. You're using md5 on your passwords now. 2 things: 1) use sha1, it's better 2) You need to update your database so the passwords in the DB are also hashed the same way. Quote Link to comment Share on other sites More sharing options...
thminco Posted October 24, 2011 Author Share Posted October 24, 2011 DAN!!!!!!!!!! You are truly the man! There was one blank line in the code before the opening php (<?php) statement in line 2. Once I deleted that first blank line...It worked just fine! Thank you so much for all your help, and BTW I will look into the other encrypting method you mentioned. I have only 2 passwords in the database (one of them encrypted/hashed) so now is a good time to start using whatever is the best encryption!! Thanks again SO MUCH to everyone who helped! I am sure my code is overall a LOT cleaner and better, but with plenty still to do!! Tom Quote Link to comment Share on other sites More sharing options...
ManiacDan Posted October 24, 2011 Share Posted October 24, 2011 If you're starting from scratch, use sha1() with a salt. Your encrypted password should be sha1($password . 'someReallyLongRandomStringYouComeUpWithYourself'). Keep the random string secret. It prevents your code from being vulnerable to rainbow table attacks. 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.