0

Possible Duplicate:
Headers already sent by PHP

In the code below somehow NONE of the headers are redirecting to the specified locations..i don't know why...i am not sending or echoing any output before the header is called . also there are no accidental "white spaces" which can also lead to malfunctioning of header (). I also tried using ob_start() but it was in vain. also all my files are in a single folder ie in the "www" folder of WAMP.....can someone tell me whats the snag ?

this code processes the form used for registering a new user.....POST method is used

$user= "root" ;
$host= "localhost" ;
$password= "" ;

$database= "online_examination" ;
$fn=$_POST['fn'] ;    // firstname
$ln=$_POST['ln'] ;    // lastname
$un=$_POST['un'] ;  // username
$pass=$_POST['pw'] ;  // password

$connection= mysql_connect($host,$user,$password) ;
$db= mysql_select_db($database,$connection);
$query=" SELECT username FROM user_info " ;
$result=mysql_query ($query,$connection) ;

for ($i=0 ; $i<mysql_num_rows($result) ; $i++ )
{
    $uname=mysql_result($result,$i,"username") ;

    if ($un==$uname)
       {
           header ("Location : /username_exists.php") ;
           exit;
       }
}

$query=" SELECT password FROM user_info " ;
$result=mysql_query ($query,$connection) ; 

for ($i=0 ; $i<mysql_num_rows($result) ; $i++ )
{
     $pword=mysql_result($result,$i,"password") ;
     if ($pass==$pword)
       {
            header ("Location : /password_exists.php") ;
            exit;
       }
}

$query=" INSERT INTO user_info (firstname,lastname,username,password) VALUES
('$fn','$ln','$un','$pass') " ;

mysql_query ($query,$connection)

header ("Location : /successfully_registered.php") ;
Community
  • 1
  • 1
Halo
  • 1
  • 2
  • How is it 'malfunctioning'? What errors are you getting? – j08691 Jun 25 '12 at 19:20
  • I assume you have error reporting enabled? Also could you please try to format your sentences? – PeeHaa Jun 25 '12 at 19:20
  • Also, what about fixing those [SQL injection](http://en.wikipedia.org/wiki/SQL_injection) vulnerabilities? – PeeHaa Jun 25 '12 at 19:22
  • One last thing: "is not working" is not an error message nor a problem description. – PeeHaa Jun 25 '12 at 19:24
  • malfunctioning as in the page is not redirected to the required destination .... the form action is process_register.php...and the page stays on process_register.php instead of redirecting to the specified ones – Halo Jun 25 '12 at 19:36

3 Answers3

0

This code is full of security holes and logical errors, but I have tried to rewrite it as best I can.

$user = 'root';
$host = 'localhost';
$password = '';
$database = 'online_examination';

// Attempt to connect to MySQL
if( !( $connection = mysql_connect( $host , $user , $password ) ) ){
  die( 'Failed to connect to server' );
}elseif( !( $db = mysql_select_db( $database , $connection ) ) ){
  die( 'Failed to connect to database' );
}

// Default values for Form Submitted Fields
$fn = $ln = $un = $pass = false;
// Check if Form Submitted
if( $_POST ){
  // For each value, perform some basic validation before trusting them
  if( isset( $_POST['fn'] ) && $_POST['fn']!='' )
    $fn = $_POST['fn'] ;    // firstname
  if( isset( $_POST['ln'] ) && $_POST['ln']!='' )
    $ln = $_POST['ln'] ;    // lastname
  if( isset( $_POST['un'] ) && $_POST['un']!='' )
    $un = $_POST['un'] ;  // username
  if( isset( $_POST['pw'] ) && $_POST['pw']!='' )
    $pass = $_POST['pw'] ;  // password
}

// If a Username was submitted
if( !$fn || !$ln || !$un || !$pw ){

  // One or more of the fields were empty or not submitted.
  // Show the form again (maybe with an error message)

}else{

  // Perform a Query looking for any instances where the same username is already in use
  $query = 'SELECT COUNT(*) AS matches FROM user_info WHERE username="'.mysql_real_escape_string( $un ).'"';
  $result = @mysql_query( $query , $connection ) ;
  if( !$result ){
    die( 'Query for Usernames Failed' );
  }
  $row = mysql_fetch_array( $result )
  if( $row['matches']!=0 ){
    // The Username is already in use
    if( !headers_sent() ){
      header( 'Location: /username_exists.php' );
    }else{
      echo 'Username already in use - <a href="/username_exists.php">Click here</a>';
    }
    die();
  }

  // If we have gotten to this point, the username is OK to use
  $sqlTpl = 'INSERT INTO user_info ( firstname , lastname , username , password ) VALUES ( "%s" ,  "%s" ,  "%s" , "%s" )';
  $sqlStr = sprintf( $sqlTpl ,
    mysql_real_escape_string( $fn ) ,
    mysql_real_escape_string( $ln ) ,
    mysql_real_escape_string( $un ) ,
    mysql_real_escape_string( $pw ) );
  $result = mysql_query( $sqlStr , $connection );
  if( $result ){
    if( !headers_sent() ){
      header( 'Location: /successfully_registered.php' );
    }else{
      echo 'Successfully registered - <a href="/successfully_registered.php">Click here</a>';
    }
    die();
  }else{
    // Something went wrong
  }
}

A few points from the hip:

  • Looping through all returned rows and matching them individually is an obtuse way to check if a value exists. SQL is much better at doing that kind of thing - read up on it.
  • Performing a check to see whether a password is already in use is pointless. I would bet that one or two people on StackOverflow have the same passwords, but they were not prompted with a message saying "Someone already has 'abc123' as a password. Pick another." If anything, that kind of message is a security risk rather than a security measure.
  • Never trust input. Assuming that there will be a POST submission is a recipe for disaster.
  • So is not validating whatever input you get.
  • And even more so not escaping it for use within a database query. Google for "Little Bobby Tables".
  • If you are going to store passwords, you should NEVER STORE THEM IN PLAIN TEXT. They should be hashed and salted. (Again, Google is your friend.)
  • Assuming that you can change headers is something to be done with caution. Checking with headers_sent() is a good practice.
  • Always test for errors as you go. A small error at the start, which could be detected and abort any following actions is better than letting a small mistake snowball.

Check out some of the pre-existing tutorials and/or PHP classes which handle user registrations. Alot of them have good ideas, which you should incorporate into your solutions rather than reinventing the wheel.

Luke Stevenson
  • 10,357
  • 2
  • 26
  • 41
-1

Also, make sure that you have no whitespace outside of your <?php tags as this causes text to be sent to the browser and unless you have output buffering on, will result in an error saying "headers already sent". Also, you have very unsafe SQL handling. Any of your variables can be exploited for a successful SQL injection.

pedigree
  • 101
  • 1
  • 1
  • 8
-1
$connection= mysql_connect($host,$user,$password); 
$db= mysql_select_db($database,$connection);

It is probably failing due to a php error. You were missing your semicolons.

Try error_reporting(E_ALL); at the top to double check for errors;

K.B.
  • 452
  • 2
  • 9
  • sorry but i forgot to add the semicolons here....they are present in my actual code – Halo Jun 25 '12 at 19:52
  • Including the one on mysql_query ($query,$connection) ? – K.B. Jun 25 '12 at 20:18
  • Also, just tested your code, the header location does NOT work if it is not followed directly by a colon. It needs to be header("Location: /successfully_registered.php"); NOT header("Location : /successfully_registered.php"); – K.B. Jun 25 '12 at 20:26
  • thnks a ton K.B :) :)....it worked /.... – Halo Jun 25 '12 at 20:34
  • Great news! Don't forget to mark it as answered. :) – K.B. Jun 25 '12 at 21:12