Loop until passcode is unique

This is for a file sharing website. In order to make sure a "passcode", which is unique to each file, is truely unique, I'm trying this:

$genpasscode = mysql_real_escape_string(sha1($row['name'].time())); //Make passcode out of time + filename.
    $i = 0;
    while ($i < 1) //Create new passcode in loop until $i = 1;
    {
        $query = "SELECT * FROM files WHERE passcode='".$genpasscode."'";
        $res = mysql_query($query);
        if (mysql_num_rows($res) == 0) // Passcode doesn't exist yet? Stop making a new one!
        {
            $i = 1;
        }
        else // Passcode exists? Make a new one!
        {
            $genpasscode = mysql_real_escape_string(sha1($row['name'].time()));
        }
    }

This really only prevents a double passcode if two users upload a file with the same name at the exact same time, but hey better safe than sorry right? My question is; does this work the way I intend it to? I have no way to reliably (read: easily) test it because even one second off would generate a unique passcode anyway.

UPDATE: Lee suggest I do it like this:

do {
    开发者_如何学Go$query = "INSERT IGNORE INTO files 
       (filename, passcode) values ('whatever', SHA1(NOW()))";
    $res = mysql_query($query);
} while( $res && (0 == mysql_affected_rows()) )

[Edit: I updated above example to include two crucial fixes. See my answer below for details. -@Lee]

But I'm afraid it will update someone else's row. Which wouldn't be a problem if filename and passcode were the only fields in the database. But in addition to that there's also checks for mime type etc. so I was thinking of this:

//Add file
        $sql = "INSERT INTO files (name) VALUES ('".$str."')";
        mysql_query($sql) or die(mysql_error());

        //Add passcode to last inserted file
        $lastid = mysql_insert_id();
        $genpasscode = mysql_real_escape_string(sha1($str.$lastid.time())); //Make passcode out of time + id + filename.
        $sql = "UPDATE files SET passcode='".$genpasscode."' WHERE id=$lastid";
        mysql_query($sql) or die(mysql_error());

Would that be the best solution? The last-inserted-id field is always unique so the passcode should be too. Any thoughts?

UPDATE2: Apperenatly IGNORE does not replace a row if it already exists. This was a misunderstanding on my part, so that's probably the best way to go!

Strictly speaking, your test for uniqueness won't guarantee uniqueness under a concurrent load. The problem is that you check for uniqueness prior to (and separately from) the place where you insert a row to "claim" your newly generated passcode. Another process could be doing the same thing, at the same time. Here's how that goes...

Two processes generate the exact same passcode. They each begin by checking for uniqueness. Since neither process has (yet) inserted a row to the table, both processes will find no matching passcode in database, and so both processes will assume that the code is unique. Now as the processes each continue their work, eventually they will both insert a row to the files table using the generated code -- and thus you get a duplicate.

To get around this, you must perform the check, and do the insert in a single "atomic" operation. Following is an explanation of this approach:

If you want passcode to be unique, you should define the column in your database as UNIQUE. This will ensure uniqueness (even if your php code does not) by refusing to insert a row that would cause a duplicate passcode.

CREATE TABLE files (
  id int(10) unsigned NOT NULL auto_increment PRIMARY KEY,
  filename varchar(255) NOT NULL,
  passcode varchar(64) NOT NULL UNIQUE,
)

Now, use mysql's SHA1() and NOW() to generate your passcode as part of the insert statement. Combine this with INSERT IGNORE ... (docs), and loop until a row is successfully inserted:

do {
    $query = "INSERT IGNORE INTO files 
       (filename, passcode) values ('whatever', SHA1(NOW()))";
    $res = mysql_query($query);
} while( $res && (0 == mysql_affected_rows()) )

if( !$res ) {
   // an error occurred (eg. lost connection, insufficient permissions on table, etc)
   // no passcode was generated.  handle the error, and either abort or retry.
} else {
   // success, unique code was generated and inserted into db.
   // you can now do a select to retrieve the generated code (described below)
   // or you can proceed with the rest of your program logic.
}

Note: The above example was edited to account for the excellent observations posted by @martinstoeckli in the comments section. The following changes were made:

• changed mysql_num_rows() (docs) to mysql_affected_rows() (docs) -- num_rows doesn't apply to inserts. Also removed the argument to mysql_affected_rows(), as this function operates on the connection level, not the result level (and in any case, the result of an insert is boolean, not a resource number).
• added error checking in the loop condition, and added a test for error/success after loop exits. The error handling is important, as without it, database errors (like lost connections, or permissions problems), will cause the loop to spin forever. The approach shown above (using IGNORE, and mysql_affected_rows(), and testing $res separately for errors) allows us to distinguish these "real database errors" from the unique constraint violation (which is a completely valid non-error condition in this section of logic).

If you need to get the passcode after it has been generated, just select the record again:

$res = mysql_query("SELECT * FROM files WHERE id=LAST_INSERT_ID()");
$row = mysql_fetch_assoc($res);
$passcode = $row['passcode'];

Edit: changed above example to use the mysql function LAST_INSERT_ID(), rather than PHP's function. This is a more efficient way to accomplish the same thing, and the resulting code is cleaner, clearer, and less cluttered.

I'd personally would have write it on a different way but I'll provide you a much easier solution: sessions.

I guess you're familiar with sessions? Sessions are server-side remembered variables that timeout at some point, depending on the server configuration (the default value is 10 minutes or longer). The session is linked to a client using a session id, a random generated string.

If you start a session at the upload page, an id will be generated which is guaranteed to be unique as long the session is not destroyed, which should take about 10 minutes. That means that when you're combining the session id and the current time you'll never have the same passcode. A session id + the current time (in microseconds, milliseconds or seconds) are NEVER the same.

In your upload page:

session_start();

In the page where you handle the upload:

$genpasscode = mysql_real_escape_string(sha1($row['name'].time().session_id()));
// No need for the slow, whacky while loop, insert immediately
// Optionally you can destroy the session id

If you do destroy the session id, that would mean there's a very slim chance that another client can generate the same session id so I wouldn't advice that. I'd just allow the session to expire.

Your question is:

does this work the way I intend it to?

Well, I'd say... yes, it does work, but it could be optimized.

Database

To make sure to not have the same value in the field passcode on the database layer, add a unique key to this:

/* SQL */
ALTER TABLE `yourtable` ADD UNIQUE `passcode` (`passcode`);

(duplicate key handling has to be taken care of than ofc)

Code

To wait a second until a new Hash is created, is ok, but if you're talking heavy load, then a single second might be a tiny eternity. Therefore I'd rather add another component to the sha1-part of your code, maybe a file id from the same database record, userid or whatever which makes this really unique.

If you don't have a unique id at hand, you still can fall back to a random number rand-function in php.

I don't think mysql_real_escape_string is needed in this context. The sha1 returns a 40-character hexadecimal number anyway, even if there are some bad characters in your rows.

$genpasscode = sha1(rand().$row['name'].time());

...should suffice.

Style

Two times the passcode generation code is used in your code sample. Start cleaning this up in moving this into a function.

$genpasscode = gen_pc(row['name']);

...

function gen_pc($x) 
{
  return sha1($row[rand().$x.time());
}

If I'd do it, I'd do it differently, I'd use the session_id() to avoid duplicates as good as possible. This way you wouldn't need to loop and communicate with your database in that loop possibly several times.

You can add unique constraint to your table.

ALTER TABLE files ADD UNIQUE (passcode);

PS: You can use microtime or uniqid to make the passcode more unique.

Edit: You make your best to generate a unique value in php, and unique constraint is used to guarantee that at database side. If your unique value is very unique, but in very rare case it failed to be unique, just feel free to give the message like The system is busy now. Please try again:).