Display usernames at random

Question

I’m trying to create a script for a user to enter in their username, and then have other logged in usernames randomly show, in a chatroulette fashion.

So, you will enter in your name and hit submit, then your name will be stored in a database and someone else’s name will be pulled out at random and shown. Then the user can hit a next button to see another random user name. When the user closes the page, their name will be unloaded from the system.

What I have tried is creating a simple post submission form which will return you to the same page logged in with your name, and it inserts your name into a mysql database. That worked.

Then I added some PHP code to detect that the name variable has been set and to find a random username in the database by finding the amount of users in the database and using a random integer to pick one out. I’m pretty sure it worked, however I was unable to get the user name to show with echo "$name";.

Then I tried adding an automatic logout by using:

That didn’t work. I didn’t get around to creating a next button because I was having a few problems, because I figured out that the logout wouldn’t work because I would be dropping rows from the database that wouldn’t be filled in again as new rows were added to the SQL database with an auto increment function causing blank pages to be shown.

Here is my code:



random name 



random name
By DingleNutZ

name:


";
}else{
$name = $_POST['name'];
$host="localhost"; // Host name 
$username="root"; // Mysql username 
$password=""; // Mysql password 
$db_name="ftr"; // Database name 
$tbl_name="players"; // Table name
// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect"); 
mysql_select_db("$db_name")or die("cannot select DB");

// To protect MySQL injection (more detail about MySQL injection)
$name = stripslashes($name);
$name = mysql_real_escape_string($name);

$sql="SELECT * FROM $tbl_name WHERE name='$name'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);


if($count==1){

session_register("name");
session_start();
if(session_is_registered(name)){
$players=mysql_query("SELECT MAX (id) FROM $tbl_name");
$chooserand=rand(1,$players);
$callee=mysql_query("SELECT name FROM $tbl_name WHERE id=$chooserand");
echo "$callee";
echo "Logout";
if (isset($playing)){
if ($playing == 1){
$drop_name=mysql_query("DELETE FROM $tbl_name WHERE name=$name");
}}
}
}
echo "show random name here";
}
?>

There is a variable in there called $playing which was an attempt at a logout system.

I would be very grateful for any answers. Many thanks in advance.

as i didnt make it obvious (sorry guys) i need to fix my main problem which is being able to show a random user without ever showing a blank page due to the rows being dropped from the database. it is essential that usernames are removed from the system for privacy

Johan · Accepted Answer

You have a few issues in your code, not all are errors as such, some code is unneeded, other code is potentially dangerous.

$name = stripslashes($name);              <<-- delete this line.
$name = mysql_real_escape_string($name);  <<-- this is all you need.

mysql_real_escape_string() is all you need. No other escaping is need to protect against SQL-injection.
A few caveats apply, which I will discuss below.

$sql="SELECT * FROM $tbl_name WHERE name='$name'";  
$result=mysql_query($sql);

Select * is an anti-pattern, never use it in production code. Explicitly select the fields you need.
You are using dynamic tablenames, I fail to see the need for this and it's also a dangerous SQL-injection hole.
Never use it but if you must, see this question how to secure your code: How to prevent SQL injection with dynamic tablenames?
You do the query, but you don't test if it succeeds, put a test in there:

$sql = "SELECT id FROM users WHERE name='$name' ";  
$result = mysql_query($sql);  
if ($result) 
{
  $row = mysql_fetch_array($result);
  $user_id = $row['id'];  
}
else { do stuff to handle failure }

You are trying to get data out of the database, but this is not the way to do it:

$players = mysql_query("SELECT MAX (id) FROM $tbl_name");                
$chooserand = rand(1,$players);                
$callee = mysql_query("SELECT name FROM $tbl_name WHERE id=$chooserand");                
echo "$callee";

But I see a few issues:
Please stop using dyname tablenames, it is a really bad idea.
The return value of mysql_query is a query_handle, not the actual data you're quering.
I would suggest escaping all values, whether from outside or inside your code; I know this is paranoid, but that way, if you code design changes, you cannot forget to put the escaping in.
Never ever ever echo unsanitized data in an echo statement.
If you echo a $var, always sanitize it using htmlentities. If you don't XSS security holes will be your fate.
See: What are the best practices for avoiding xss attacks in a PHP site

rewrite the code to:

$result = mysql_query("SELECT MAX (id) as player_id FROM users"); 
$row = mysql_fetch_array($result);
$max_player = $row['player_id'];               
$chooserand = mysql_real_escape_string(rand(1,$max_player)); 
//not needed here, but if you change the code, the escaping will already be there.
//this also makes code review trivial for people who are not hep to SQL-injection.               
$result = mysql_query("SELECT name FROM users WHERE id = '$chooserand' ");
$row = mysql_fetch_array($result);  
$callee = $row['name'];            
echo "callee is ".htmlentities($callee);

Finally you are deleting rows from a table, this looks like a very strange thing to do, but it is possible, however your code does not work:

$drop_name = mysql_query("DELETE FROM $tbl_name WHERE name=$name");

As discussed mysql_query does not return values.
On top of that only a SELECT query returns a resultset, a DELETE just returns success or failure.
All $vars must be quoted, this is a syntax error at best and an SQL-injection hole at worst.
Technically integers don't have to be, but I insist on quoting and escaping them anyway, because it makes your code consistent and thus much easier to check for correctness and it elimiates the chance of making errors when changing code
Rewrite the code to:

$drop_name = $name;
$result = mysql_query("DELETE FROM users WHERE id  = '$user_id' ");  
//user_id (see above) is unique, username might not be.
//better to use unique id's when deleting.
$deleted_row_count = mysql_affected_rows($result);  
if ($deleted_row_count == 0)
{ 
  echo "no user deleted"; 
} else {
  echo "user: ".htmlentities($drop_name)." has been deleted";
}

Display usernames at random

Answers (1)

Related Questions