1

I have a form that I am using to add information to a database, but the query will not run properly and the information is not being added at all.

This is the php code I am trying to execute, but it keeps hitting the first error or completely going to the last else statement:

<?php       
    if (!isset ($_POST['submit']))
    {
        include ("add_contact.php");
    }

    else 
    {
        $name = $_POST['Name'];
        $phone_num = $_POST['main_num'];
        $sec_num = $_POST['sec_num'];
        $email = $_POST['email'];
        $cus_type=$_POST['cusType'];
        $business = $_POST['business'];
        $address = $_POST['address'];
        $service = $_POST['service'];
        $notes = $_POST['comment'];


        include ("dbcon.php");
        fopen ("dbcon.php", "r"); //used because of bad experiences prior with include() only

        if ($cus_type == 'Corporate'){
            $result = mysqli_query($connect,"INSERT INTO customers WHERE ('$name', '$phone_num', '$cus_type', '$sec_num', '$email', '$address', '$business', '$service', '$notes') ");
            if ($result){
                echo "Thank you for adding the Contact to the Database!";
            }
            else{
                echo "ERROR: Corporate Customer Not Added to Database";
            }

        }
        else if ($cus_type == 'Private'){
            $result = mysqli_query($connect,"INSERT INTO private_customers WHERE ('$name', '$phone_num', '$cus_type', '$sec_num', '$email', '$address', '$business', '$service', '$notes') ");
            if ($result){
                echo "Thank you for adding the Contact to the Database!";
            }
            else{
                echo "ERROR: Private Customer Not Added to Database";
            }
        }
        else{
            echo "Contact Invalid. Please Try Again.";
        }
    }
?>

Any comments or answers would be helpful to spot what is going wrong in my code. Also just a note, this is for an internal website for a company and no one here (besides the one who told me to make this) knows MYSQL and PHP.

arrow99
  • 13
  • 5
  • 4
    Invalid sql syntax, sql injection and why would you open your database connection file with `fopen()`? – jeroen Aug 20 '14 at 16:57
  • When using `mysqli` you should be using parameterized queries and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation to accomplish this because you have created severe [SQL injection bugs](http://bobby-tables.com/). `$_POST` data **NEVER** goes directly into a query. – tadman Aug 20 '14 at 16:58
  • @Mihai is correct. Should be: INSERT INTO customers VALUES ('$name', '$phone_num', '$cus_type', '$sec_num', '$email', '$address', '$business', '$service', '$notes') // assuming you are validating and sanitizing (escaping) ALL data before insert – Pagerange Aug 20 '14 at 16:58
  • 3
    @Mihai Please don't [link to w3schools](http://w3fools.com/). That site is full of obsolete advice and toxically bad examples. It does more harm than good, especially in the PHP world. That example advocates doing manual escaping which is prone to failure. – tadman Aug 20 '14 at 16:59
  • @Pagerange you should not tell the OP to use a query that is vulnerable to sql injection – John Ruddell Aug 20 '14 at 17:01
  • @tadman While I generally agree about w3fools,I thought that the mysql docs for INSERT would be over his/her head with that schema diagram. – Mihai Aug 20 '14 at 17:02
  • 1
    @Mihai It's better to be thirsty than to drink poisoned water. – tadman Aug 20 '14 at 17:04
  • @john-ruddell Noted. First instinct was merely to point out the incorrect syntax. – Pagerange Aug 20 '14 at 17:10
  • Could someone use simplistic words since I am still new to PHP coding and would like to understand what I am doing wrong? @tadman Please tell me where you see the $_POST going directly into my query? Because I'm not seeing what you are seeing. – arrow99 Aug 20 '14 at 17:40
  • @arrow99 look at http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php... create your connection like this. $mysqli = new mysqli("server", "username", "password", "database_name"); – John Ruddell Aug 20 '14 at 17:42
  • @JohnRuddell Thank you for clearing that up. I was confused as to what was being talked about. – arrow99 Aug 20 '14 at 17:45
  • @arrow99 `$cus_type` is assigned directly from `$_POST` and then jammed right into your query. That's the textbook definition of a SQL injection bug. – tadman Aug 20 '14 at 17:46
  • @tadman How else would you place a condition on something if you didn't know what the user put in, but wanted to check it? If they put anything else in, it would still run an error because of my checks I put on it. – arrow99 Aug 20 '14 at 17:51
  • @arrow99 why are you doing an assignment in your if statement for your $cus_type? – John Ruddell Aug 20 '14 at 17:54
  • You can check it, that's always fine, but look at John's answer for how to properly insert data into your database. Use the `bind_param` method when handling arbitrary user data. – tadman Aug 20 '14 at 17:54
  • @arrow99 look at my EDIT2: in my answer... you need to fix your if statement – John Ruddell Aug 20 '14 at 18:02

2 Answers2

3

you have invalid insert syntax this is the valid syntax

INSERT INTO customers (field1, field2) VALUES (val1, val2);

SEE DOCUMENTATION

also you have a serious sql injection vunerability.. you should look HERE for help on that

I would recommend you use parameterized queries and prepared statements... this SO POST covers it nicely

EDIT:

just so i'm not only providing a link only answer here is a sample of what you should do

$mysqli = new mysqli("server", "username", "password", "database_name");

if (mysqli_connect_errno()) {
    printf("Connect failed: %s\n", mysqli_connect_error());
    exit();
}
$qry = $mysqli->prepare('INSERT INTO customers (name, phone, type, section, email, address, business, service, notes) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)');
$qry->bind_param('s', $name, $phone_num, $sec_num, $email, $cus_type, $business, $address, $service, $notes);

// can do it in one statement rather than multiples..
//$qry->bind_param('s', $name);
//$qry->bind_param('s', $phone_num);
//$qry->bind_param('s', $sec_num);
//$qry->bind_param('s', $email);
//$qry->bind_param('s', $cus_type);
//$qry->bind_param('s', $business);
//$qry->bind_param('s', $address);
//$qry->bind_param('s', $service);
//$qry->bind_param('s', $notes);

$qry->execute();
$qry->close();

EDIT2:

you must be new to programming.. your if() statement will ALWAYS get executed... meaning you are always going to insert into the database.. this is why..

if ($cus_type = $_POST['Corporate']){ here $cus_type is equal to something else aka $_POST['cusType'] but in the if statement you are assigning it to $_POST['Corporate']... which will always execute because its a true statement.. this is how if statements get executed logically..

if(boolean statement){
    //executes when true
};

if(true){
    //always executes
};

if('a' == 'b'){
    //will not execute
};

$a = 'a';
$b = 'b';
if($a == $b){
    //will not execute
};

if($a = $b){
    //will always execute because its assigning the value which is a boolean true statement.
};
Community
  • 1
  • 1
John Ruddell
  • 25,283
  • 6
  • 57
  • 86
  • 2
    You should always spell out what columns you're inserting into even if it's all of them because schemas change. It's better to have an error on insert than for data to go into the wrong columns. – tadman Aug 20 '14 at 17:05
  • @tadman that is true.. i just pointed it out as it is an operation MySQL supports – John Ruddell Aug 20 '14 at 17:06
  • 1
    Nice update with a demonstration of `bind_param`. Yeah, all it takes is someone to accidentally re-order two columns on your production database and it's a whole world of hurt to undo the damage. If you spell out the column names, either it inserts correctly or produces an error, it never just puts it in arbitrarily. – tadman Aug 20 '14 at 17:47
  • @JohnRuddell My code is running an error when it gets to the bind_param() function. This is the error it runs: 'Fatal error: Call to a member function bind_param() on a non-object in submit_contact.php on line 71' – arrow99 Aug 20 '14 at 18:13
  • @arrow99 this is probably because you do not have your connection set up properly to the database.. you should change it.. look at that link i posted.. i also just edited with a sample way to connect to your database – John Ruddell Aug 20 '14 at 18:14
  • @JohnRuddell Thank you. I am fairly new to PHP and MYSQL. I did change the if-statement. I keep forgetting that '=' is an assignment sign not an actual 'equal to this' sign. I will try the new way to connect to my database. – arrow99 Aug 20 '14 at 18:18
  • @JohnRuddell I am still getting that error, even with trying to connect to the database directly from the file instead of including a file. – arrow99 Aug 20 '14 at 18:26
  • @arrow99 look at this example (example 1) http://php.net/manual/en/mysqli-stmt.bind-param.php – John Ruddell Aug 20 '14 at 19:01
0

You can also specify your insert using and update like syntax which some find more readable.

INSERT INTO customers
SET
    field1   = value1
    , field2 = value2
    , field3 = value3

et cetera...

Michael MacDonald
  • 708
  • 1
  • 6
  • 24