PHP delete values from mysql bad

Question

I have a script which works without errors, but can't delete chosen value from mysql. It looks like: What problem could be?

include('opendb.php');
$a = $_GET['new_pav'];
$select = mysql_query("SELECT * from naujiena WHERE new_pav = '$a'");
while($row = mysql_fetch_row($select)){
    $result = mysql_query("DELETE FROM `naujiena` WHERE new_pav='".mysql_real_escape_string($a)."' ");
}

You better check whether you get the `GET` parameter properly, and then whether you have a populated resultset as a result of the SELECT statement and finally, whether you ever get into the while loop and if yes, what statements are executed. — PCoder, Mar 08 '13 at 23:46
Please don't use the mysql_*-functions anymore. Why: read this http://stackoverflow.com/a/12860046/765805 — stUrb, Mar 08 '13 at 23:48
@stUrb It's quite astonishing that people are _still_ using it for new code despite the big shiny red warning box — Bojangles, Mar 08 '13 at 23:53
-1 since you have mentioned in one of the comments that you have code for mysqli_ library. — Ed Heal, Mar 09 '13 at 08:53
Can you explain everything *in your question* in future, rather than posting an incorrect answer and several comments with code. — Amelia, Mar 09 '13 at 11:19
@Bojangles *"If an organism has a choice between two things, the organism will do what it damn well pleases"* — Amelia, Mar 09 '13 at 11:23
@Hiroto I'm not sure what you mean by your quote. Are you saying that using deprecated functionality is ok, or that people will often blindly use whatever they are presented with first? — Bojangles, Mar 09 '13 at 14:40
@Bojangles it's a quote from jeff atwood about how no matter many times you tell someone something, they'll do what they feel like — Amelia, Mar 09 '13 at 14:55
Ah, now I see, thanks! I should have known that quote seeing as I read his blog `:P` — Bojangles, Mar 09 '13 at 15:06

score 1 · Answer 1 · edited May 25 '19 at 14:27

Firstly, read this (and below):

Please, don't use mysql_* functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO, or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial.

The red warning box is telling you to stop using mysql_* in anything new.

As for your query, DELETE FROM x WHERE y=z is a valid query, so the error could be from your use of quotes (if new_pav is an int, then this could explain it); strings are quoted in MySQL.

Also, do not interpolate/concat strings in an SQL query, or you risk SQL Injection. Look up pdo, and start using classes for something that involves a state (the db connection), rather than a variable and countless functions. (I originally used mysqli here):

try {
    $db = new PDO("mysql:dbname=$dbname;host=$dbhost", $dbuser, $dbpass);
    $query = $db->prepare("SELECT COUNT(*) FROM naujiena WHERE new_pav = :pav");

    if (!$query->bindParam(":pav", $_POST["new_pav"])) {
        die("Input incorrect; couldn't bind");
    }

    $query->execute();
    $rows = $query->fetchColumn(0); // fetch a single column. count(*) here.

    if ($rows !== 0) { // It has a result~
        $query = $db->prepare("DELETE FROM naujiena WHERE new_pav = :pav");
        $query->execute(array(":pav" => $_POST["new_pav"]));
    }
    $db = null; // explicitly close connection

} catch (PDOException $e) { // catch any exception PDO throws at you.
    // note that you should catch where appropriate.
    die("Connection Failed: " . $e->getMessage());
}

Note that with SQL Injection, I could type ' OR 1=1 -- and delete your whole table.

As you can see, this is far from a one/two-liner, but you must never trust anything added to SQL that you didn't hardcode in yourself, period.

But i need to get an accurate value from database and delete it ;/. For example if i have a new with 3 column such as: 1 New_Name Istrinti(Delete) so when i click istrinti a new deletes from database. — user2149578, Mar 09 '13 at 07:59

score 0 · Answer 2 · answered Mar 09 '13 at 08:29

0

Apart from using mysql_ libraries your code:

$select = mysql_query("SELECT * from naujiena WHERE new_pav = '$a'");
while($row = mysql_fetch_row($select)){
    $result = mysql_query("DELETE FROM `naujiena` WHERE new_pav='".mysql_real_escape_string($a)."' ");
}

In the SELECT you are not escaping the value of $a but in the delete you are escaping it.

Anyway if you are just doing a delete you do not need the SELECT or while loop. So you could use the following code:

$result = mysql_query("DELETE FROM `naujiena` WHERE new_pav='".mysql_real_escape_string($a)."' ");

answered Mar 09 '13 at 08:29

Ed Heal

59,252
17
87
127

0 down vote accept $db = new mysqli($dbhost, $dbuser, $dbpass, $dbname); if(isset($_GET['new_pav'])) { $result = $db->prepare("DELETE FROM `naujiena` WHERE new_pav= ? "); $result->execute(); $result->close(); if($result) { echo "succces"; } } else { die("Nepavyko"); } Check it out – user2149578 Mar 09 '13 at 08:48
@user2149578 - I did mention not using mysql_ libraries but fixed your code that uses those deprecated libraries. If you know that these libraries are deprecated why post the code and not code that uses mysqli_ library? – Ed Heal Mar 09 '13 at 08:53

PHP delete values from mysql bad

2 Answers2