What's wrong with MySQL?

Question

INSERT doesn't work, not sure why.

db.php:

function check($value)
{
    if ($value)
        return htmlspecialchars(stripslashes(trim($value)));
    else
        return null;
}

$db_host = 'localhost';
$db_user = 'root';
$db_pass = '';
$db_name = 'cart';

$connect = mysql_connect($db_host, $db_user, $db_pass) or die ('no connect');
mysql_select_db($db_name) or die ('no db');

function add($name, $price, $description, $image)
{
    global $connect;

    $sql = "INSERT INTO items (name, price, description, image)
            values ('{$name}', '{$price}', '{$description}', '{$image}');";
    $query = mysql_query($sql, $connect);

    if (!$query)
        die(mysql_error());

    mysql_close($connect);
}

add.php:

header('Content-Type: text/html; charset=utf-8');
mb_internal_encoding('utf-8');
include_once('./db.php');

$name = check($_POST['name']);
$price = check($_POST['price']);
$description = check($_POST['description']);
$image = check($_POST['image']);

if ($name && $price && $description && $image)
{
    add($name, $price, $description, $image);
    echo 'sent';
}

When I get sent message, I've no new rows in items table. I check it using phpmyadmin.

DB is created using this code (by install script):

function reset_mysql()
{
    global $connect;

    $sql = "CREATE TABLE items (
        id INT AUTO_INCREMENT NOT NULL,
        name VARCHAR(50) NOT NULL,
        price VARCHAR(10) NOT NULL,
        description VARCHAR(200) NOT NULL,
        image VARCHAR(30) NOT NULL,
        primary key (id)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8;";

    $query = mysql_query($sql, $connect);

    if (!$query)
        die(mysql_error());

    mysql_close($connect);
}

What's wrong?

What's the data type of each? Maybe `price` is numeric and doesn't need quotes? BTW, look into *script injection attacks* when you have some free time. — Mike Christensen, Dec 14 '12 at 22:27
On a side note, you should be using `mysqli_*` now instead of `mysql_*` — jamis0n, Dec 14 '12 at 22:27
You are using [an **obsolete** database API](http://stackoverflow.com/q/12859942/19068) and should use a [modern replacement](http://php.net/manual/en/mysqlinfo.api.choosing.php). You are also **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that a modern API would make it easier to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. — Quentin, Dec 14 '12 at 22:28
`$name && $price && $description && $image` => one of those equates to false most likely. — Wrikken, Dec 14 '12 at 22:28
You also need to be using `mysql_real_escape_string()` on those variables. That said you shouldnt be using that mysql api at all as Quentin mentioned. — prodigitalson, Dec 14 '12 at 22:31
@Mike Christensen I use one more function to prevent injections, it gives no error. The most important part is to make this code work, price would be translated into float then. — Mark, Dec 14 '12 at 22:31
Love your title... But you'd better ask what is *right* with MySQL — , Dec 14 '12 at 22:32
@Wrikken but a get `sent` message after validation. Seems the dark is on the other side. — Mark, Dec 14 '12 at 22:36
I've added a function I use to prevent injections, its name is `check` — Mark, Dec 14 '12 at 22:36
Thanks for reminding me about the obsolete api. Would use them when I understand what's wrong with the current code. I would like to know how can I make it work. — Mark, Dec 14 '12 at 22:42
@JonathanLonowski I wasn't talking to you specifically. Just saying, web dev moves so quickly it's hard for those who just learned one thing to find out they have to learn a whole new thing. — bobthyasian, Dec 14 '12 at 22:42
@Mike Brant I get no errors, even after `INSERT INTO`, too strange. — Mark, Dec 14 '12 at 22:43
What does `$query`returns ? Are you sure `price` column is a `string` ? — Ricardo Alvaro Lohmann, Dec 14 '12 at 22:47
@bobthyasian: Its not a high horse thing. You shouldnt be using it for new dev even if it wasnt deprecated. PDO and Mysqli are superior and easier to work with once you know them. He shouldnt have even learned ext/mysql, but until google starts yielding on mysqli or PDO tutorials when people search its going to be a rough battle. Mentioning its deperecated and what not as much as possible is part of fighting that battle. — prodigitalson, Dec 14 '12 at 22:49
I test it using `something` as a value for all the `$_POST` variables. No error, no new row in the `items` table. — Mark, Dec 14 '12 at 22:50
@Steve Do you actually have `error_reporting` turned on? Is `display_errors` on? — Mike Brant, Dec 14 '12 at 22:52
@Steve - Try adding `COMMIT;` to the end of your SQL statement? — Mike Christensen, Dec 14 '12 at 22:55
add('something', 'something', 'something', 'something'); =>>> Warning: mysql_query(): 4 is not a valid MySQL-Link resource in V:\home\dev\www\Steve\db.php on line 23 — Mark, Dec 14 '12 at 22:56
Not sure where do I got `4`, there is no such number in the code. — Mark, Dec 14 '12 at 22:57
If you're already connected to the DB and selected table, you don't need to specify `$connect` in your query. — bobthyasian, Dec 14 '12 at 22:58
@bobthyasian I replaced `$connect` to the `add` function and it began to work. Thanks! — Mark, Dec 14 '12 at 23:09
Seems that `mysql_close($connect)` inside a function doesn't work nice with the global `$connect`. — Mark, Dec 14 '12 at 23:10

bobthyasian · Answer 1 · 2012-12-14T22:51:05.717

0

$sql = "INSERT INTO items VALUES ('', '$name', '$price', '$description', '$image')";

UPDATED

edited Dec 14 '12 at 22:51

answered Dec 14 '12 at 22:30

bobthyasian

933
5
17

3

That is good coding style advice, but what he has is not wrong. – Mike Brant Dec 14 '12 at 22:34
INSERT INTO items (name, price, description, image) values (sometext, sometext, sometext, sometext);>>Unknown column 'sometext' in 'field list' for values ($name, $price, $description, $image) – Mark Dec 14 '12 at 22:47
No errors for values ('{$name}', '{$price}', '{$description}', '{$image}') – Mark Dec 14 '12 at 22:48
Same result, no new rows in `items` table – Mark Dec 14 '12 at 22:48
@MikeBrant There is a problem with using just quotes when you are trying to use variables that are multidimensional arrays eg. one_array['name'][$i] – IROEGBU Dec 14 '12 at 23:00

score 0 · Answer 2 · answered Dec 14 '12 at 23:08

Finally fixed by replacing mysql_connect to the add function:

function add($name, $price, $description, $image)
{
    global $db_host, $db_user, $db_pass, $db_name;

    $connect = mysql_connect($db_host, $db_user, $db_pass) or die ('no connect');
    mysql_select_db($db_name) or die ('no db');

    $sql = "INSERT INTO items (name, price, description, image)
            values ('{$name}', '{$price}', '{$description}', '{$image}');";
    $query = mysql_query($sql, $connect);

    if (!$query)
        die(mysql_error());

    mysql_close($connect);
}

Thanks for all of the comments, you're awesome!

do move onto mysqli_ ASAP... rewrite your db.php file to be more reusable, this file is going to be a mess if need to run many queries. — IROEGBU, Dec 14 '12 at 23:12

What's wrong with MySQL?

2 Answers2