Add and retrieve to mysql

Question

I am building a small php application where you can add people and then see them on a page. When I was simply adding it went fine, but then I started using a switch and now it doesn't work to either add or retrieve. I cannot see any problem in my syntax, can anyone see something wrong?

php

<?php

$con = mysql_connect("hostWasHere","username","password");
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

mysql_select_db("dbIsHere", $con);

try{

    switch($_POST['action'])
    {
        case 'retrieve':
            $show=mysql_query("Select * from test",$con);

     while($row=mysql_fetch_array($show)){
        echo "<li><b>$row[firstName]</b> : $row[lastName]</li>";
     }
        mysql_close($con);
            break;

        case 'new':
            $sql="INSERT INTO test (firstName, lastName)
VALUES
('$_POST[fname]','$_POST[lname]')";

if (!mysql_query($sql,$con))
  {
  die('Error: ' . mysql_error());
  }
echo "1 record added";

mysql_close($con);
            break;
    }

}




?>

The javascript using this is :

function saveToServer() {
alert("clicked");
$.post("api.php", {
        'action': "new",
        'fname': $('#fname').val(),
        'lname': $('#lname').val()
    },
    function () {
        alert("succes");
    }
);

}

function getFromServer() {
  console.log("in get!");
  $.ajax({
    type: "post",
    url: "api.php",
    data: "action=retrieve",
    success: function (data) {
        $("#comment").html(data);
        console.log("success!");
    }
  });

}

Add error reporting to the top of your file(s) `error_reporting(E_ALL); ini_set('display_errors', 1);` - Plus, your present code is open to [**SQL injection**](http://stackoverflow.com/q/60174/). Use [**`mysqli_*` with prepared statements**](http://www.php.net/manual/en/mysqli.quickstart.prepared-statements.php), or [**PDO**](http://php.net/pdo) with [**prepared statements**](http://php.net/pdo.prepared-statements). — Funk Forty Niner, Jul 03 '14 at 12:54
Indent the code, it often reveals obvious errors. Don't use mysql_* since it's deprecated and won't be supported in the future. Use prepared statements, someone (anyone) could drop your database after only 1 minute research of how to. Are there any bugs in your developer tools? — Jonast92, Jul 03 '14 at 12:55
`data: "action=retrieve",` in your ajax call should be `data: {'action':'retrieve'},`. And if you add a default case to switch, you will get the answer :) — Think Different, Jul 03 '14 at 12:57
I will use mysqli and prepared statements, thanks for the tip, I also added the error reporting and I get nothing. Also added case default to echo "skipped to default" and I don t get that either. It does make the request to php because I see in the network of developer tools and no I have no bugs in the developer tools. I also modified the data : action=retrieve thing, thanks Think Different — user3479297, Jul 03 '14 at 13:09

ljacqu · Accepted Answer · 2014-07-03T19:42:55.033

You are using a try block without any catch or finally – this doesn't work. Most likely, your server is configured not to output any errors, so it dies silently.

A few other remarks:

As pointed out in the comments, please use PDO or MySQLi instead of the deprecated MySQL class.
Beware of SQL injection and always sanitize properly, no excuses. (My code below with PDO uses prepare and takes care of this.)
Use quotes when you're accessing an array with a string as key: $_POST['fName'] or $row["lName"], as opposed to $row[lName].
Output all errors while you're developing your page by adding error_reporting(E_ALL) at the top of your file. Note that server settings may still suppress the error output, but this generally takes care of everything.
Using a switch statement with a lot of code is never a good idea; you want to keep all code there rather lightweight or switch to a combination of if, else if and else.

Enough talk. Here's my edit for your page, using PDO instead of the deprecated MySQL family.

<?php
error_reporting(E_ALL);

// PDO has more options to read about
// for initialization, but this should do for now
$con = new PDO("host=host;dbname=db_here", "username", "password");

if (!$con) {
    die('Could not connect: !');
}

// Do some validation on $_POST before using it.
$action = '';
if(isset($_POST['action'])) {
    $action = $_POST['action'];
}

if($action == 'retrieve') {
    $sql = $con->execute('SELECT * FROM test');
    $rows = $sql->fetchAll(PDO::FETCH_ASSOC);

    foreach($rows as $row) {
        echo '<li><b>'.$row['firstName'].'</b> : '.$row['lastName'].'</li>';
    }
    $con = null;
}
else if($action == 'new') {
    $sql = $con->prepare('INSERT INTO test (firstName, lastName)
                            VALUES (?, ?)');

    // TODO: more checks on fname and lname before accepting
    if(isset($_POST['fname']) || isset($_POST['lname'])) {
        $result = $sql->execute( array($_POST['fname'], $_POST['lname']) );
        if(!$result) {
            die('Error occured');
        }
        else {
            echo 'Added 1 row';
        }
    }

    $con = null;
}
else {
    // TODO: Default page
}

PS: Please don't ever trust user input. The code is still inserting $_POST values rather blindly (just checking that they're at least set), further checks with is_scalar() and some length checks would probably be good.

I hope this can help – good luck with your project!

thank you so much, not only is this a great example but all of your notes are of huge help to me. I agree with every point you made, as for the large amount of code in the switch, I was eventually going to make a class with it's own methods and just call them there. For the user input, I'll do all the checks in the frontend I think, but again, great tips — user3479297, Jul 03 '14 at 15:41

Add and retrieve to mysql

1 Answers1