0

I am using following php script to insert data in my MySQL database.But no rows are getting inserted..when I run same sql query in phpmyadmin its working fine.

<?php

if(isset($_POST['submit']) && isset($_POST['Pnr']) && isset($_POST['Mobile']))

{
    $hostname = "";
    $username = "";
    $password = "";
    $Pnr = $_POST['Pnr'];
    $Mobile = $_POST['Mobile'];

    $con = mysql_connect($hostname,$username,$password);

if(!$con)
{
    die("Could not connect to Database");
}

    mysql_select_db("freepnra_userinfo",$con);

    $sql = "INSERT INTO users (pnr,mobile) VALUES ('$Pnr','$Mobile')";

    mysql_query($sql) or die("insertion error");
}

?>  
Siddhesh
  • 472
  • 1
  • 9
  • 24
  • 1
    Are any of your errors appearing, such as 'could not connect to Database', or 'insertion error'.? – dotty Feb 25 '14 at 17:56
  • any errror messages? MySQL should be telling if something is wrong (duplicate keys, for example) – Barranka Feb 25 '14 at 17:56
  • my guess is that the first `if` does not equal true. you tried a `echo 'hello world'` in the if statement? – Félix Adriyel Gagnon-Grenier Feb 25 '14 at 17:56
  • Also, are we assuming that `$hostname`,`$username` and `$password` are set? – dotty Feb 25 '14 at 17:58
  • @dotty: I'd assume he just didn't include them in the question. – gen_Eric Feb 25 '14 at 17:58
  • 3
    Just a side-note, this code is *horribly unsafe* (what would happen if I sent `Pnr=',''); DROP TABLE users; -- ` to your page?). You should consider upgrading from the deprecated `mysql_*` methods and looking into "prepared statements". – gen_Eric Feb 25 '14 at 17:59
  • Have you ensured $Pnr and $Mobile are values? Or that the columns take NULL values if not? – xd6_ Feb 25 '14 at 17:59
  • 2
    Please, before you write **any** more SQL interfacing code, you must read up on [proper SQL escaping](http://bobby-tables.com/php) to avoid severe [SQL injection bugs](http://bobby-tables.com/) like the ones you have here. Also, `mysql_query` should not be used in new applications. It's a deprecated interface that's being removed from future versions of PHP. A modern replacement like [PDO is not hard to learn](http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/) and is a safer way to compose queries. `$_POST` data never goes directly in a query. – tadman Feb 25 '14 at 18:02
  • @dotty :No errors are shown – Siddhesh Feb 25 '14 at 18:03
  • 1
    @FélixGagnon-Grenier : the first if is not true..thanx. – Siddhesh Feb 25 '14 at 18:13
  • Try adding `ini_set('display_errors', 1); error_reporting(-1);` to the top of the page. – gen_Eric Feb 25 '14 at 18:14

3 Answers3

1

I am not completely sure what was the actual reason why it didn't work, but I suspect either SQL query nesting or issues with deprecated MySQL database extension.

I tested the following code and seems to be working flawlessly to me.

<?php
    if (isset($_POST['submit']) && isset($_POST['Pnr']) && isset($_POST['Mobile'])) {
        $hostname = "";
        $username = "";
        $password = "";
        $database = "freepnra_userinfo";
        $Pnr = $_POST['Pnr'];
        $Mobile = $_POST['Mobile'];

        $con = mysqli_connect($hostname, $username, $password, $database);

        if (mysqli_connect_errno()) {
            die("<p>Could not connect to database with the credentials passed in (" . mysqli_connect_errno() . ": " . mysqli_connect_error() . ").");
            exit(1);
        }

        $Pnr = preg_replace("/[^0-9-]/", "", $Pnr);
        $Mobile = preg_replace("/[^0-9-]/", "", $Mobile);
        $sql = "INSERT INTO `users` (`pnr`, `mobile`) VALUES (?, ?)";

        if ($stmt = mysqli_prepare($con, $sql)) {
            mysqli_stmt_bind_param($stmt, "ss", $Pnr, $Mobile);
            mysqli_stmt_execute($stmt);
            mysqli_stmt_close($stmt);
        }

        mysqli_close($con);
    }
?>

What I did, was that I nested the table name, the column names and instead of placing the variable values straight into the query, I placed double apostrophes.

I also switched to MySQLi database extension instead of your deprecated MySQL. In addition, I'd like to warn you of a very serious SQL injection problem and I fixed that issue by using prepared statements and some extra preg_replace function just to keep all unwanted characters out of the query and table in general. I am not entirely certain of what those values should contain, so I went on assuming you want two different phone numbers (which is why I went for only allowing numbers from 0 to 9 and dashes).

Additionally, I would like to refer you to read the following material to get to know some better ways of defending your queries against potential SQL injections:

I highly recommend switching to MySQL PDO if just possible. It's very simple, easy and works a lot better in my opinion!

Hopefully this helped you out!

Community
  • 1
  • 1
ascx
  • 473
  • 2
  • 13
  • 1
    Why would you use MySQLi without prepared statements? – gen_Eric Feb 25 '14 at 18:15
  • As the OP was using procedural style with `mysql_connect` and so on and so forth, I continued with the same style. Sorry, that I wasn't able to please everybody. – ascx Feb 25 '14 at 18:19
  • 1
    You can use prepared statements in procedural style: `mysqli_prepare()`. – gen_Eric Feb 25 '14 at 18:25
  • Two things: That link to [w3schools should be removed](http://www.w3fools.com), that site is one of the reasons Stack Overflow is littered with `mysql_query` questions, and secondly, it's like *two lines* more to add prepared statements and make this query air-tight. When you're using `mysqli_real_escape_string` you're doing it wrong. – tadman Feb 25 '14 at 18:29
  • Thank you for your comments, changed to prepared statements instead and removed that link ;-) All should be good now! – ascx Feb 25 '14 at 18:35
0

since your first if has no else statement, if it does not evaluates as true, you have no mean of knowing what happened. add a else statement at end of your script:

else {
echo 'problem with post data';
}

also, give yourself a huge plus and read about PDO. It has a way with prepared statements that overpasses mysqli I'd say, and you can do lots of things with the fetch modes pre built. that single line will give you all lines from in a table in a multi dimensional associative array, assuming $PDO is a valid PDO connection:

$rows = $PDO->query('SELECT * FROM table JOIN table2 USING (id)')->fetchAll(PDO::FETCH_ASSOC);

over which you can iterate with a foreach. coupled together, fetchAll and foreach will gain a lot of time over while ($row = mysqli_fetch_assoc())

foreach ($rows as $row) {
 // do whatever with your cool array of values
}
-1

I think you need to write your query as

$sql = mysql_query("INSERT INTO `users` (pnr,mobile) VALUES ('$Pnr','$Mobile')");