Log in

View Full Version : 2 primary keys?



Schmoopy
01-14-2009, 01:16 AM
Hi, I've currently got 2 fields (email and id) in a table (subscriptions), and the email field is the primary key.

To unsubscribe to a newsletter, the user needs to enter a valid email, but the id also has to correspond to that email, ensuring that not anyone can delete someone else off the database.

The code for the subscribing part is the important bit here:


mysql_select_db("bristoldnb_mail",$con);

$result = mysql_query("SELECT * FROM subscribers");

while($row = mysql_fetch_array($result))
{
if ($row['Email'] == $email)
{
header("Location: $server_url" . "php/index.php?pass=taken");
die();
}
elseif ($row['ID'] == $id)
{
while ($row['ID'] == $id)
{
$id = mt_rand(1000,100000000);
}
}
}


mysql_query("INSERT INTO subscribers (Email, ID)
VALUES ('$email','$id')");

I am not sure how to accomplish this, but what I want to happen in the "elseif ($row['ID'] == $id)" part is for the script to keep generating a random number until it is not equal to any other IDs in the table. This would be much easier if I could make two primary keys, but I can't - anyone know how to achieve such an effect?

Thanks,

Jack.

Twey
01-14-2009, 06:47 AM
Argh! You're looping through every single row in the database!

The solution is to redesign your database (and remove this gods-awful code!). It should look something like this (MySQL):
CREATE TABLE subscribers (id INT AUTO_INCREMENT, email VARCHAR(320), UNIQUE(email), PRIMARY KEY(id));Then, that hideous mess above becomes:
mysql_select_db('bristoldnb_mail', $con);

if (!mysql_query(sprintf('INSERT INTO subscribers (email) VALUES (\'%s\')', mysql_real_escape_string($email))))
if (mysql_errno() === 1062) // ER_DUP_ENTRY
die(header("Location: ${server_url}php/index.php?pass=taken"));
else
die(mysql_error());Or, preferably, using PDO if your server supports it (this code also includes the equivalent of your mysql_connect() call [with variables $hostname, $username, $password]):
$db = new PDO("mysql:host=$hostname;dbname=bristoldnb_mail", $username, $password);

try {
$st = $db->prepare('INSERT INTO subscribers (email) VALUES (:email)');
$st->bindParam(':email', $email);
$st->execute();
} catch (PDOException $e) {
if ($e->getCode() === 1062) // ER_DUP_ENTRY
die(header("Location: ${server_url}php/index.php?pass=taken"));
else
die($e->getMessage());
}

Schmoopy
01-14-2009, 10:04 AM
Ok thanks for that, but still 2 problems, the:



if (mysql_errno() === 1169) // ERR_DUP_UNIQUE
die(header("Location: $server_url" . "php/index.php?pass=taken"));


code doesn't work, and when it hits a duplicate entry it just echos it instead of redirecting back.

Also, I really wanted the ID to be random, instead of being auto incremented, any way to do that?

Twey
01-14-2009, 10:23 AM
What exactly does it echo?

Why do you want the ID to be random? This is inefficient (worst-case O(n) instead of the standard O(1)).

Schmoopy
01-14-2009, 10:42 AM
It echos : Duplicate entry 'test@test.com' for key 2

And I want it to be random so it isn't so predictable, but I guess it's not that important, would still be more secure though.

Twey
01-14-2009, 10:56 AM
Hm, it seems that the appropriate error was in fact ER_DUP_ENTRY. Try the edited version I've posted above (1062 [ER_DUP_ENTRY] instead of 1169 [ER_DUP_UNIQUE]).

It's not really more secure. If this is an issue, then you're already in trouble — people shouldn't be able to view sensitive information just by trying different IDs.

Schmoopy
01-14-2009, 11:08 AM
Nah it's not really like that, it's all for an unsubscribing script, so the user has to have the email and the ID right to be able to unsubscribe, I just wanted to make it random so it would be even harder to get both right.

Twey
01-14-2009, 11:59 AM
Well, yes, then, it is like that. You should do some verification on that — email them a password to enter, perhaps (hint: sha1(date('G') . $email . 'some salt') for a password that automatically expires at the end of the current hour).

Schmoopy
01-14-2009, 03:48 PM
Ok, thanks - that's all been very useful :)