Generate unique key and save in the database not working

Question

I'm trying to create a unique key to save information in the database, but for some reason the $randStr variable has nothing at the end. After submitting, I get only a new id and email, nothing appears in the keystring. What's wrong?

Here's my sql table:

create table users (
    id int(11) not null PRIMARY KEY AUTO_INCREMENT,
    keystring varchar(110) not null,
    email varchar(220) not null
);

php code:

<?php

include_once 'includes/dbh.php';

function checkKeys($conn, $randStr) {
    $sqlcheck = "SELECT keystring FROM users";
    $result = mysqli_query($conn, $sqlcheck);

    while ($row = mysqli_fetch_assoc($result)) {
        if ($row['keystring'] == $randStr) {
            $keyExists = true;
            break;
        } else {
            $keyExists = false;
        }
    }

    return $keyExists;
}

function generateKey($conn) {
    $keyLength = 8;
    $str = "0123456789abcdefghijklmnopqrstuvwxyz";
    $randStr = substr(str_shuffle($str), 0, $keyLength);

    $checkKey = checkKeys($conn, $randStr);

    while($checkKey == true) {
        $randStr = substr(str_shuffle($str), 0, $keyLength);
        $checkKey = checkKeys($conn, $randStr);
    }

    return $randStr;
}

$recipient = $_POST['emailFor'];
$sender = $_POST['senderEmail'];
$message = $_POST['message'];

$sql = "INSERT INTO users (keystring, email) VALUES('$randStr', '$sender');";
mysqli_query($conn, $sql);

See about sql injection and the importance of prepared and bound queries — Strawberry, Dec 28 '20 at 00:56
Side note: Do not use string interpolation or concatenation to get values into SQL queries. That's error prone and might make your program vulnerable to SQL injection attacks. Use parameterized queries. See ["How to include a PHP variable inside a MySQL statement"](https://stackoverflow.com/questions/7537377/how-to-include-a-php-variable-inside-a-mysql-statement) and ["How can I prevent SQL injection in PHP?"](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). — sticky bit, Dec 28 '20 at 01:18
aside about sql injection, where is the code to call the generateKey function? have you posted all the code? — Ronny Sulistio, Dec 28 '20 at 01:20
pregenerate the keys in a table perhaps 100, then put a flag on it that its taken, then you can just grab a free key.. before that you could count how many free keys are left and spawn a process to insert say another 100 keys. better than hanging the user whilst trying to allocate a key — Lawrence Cherone, Dec 28 '20 at 02:08
@Lawrence That seems overkill given that the key space is 36^8 entries, or something around 3 trillion. Collisions will be rare. The real issue will be reading them all to check, but a suitably indexed column will allow MySQL to do that as part of the INSERT query. — Tangentially Perpendicular, Dec 28 '20 at 03:25

score 1 · Accepted Answer · answered Dec 28 '20 at 02:18

1

You are not calling generateKey function. Call this function and store return value to $randStr before inserting in database.

Note: please see SQL injection prevention for same.

$randStr = generateKey($conn);

// Your insert statement

answered Dec 28 '20 at 02:18

Dark Knight

6,116
1
15
37

score 1 · Answer 2 · answered Dec 28 '20 at 03:14

While the accepted answer is correct as far as it goes, there is much here that can be improved. Reading all the existing keys and checking them one by one is intensive, and will get slower as the number of keys increases. It also opens a window for a race condition, where two requests are competing fro resources.

Applying a unique index to the keystring column allows us to perform an atomic 'test and set' with MySQL checking the key and inserting it in one operation.

We can also switch to prepared queries to eliminate the risk of SQL injection.

So, apply a UNIQUE index to the keystring column. This modification to the table only needs to be done once.

ALTER TABLE `users` ADD UNIQUE INDEX `keystring_UNIQUE` (`keystring` ASC) VISIBLE;

Then the new code looks like this:

function generateKey() {
    $keyLength = 8;
    $str = "0123456789abcdefghijklmnopqrstuvwxyz";
    $randStr = substr(str_shuffle($str), 0, $keyLength);
    return $randStr;
}

$recipient = $_POST['emailFor'];
$sender = $_POST['senderEmail'];
$message = $_POST['message'];

// This will be required somewhere
$conn = mysqli_connect("xxxx", "xxxx", "xxxx", "xxxx");

// Prepare the query
$stmt = mysqli_prepare($conn, "INSERT INTO users (keystring, email) VALUES(?, ?);");

// If the key generated already exists the loop will try again.
// Limit the collisions before we give up
$collisionLimit = 10;

do {
    $randstr = generateKey();
    mysqli_stmt_bind_param($stmt, 'ss', $randstr, $sender);
    $result = mysqli_stmt_execute($stmt);


    // check the result. If it's TRUE all's well and we continue. 
    // If we have an error throw an Exc eption if it's not a duplicate key error

    if ($result === false) {
        if (mysqli_stmt_errno($stmt) != 1062) {
            throw new Exception(mysqli_stmt_error($stmt));
        }

        // If we have a collision, check the limit and throw an Exception if necessary
        if (--$collisionLimit) {
            throw new Exception("Unable to insert key - too many collisions");
        }
    }

} while ($result===false);

I appreciate your tips, I tried to make this change in the database but this error appears: #1064 -You have an error in your SQL syntax next to 'VISIBLE' on line 1. — Vitor Freitas, Dec 28 '20 at 11:52
`VISIBLE` and `INVISIBLE` indexes are a [feature](https://dev.mysql.com/doc/refman/8.0/en/invisible-indexes.html) of MySQL 8 — Tangentially Perpendicular, Dec 28 '20 at 15:19

Generate unique key and save in the database not working

2 Answers2