[unum_sanctum()]

You're missing one major point. What is the return status of the function?

if( user_exists("foobar")) 
{
   // will this ever execute? 
}
else
{ 
   // or this???
}

	function user_exists($username){
	    $username = sanitize($username);
	    $sql = "SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'";
	    $result = mysql_query($sql);
	    if(!$result) {
	        echo "There was a error in the following sql statement : <br>".$sql;
                //maybe a throw new Exception goes here
	    }
            /* start patch */
	    // Don't you want to return true or false here? 
            if( mysql_num_rows($result) > 0 ) {
              return true; 
            }else {
              return false;
            }
            /* end patch */
	}

*** EDIT ***
wow, I really am batting 1,000 today..

I didn't notice your query was "SELECT COUNT(*)" ... My patch may not work if you use COUNT(*).
Better to change your query to select an ID from the table.

[Atli]

unum_sanctum(), on 09 October 2012 - 04:01 PM, said:

My patch may not work if you use COUNT(*).

It won't. A SELECT COUNT(*) will always return one row, which means you version would always return true.

unum_sanctum(), on 09 October 2012 - 04:01 PM, said:

Better to change your query to select an ID from the table.

No, that's rarely a good idea. You don't fetch data from the database to count it in PHP when MySQL can easily, and more efficiently, count it for you.

You'd want to fetch the row and return based on the COUNT from MySQL:

function user_exists($username){
    // What does "sanitize" do? I don't know, but judging by the
    // name it would not be sufficient to prevent SQL Injection.
    $username = sanitize($username);

    // And so, you would have to also do this, unless this is
    // actually done inside "sanitize", in which case you should
    // consider renaming it to indidcate that.
    $username = mysql_real_escape_string($username);
    
    $sql = "SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'";
    $result = mysql_query($sql);
    if($result) {
        $row = mysql_fetch_row($result);
        return $row[0] === 1;
    } else {
        // Don't use echo or die to print errors. Use PHP's
        // trigger_error() function instead. That way, PHP can
        // either display them or log them according to the PHP
        // config. This is far better, as you can configure your
        // production servers so that the user will never see the
        // errors (which they never should), but they'll still be
        // logged.
        trigger_error("Query failed: " . mysql_error(), E_USER_WARNING);
        return null;
    }
}

[unum_sanctum()]

Atli, on 09 October 2012 - 09:38 AM, said:

// Don't use echo or die to print errors. Use PHP's
// trigger_error() function instead. That way, PHP can

What's great about this solution is you can use a custom error handler to catch your trigger_error() calls. Good call on the sanitize function as well.

[CTphpnwb]

JackOfAllTrades, on 09 October 2012 - 09:17 AM, said:

This was covered by RudiVisser above:

It can't be emphasized enough!

[no2pencil]

Duplicate topics merged. There is no reason to post the same question twice.