Generate random and if not exist in DB,add it

Question

I'm almost there,but something is missing.My PHP app:

1)User is requesting to the server

2)The server is generating a long unique string and checks if exists in the DB:If YES then generate again(until it doesn't exists),if NO then add it to the DB and finish. All logic should be executed with a single request,i.e user should not request/refresh page if generated string exist.

I am stuck in the YES part.

My code (DISCLAIMER:I do not own parts of the following code)

 <?php
    class genPass
    {
    private $db;
        function __construct() {
            $this->db=new mysqli('localhost', 'user', 'pass', 'db');
                $this->db->set_charset("utf8");
            $this->db->autocommit(FALSE);
        }
        function __destruct() {
            $this->db->close();
        }  

    function isUsed($uid)
    {
        $stmt=$this->db->query("SELECT * FROM id WHERE udid='".$uid."'")or die($this->db->error);

        while($stmt->num_rows <1) {
        $newnum = $this->generateStrongPassword();
        $newcheck=$this->db->query("SELECT * FROM id WHERE udid='".$newnum."'")or die($this->db->error);

        if ($newcheck->num_rows >= 1) {
            echo $newnum . " exists! \n";  <- WHAT TO DO IF EXISTS?WHICH PART OF THE SCRIPT SHOULD I RUN AGAIN
        } else {
            $this->db->query("INSERT INTO id (udid) VALUES ('".$newnum."')")or die($this->db->error);
            echo "$newnum - CAN ISNERT@!@!@";
            break;
        }
    }

    }
    public function generateStrongPassword($length = 3, $add_dashes = false, $available_sets = 'lu')
    {
        $sets = array();
        if(strpos($available_sets, 'l') !== false)
            $sets[] = 'ab';//'abcdefghjkmnpqrstuvwxyz';
        if(strpos($available_sets, 'u') !== false)
            $sets[] = 'AB';//'ABCDEFGHJKMNPQRSTUVWXYZ';
        if(strpos($available_sets, 'd') !== false)
            $sets[] = '23456789';
        if(strpos($available_sets, 's') !== false)
            $sets[] = '!@#$%&*?';

        $all = '';
        $password = '';
        foreach($sets as $set)
        {
            $password .= $set[array_rand(str_split($set))];
            $all .= $set;
        }

        $all = str_split($all);
        for($i = 0; $i < $length - count($sets); $i++)
            $password .= $all[array_rand($all)];

        $password = str_shuffle($password);

        if(!$add_dashes)
            return $password;

        $dash_len = floor(sqrt($length));
        $dash_str = '';
        while(strlen($password) > $dash_len)
        {
            $dash_str .= substr($password, 0, $dash_len) . '-';
            $password = substr($password, $dash_len);
        }
        $dash_str .= $password;
        return $this->$dash_str;
    }
    }


    $obj = new genPass;
    $ran=$obj->generateStrongPassword();
    $obj->isUsed($ran);

    ?>

This code looks just bad. Just one example: isUsed function... that returns nothing? Even if it did return something, what would be the point of running a loop and checking more uids? The caller is obviously only interested in the one uid they passed in! — Jon
– Jon, Commented Oct 6, 2013 at 18:33
Jon,look at the code and this line echo "$newnum - CAN ISNERT@!@!@";,this one is the response!!!! — Theodoros80
– Theodoros80, Commented Oct 6, 2013 at 18:34
@Theodoros80: Elias is talking about this: php-fig.org/psr/0 , php-fig.org/psr/1 , php-fig.org/psr/2 and php-fig.org/psr/3 — pablofiumara
– pablofiumara, Commented Oct 6, 2013 at 18:38

Marty McVry · Accepted Answer · 2013-10-06 18:37:33Z

4

You're using a function isUsed(), which is good, but I'd suggest limiting that function to checking if the random key is used or not.

function isUsed($uid)
{
    $stmt=$this->db->query("SELECT * FROM id WHERE udid='".$uid."'")or die($this->db->error);
    if ($stmt->num_rows < 1) {
        return FALSE;
    } else {
        // Already a duplicate key, so should return TRUE for sure!!!
        return TRUE;
    }
}

That way, you could use a loop to check:

while $obj->isUsed($ran) {
    $ran = $obj->generateStrongPassword();
}
// put pwd in database after this loop!

By the way, this is just to explain the logic that has to be used... Check your code for further inconsistencies... :-)

answered Oct 6, 2013 at 18:37

Marty McVry

3,0711 gold badge22 silver badges24 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Jon Over a year ago

Why is isUsed longer than one line? Why does it generate a password again? Copy/pasting from a bad source is going to leave you with a bad result.

Elias Van Ootegem Over a year ago

@Theodoros80: Be weary of the fact that isUsed is public, and that, when passing unsantized data to it, your system is vulnerable to injection attacks, as you're just blindly stringing the passed argument to the query, and thus the DB

Theodoros80 Over a year ago

Thank you Elias! For sure i will consider making my script safer and more "PHP-FIG" standard! I didm't know that a simple question about this,would raise such comments!

Elias Van Ootegem · Accepted Answer · 2013-10-06 19:13:11Z

Ok, I'll bite:

class GenPass
{
    private $isUsedStmt = null;
    private $db = null;
    //constructor etc...
    /**
     * Works like most hashtables: re-hashes $id param until a unique hash is found
     * uses sha256 algo, currently, no hash-collisions have been found, so pretty solid
     * you could change to sha512, but that would be overkill
     * @return string
     **/
    public function insertUniqueRandom($id = null)
    {
        $id = $id ? $id : $this->getRandomId();
        do
        {//use hashes, rehash in case of collision
            $id = hash('256', $id);
        }while($this->isUsed($id));
        //once here, current $id hash has no collisions
        $this->db->query('INSERT INTO `id` (udid) VALUES ("'.$id.'")');
        return $id;//returns unique has that has been found & used/storred
    }
    /**
     * Random string generator... returns random string
     * of specfied $length (default 10)
     * @param int $length = 10
     * @return String
     **/
    public function getRandomId($length = 10)
    {
        $length = (int) ($length > 1 ? $length : 10);
        $src = '0`12345~6789abc2%def&gh$ijklm!nopq?,rs><tuvwxyz';
        $out ='';
        for ($p = 0; $p < $length; $p++)
        {
            $char = $src{mt_rand(0, strlen($src))};
            $out .= $p%2 ? strtoupper($char) : $char;
        }
        return $out;
    }
    /**
     * Check if current hash already exists in DB, if so, return false, if not, return true
     * @return Boolean
     * @throws RuntimeException
     **/
    private function isUsed($uid)
    {
        $stmt = $this->getCheckUidStmt();
        $stmt->bindParam('s', $uid);
        if ($stmt->execute)
        {
            return $stmt->num_rows === 0 ? false : true;
        }
        throw new RuntimeException('failed to query for uid usage: '.$this->db->error);
    }
    /**
     * Lazy-load isUsed's prepared statement
     * The statement won't be prepared, unless the isUsed function is called
     * @return \mysqli::prepare
     **/
    private function getCheckUidStmt()
    {
        if ($this->isUsedStmt === null)
        {
            $this->isUsedStmt = $this->db->prepare('SELECT udid FROM `id` WHERE udid = ?');
        }
        return $this->isUsedStmt;
    }
}

This is, as the comments say, how most hashtables work anyway: hash a random value, if that hash is already being used, simply hash the duplicate hash again, until that hash is not being used anywhere.
Usage:

$gen = new GenPass;
$usedID = $gen->insertUniqueRandom();
echo $usedID, ' was just inserted';
$another = $gen->insertUniqueRandom('foobar');//works, 
echo $another;//will echo:
//c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2
$foobarAgain = $gen->insertUniqueRandom('foobar');
echo $foobarAgain;//foobar already existed, now this will echo:
//181cd581758421220b8c53d143563a027b476601f1a837ec85ee6f08c2a82cad

As you can see, trying to insert "foobar" twice will result in 2 unique id's. What's more, the length of a sha256 hash is a given: its 256 bits long, or 64 chars, so that makes it easy to store in a DB: VARCHAR(64) is what you need... easy!
All things considered, I think it fair to say that this is probably the closest you'll get to a reliable, reasonably fast unique-random-id-generator you're going to get

Since your class does computations and manages storage logic, it violates the SRP. Why not to wrap storage logic inside separated dataMapper? Also have you heard about PoLS? And what it has to do with getters/setters?
@DaveJust: Aw please... Do you really expect me to write an entire layer as an answer? I don't know what the bigger picture is here, So I stuck to the task at hand, and wrote some snippet as clean as possible, without taking it too far. Why don't you take the time and explain everything to the OP, put together a couple of gists, one focussing on IoC, the other more of a DI approach... perhaps set up a third using a framework, for good measure... Yes, I may have vented some frustration in my comment to the OP's question, but at least I then put in the effort, posting an answer to try and help

Collectives™ on Stack Overflow

Generate random and if not exist in DB,add it

2 Answers 2

3 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related