Deleteing SQL records via HTML form?

mufar

Hi,
I am new to coding using PHP. Im basically on the fast track learning as much as possible. I have been playing round, and got to the stage where i have a script, that inserts data in to a table. Upon completing that task, I want to take things a little further, and create a form, where i can enter the data to remove a record.

So going by what i learnt so far, using the insert.php as a guide, i attempted to create a form in the same manner using the following script for the form:

<!--START HTML REMOVE USER FORM-->
<html>
<body>
<br>
<br>
<br>
<u><b>User Account Management:</b></u>
<form action="delete.php" method="post">
Username: <input type="text" name="username" />
<br>
E-Mail Address: <input type="text" name="email" />
<br>
<br>
<input type="submit" name="remove" value="Remove Record!" />
</form>
<!--END HTML EDIT USER FORM-->

I then created a "delete.php" file with the following script:

<?php
$con = mysql_connect("localhost","***_***","**********");
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

mysql_select_db("hockley_apgame", $con);
mysql_query("DELETE FROM users WHERE username='$_POST[username]'");
mysql_query("DELETE FROM users WHERE email='$_POST[email]'");
if (!mysql_query($sql,$con))
  {
  die('Error: ' . mysql_error());
  }
echo "Account Successfully Removed!";

mysql_close($con)
?>

the table is called " users" and the database is called "hockley_apgame"

When I attempt to run that, i get the error message "Query is empty"
as you can see, im attempting to use the data from the form via $_POST.
But evidently its not being recognized. What am i missing, or am i going about this completely wrong?

Sorry if this seems like a stupid question like i said, im pretty new to scripting in PHP (Tho i feel i have made a little progress in my first 24hrs,on and off, of coding)

thx

=mufar=

mufar

ok, well i found that this does indeed delete the record i specify, but "Query is empty" error still comes up instead of the desired

echo "Account Successfully Removed!";

Leads me to belive that its trying to run a different Query after the initial deletion (The same thing also happened when i removed the second line:

mysql_query("DELETE FROM users WHERE email='$_POST[email]'");

Is there something i need to do to "kill" the Query before the echo statement?
i gotta get some sleep its almost 7:00am.

Look forward to some words of wisdom. 😉

Piranha

I would recommend a slightly different approach. The way you currently try to delete means that if something is misspelled (mufat instead of mufar) it will delete the wrong row. Instead I suggest that you first link to a page to show the user info, and there have a button to delete the user. A good idea is to store the userid in a session variable, and compare that to a variable that is sent when delete button is pressed. Then you make sure that the user don't just change the variable.

Another, and very important thing, is to make sure that you are not vunrable to SQL injection. There are 3 articles in phpbuilder that explains what it is and what to do about it, but basically I can say that it is a way for anyone to mess up your database. You should read the articles, they can be found here, here and here.

bradgrafelman

Some other things to note:

Array indeces in your case should be strings. $row[name] does not have a string as an index, but a constant. PHP will detect this (assuming there actually isn't a constant named 'name') and throw an E_NOTICE and then assume it is a string. So, use quotes: $row['name'].
Arrays, more specifically items in arrays referenced by indeces, should not simply appear inside a double-quoted string. If they do, they should be wrapped in braces ( {} ). Instead, I usually escape out of the string and concatenate the array in the middle, like so:
```
echo 'Your name is: ' . $row['name'] . '! Hello there.';
```
It appears as though you're vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query! Instead, sanitize it with a function such as [man]mysql_real_escape_string/man.
When posting PHP code, please use the board's [PHP][/PHP] bbcode tags - they make the code much, much easier to read (as well as aiding the diagnosis of syntax errors). You should edit your post now and add these bbcode tags to help others in reading/analyzing your code.

Piranha

bradgrafelman wrote:
Array indeces in your case should be strings. $row[name] does not have a string as an index, but a constant. PHP will detect this (assuming there actually isn't a constant named 'name') and throw an E_NOTICE and then assume it is a string. So, use quotes: $row['name'].

Arrays, more specifically items in arrays referenced by indeces, should not simply appear inside a double-quoted string. If they do, they should be wrapped in braces ( {} ).

Sorry to say m8, but that is wrong. As in the manual:

<?php
// These examples are specific to using arrays inside of strings.
// When outside of a string, always quote your array string keys
// and do not use {braces} when outside of strings either.

// Let's show all errors
error_reporting(E_ALL);

$fruits = array('strawberry' => 'red', 'banana' => 'yellow');

// Works but note that this works differently outside string-quotes
echo "A banana is $fruits[banana].";

// Works
echo "A banana is {$fruits['banana']}.";

// Works but PHP looks for a constant named banana first
// as described below.
echo "A banana is {$fruits[banana]}.";

// Won't work, use braces.  This results in a parse error.
echo "A banana is $fruits['banana'].";

// Works
echo "A banana is " . $fruits['banana'] . ".";

// Works
echo "This square is $square->width meters broad.";

// Won't work. For a solution, see the complex syntax.
echo "This square is $square->width00 centimeters broad.";
?>

Really stupid that they have two ways to handle arrays, one outside strings and another inside strings.

I have to say that I prefer to escape out the string as well as Brad.

NogDog

bradgrafelman wrote:
Some other things to note:
Array indeces in your case should be strings. $row[name] does not have a string as an index, but a constant. PHP will detect this (assuming there actually isn't a constant named 'name') and throw an E_NOTICE and then assume it is a string. So, use quotes: $row['name'].
Arrays, more specifically items in arrays referenced by indeces, should not simply appear inside a double-quoted string. If they do, they should be wrapped in braces ( {} ). Instead, I usually escape out of the string and concatenate the array in the middle, like so:
echo 'Your name is: ' . $row['name'] . '! Hello there.';

Actually (and hidden deep within the documentation to further confuse us all), within a quoted string $array[index] with no quotes is valid. It's an odd anomaly of PHP syntax. Outside of a quoted string, an unquoted array index string is treated as a constant, and if that constant is undefined then it is automatically defined to be the same as its name, and a notice-level error is generated. However, inside of a double-quoted string, an array variable not using complex (curly brace) notation and with an unquoted index, that index is treated as a string literal with no constants involved and no notice being generated. But if the array variable is expressed in complex (curly brace) notation within a quoted string, then the usual rules which would require the index to be quoted so as not to be interpreted as a constant would apply. (And if by chance you actually want to use a constant as the array index, then either complex notation or concatenation must be used.) It's all a bit inconsistent, but the end result is that the first three following examples are valid and equivalent. :rolleyes:

$myArray = array('foo' => 'Hello', 'bar' => 'World');
// valid:
echo "<p>$myArray[foo], $myArray[bar]</p>\n";
echo "<p>{$myArray['foo']}, {$myArray['bar']}</p>\n";
echo "<p>".$myArray['foo'].", "/$myArray['bar']."</p>\n";
// NOT valid (will generate notice-level errors):
echo "<p>{$myArray[foo]}, {$myArray[bar]}</p>\n";
echo "<p>".$myArray[foo].", "/$myArray[bar]."</p>\n";

mufar

Piranha wrote:
I would recommend a slightly different approach. The way you currently try to delete means that if something is misspelled (mufat instead of mufar) it will delete the wrong row. Instead I suggest that you first link to a page to show the user info, and there have a button to delete the user. A good idea is to store the userid in a session variable, and compare that to a variable that is sent when delete button is pressed. Then you make sure that the user don't just change the variable.

Another, and very important thing, is to make sure that you are not vunrable to SQL injection. There are 3 articles in phpbuilder that explains what it is and what to do about it, but basically I can say that it is a way for anyone to mess up your database. You should read the articles, they can be found here, here and here.

kk, fixed my original post. So what you are sayign is if i make a spelling mistake, it will delete some random account? even if there is no account with that name?
either way, your idea to have a page that has a delete button seems a little safer approach. tho not quite as straight forward and quick, when it comes to deleting accounts.

thx for the ideas, and ill check out them articles about sql injection.

mufar

bradgrafelman wrote:
Some other things to note:

[*]It appears as though you're vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query! Instead, sanitize it with a function such as [man]mysql_real_escape_string/man.

Ok, first off, a method like this for "deleting accounts" would be a strictly admin function. Therefore in a secured part of a site.
Would the security risk still be present, even if it required an admin (or trusted personel) to access?
tho the $POST method would have some nice uses and open up a lot of possibilities to the user (if it was somewhat safe).
Is there any way of indirectly putting user submitted data (via $POST) in to a query?

sorry for all the questions, but you guys certianly know what you are talking about, and are quite a help to a beginner like myself

PS: does anyone have a specific explanation for the reason im getting the error "Query empty" ? that would still be very helpful, even if its a wrong method. nice to know everything eh 😉

Piranha

No, of course the database won't delete a random account. But if there is an account with the other name it will be deleted. As you say, it might not be as straight forward. But these kind of things will always cause problems if you don't make sure that it will be checked, rechecked and checked again. Programming is like Murphy's law: If something can go wrong it will.

Piranha

mufar wrote:
Ok, first off, a method like this for "deleting accounts" would be a strictly admin function. Therefore in a secured part of a site.
Would the security risk still be present, even if it required an admin (or trusted personel) to access?
tho the $POST method would have some nice uses and open up a lot of possibilities to the user (if it was somewhat safe).
Is there any way of indirectly putting user submitted data (via $POST) in to a query?

sorry for all the questions, but you guys certianly know what you are talking about, and are quite a help to a beginner like myself

The only system that might work without security is a system that only the programmer himself/herself handles (and very carefully), and that in addition is not accessible from the outside. So yes, the risk will still be present. This is what the articles I linked before is about.

mufar

yeah, i like the idea. Im the kind of person that likes to follow "bruce lees law" .
that is, simplicity is the shortest distance between two points.
if anything, following your new idea will tech me a couple of new handy "tools" so ill do that anyways.
I am still completely curious as to what the initial error im getting is. "Query Empty" because as you can see, the query is not empty -.-

mufar

Piranha wrote:
The only system that might work without security is a system that only the programmer himself/herself handles (and very carefully), and that in addition is not accessible from the outside. So yes, the risk will still be present. This is what the articles I linked before is about.

kk, ill be checking out them articles.

Piranha

It is a new error for me. But the problem is probably that you have 2 queries after each other that may try to remove the same row. If the first removes the row, then the second won't be able to do it, and it is from the second you get the error.

mufar

Piranha wrote:
It is a new error for me. But the problem is probably that you have 2 queries after each other that may try to remove the same row. If the first removes the row, then the second won't be able to do it, and it is from the second you get the error.

yeah, i tried, with only one row present, and it still does the same thing, yet still executes the query and deletes the account.

NogDog

The error was caused by the 3rd line, below:

mysql_query("DELETE FROM users WHERE username='$_POST[username]'");
mysql_query("DELETE FROM users WHERE email='$_POST[email]'");
if (!mysql_query($sql,$con))

You do two queries by directly entering them as the argument to mysql_query(). Then in the third line you try to do another mysql_query(), but this time specifying the variable $sql as the query string, but you have not defined $sql anywhere in your code before that. What you probably really wanted was:

mysql_query("DELETE FROM users WHERE username='$_POST[username]'") or die(mysql_error());
mysql_query("DELETE FROM users WHERE email='$_POST[email]'") or die(mysql_error());

mufar

ahhhh, i c what you are saying. thx.
i gathered that is was trying to run another query.

if yoUr code is used, the following can be removed?:

{
  die('Error: ' . mysql_error());
  }

mufar

ok, that code now works as i originally intended. minus the problem that is always gives the "Account Deleted Successfully!" echo even if it doesn't delete anything. (if i enter invalid data)

now to tackle the problem of SQL injection vulnerabilities, and making a way of confirming deletion 😉

just out of curiosity, what could i enter in to the field for account deletion that would demonstrate the use of SQL injection?. So i have a better understanding of what exactly could be done to my database.

This is completly test, and will not be damaging a huge project or anything.

Piranha

I must be blind, should have spotted the error right away.

mufar, you could READ THE ARTICLES to understand it.

mufar

just got done reading the first Sql Injection article, love how at the end the abuser is refered to as "her. Completely breaking the stereotype (nerdy guy with glasses) is it not?

ps: no im not raggin on ppl with glasses, i wear em myself -.-

mufar

Piranha wrote:
mufar, you could READ THE ARTICLES to understand it.

ya, im a very impatient guy, can you tell?
lol just read the first one (in between posts/e-mail notifications)
making a little more sense.