Account activation PHP

Question

I created this account registration activation script of my own, I have checked it over again and again to find errors, I don't see a particular error...

The domain would be like this:

http://domain.com/include/register.php?key=true&p=AfRWDCOWF0BO6KSb6UmNMf7d333gaBOB

Which comes from an email, when a user clicks it, they get redirected to this script:

if($_GET['key'] == true)
{
    $key = $_GET['p'];

    $sql = "SELECT * FROM users
            WHERE user_key = '" . $key . "'";

    $result = mysql_query($sql) or die(mysql_error());

    if(mysql_affected_rows($result) > 0)
    {
        $sql = "UPDATE users
                SET user_key = '', user_active = '1'
                WHERE user_key = '" . $key . "'";

        $result = mysql_query(sql) or die(mysql_error());

        if($result)
        {
            $_SESSION['PROCESS'] = $lang['Account_activated'];
            header("Location: ../index.php");
        }
        else
        {
            $_SESSION['ERROR'] = $lang['Key_error'];
            header("Location: ../index.php");
        }
    }
    else
    {
        $_SESSION['ERROR'] = $lang['Invalid_key'];
        header("Location: ../index.php");
    }
}

It doesn't even work at all, I looked in the database with the user with that key, it matches but it keeps coming up as an error which is extremely annoying me. The database is right, the table and column is right, nothing wrong with the database, it's the script that isn't working.

Help me out, guys.

Thanks :)

Answer 1

1. Change $_GET['key'] == true to $_GET['key'] == "true"
2. You do before this if, a successful mysql_connect(...) or mysql_pconnect(...) ?
3. Change mysql_affected_rows($result); to mysql_num_rows($result);. Affected you can use for DELETE or UPDATE SQL statements.
4. Before you second if was opened, add before you second mysql_result(...), mysql_free_result($result); to free memory allocated to previous result.
5. if($result) change to if(mysql_affected_rows($result));. You can do that here.
6. After the header(...); function call's add a return 0; or exit(0); depends on your complete code logic.
7. You are using $key variable in SQL statements, to get your code more secure on SQL Injection attacks get change $key = $_GET['p']; to $key = mysql_real_escape_string($_GET['p']);
8. I think your location in header() functions fails. In header() url address should be full like: http://www.example.com/somewhere/index.php
9. And check your $_GET['p'] variable exists!! If this not exist and if $_GET['key'] exists, you find all activated users. Then i think the setting user_key to '' is nessesary if you have user_activated marker.

Answer 2

you shouldnt be using:

if(mysql_affected_rows($result) > 0)

You should be using mysql_num_rows()

Answer 3

Your problem is:

$result = mysql_query($sql) or die(mysql_error());

"or" makes your statement boolean so $result gets a True instead of value returned by mysql_query()

echo 'Hello' or die('bye'); // outputs nothing, because result is True not 'Hello'

3 or die() == True; // true
3 or die() != 3; // true

OR is the same as || and it is operator of logical statement.

This will work:

$result = mysql_query($sql);
if(!$result) die(mysql_error());    

The same mistake was made a few hours ago: link

---

Cases where OR can be used:

defined('FOO') or
    define('FOO', 'BAR');

mysql_connect(...) or die(...);

mysql_select_db( .... ) or die(...);

mysql_query('UPDATE ...') or die(...);

if(FOO or BAR) { ... }