-1
<?php include_once("database.php");

?>
<?php include_once("header.php");

?>



<?php 
    if ($_POST['submit'] )
    {
        $food_name = $_POST['food_name'];
        $food_calories = $_POST['food_calories'];
        echo $food_name . $food_calories;

        if (!empty($food_name) && !empty($food_calories) ) 
        {
            $query = 'INSERT INTO foods VALUES(0, $food_name, $food_calories)';
            mysqli_query($con, $query) or die(mysqli_error($con));
            echo 'added';   
        } else {echo'fail';}

    } else {echo'fa2oo';}


?>

  <h1> Please Fill out Form Below to Enter a New Food </h1>
  <form action="addfood.php" method="post">
    <p>Name:</p>
    <input type="text" name="food_name"/>
    <p>Calories:</p>
    <input type="text" name="food_calories"/> </br>
    <input type="submit" value="submit" />
  </form>

<?php include_once("footer.php")?>

Really don't understand why this simple insert is not working. The form is self-referencing. The form does not echo anything and simply resets when i hit the submit button. database connects with no errors.

Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • You accepted and unaccepted my answer. Just curious was something wrong with it? Didn't it worked for you? What is the reason? – Rizier123 Apr 30 '15 at 22:31

5 Answers5

2

Since you're inserting strings, you need to enclose them by single quotes ('):

$query = "INSERT INTO foods VALUES(0, '$food_name', $food_calories)";

Note, however, that building an SQL statement by using string manipulation will leave your code vulnerable to SQL inject attacks. You'd probably be better off using a prepared statement.

Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • The only correct answer here. (Sadly I'm out of votes for today :) (Ah just spotted something: OP is missing the name attr. for the submit button! And OP should also check if `$_POST['food_name']` is empty and not `$food_name`) – Rizier123 Apr 29 '15 at 22:54
  • +1 for the comment about SQL injection vulnerability. You can't actually use this code! It's TERRIBIBBLE! Search the web and learn about SQL parameterization in your applicable language/framework/db. – ctb Apr 29 '15 at 22:57
  • This isn't entirely true; You forget `.` before and after `'` – ops Apr 29 '15 at 23:01
  • Ah just forgot something: OP also has to use double quotes that the variables gets interpreted – Rizier123 Apr 29 '15 at 23:02
  • @Mureinik You still didn't mentioned some things: http://stackoverflow.com/questions/29955925/cant-get-simple-sql-insert-to-work#comment48032776_29955972 – Rizier123 Apr 29 '15 at 23:05
  • if you are worried about sql injection, see the prepared statement example using mysqli below. – nomistic Apr 29 '15 at 23:06
2

You have a few errors in your code:

1. Missing name attribute

You are missing the name attribute for your submit button. So add it like this:

<input type="submit" name="submit" value="submit" />
                   //^^^^^^^^^^^^^

2. Wong variables in empty()

You have to check if the $_POST variables are empty! Otherwise you would try to assign an undefined variable to another variable. So change your second if statement to this:

 if (!empty($_POST['food_name']) && !empty($_POST['food_calories']) ) 
          //^^^^^^^^^^^^^^^^^^^            ^^^^^^^^^^^^^^^^^^^^^^^

And also put the assignments inside the second if statement.

$food_name = $_POST['food_name'];
$food_calories = $_POST['food_calories'];

3. Wrong quotes + missing quotes

You have to use double quotes that your variable in the query gets parsed as variables. Also you have to put single quotes around them since they are strings, so change your query to this:

$query = "INSERT INTO foods VALUES(0, '$food_name', '$food_calories')";
       //^                            ^          ^  ^              ^

Side notes:

Add error reporting at the top of your file(s) to get useful error messages:

<?php
    ini_set("display_errors", 1);
    error_reporting(E_ALL);
?>

You may also want to change to mysqli with prepared statements since they are, much safer.

Rizier123
  • 58,877
  • 16
  • 101
  • 156
1

Here's the safe way of doing this using mysqli. Prepared statements will make sure you don't have (as high of) a risk of SQL injection

editing to include the connection:

$conn = new mysqli($host, $username, $password, $dbname);

If you want to see errors, you need to tell php to give the the errors. this should suffice:

if  ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

This part is how to do the query. Note the bind_param part; this is where you identify how your variables are going to into the database, and what datatype they need, so you don't need to remember which items to put in quotes in the actual query.

$query = $conn->prepare("INSERT INTO foods VALUES(0, ?, ?)");
$query->bind_param("si",$food_name, $food_calories);
$query->execute();
$query->close();

As mentioned before, $food_name is a string, so you specify it as such with the s in the bind_param and assuming that calories are an integer, they go in as i.

Another nice feature of using this approach is you no-longer need to worry about escaping variables; items in inputs go in exactly as they are entered

If you want more information in detail there's always this reliable source: http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

If you find this a bit too much, here's a great site to learn step by step how to use prepared statements from scratch (it also includes PDO but you may find it easier to use the mysqli at first and it still pretty good). http://www.w3schools.com/php/php_mysql_prepared_statements.asp

Have fun!

nomistic
  • 2,902
  • 4
  • 20
  • 36
  • I doubt it that OP knows how to create the connection with `mysqli_*` also these are not the only errors which OP has – Rizier123 Apr 29 '15 at 23:06
  • hey, it's a start, and it's really not hard to learn this way. :) – nomistic Apr 29 '15 at 23:07
  • Yes it's a start, but it's not finished. You don't have any reference where OP can learn about mysqli_* and also you don't converted OP's full code to mysql_*. Besides that OP has still other errors which you didn't mentioned. – Rizier123 Apr 29 '15 at 23:09
1

There are a few things wrong here.

Firstly, anything inside this conditional statement will not happen because of your submit button not bearing the "submit" name attribute.

if ($_POST['submit'] ){...}

However, it's best using an isset() for this.

if (isset($_POST['submit'] )) {...}

Modify your submit to read as:

<input type="submit" name="submit" value="submit" />
                     ^^^^^^^^^^^^^

Then, we're dealing with strings, so wrap the variables in your values with quotes.

Wrap your query in double quotes and the values in single quotes:

$query = "INSERT INTO foods VALUES (0, '$food_name', '$food_calories')";

Sidenote #1: If you experience difficulties, use the actual column names in which they are to go inside of.

I.e.: INSERT INTO table (col1, col2, col3) VALUES ('$val1', '$val2', '$val3')

Sidenote #2: Make sure that 0 for your column is indeed an int, however I don't know why you're using that.

If that column is an auto_increment, then replace the 0 with '' or NULL, should your schema accept it.

Now, should there be any character that MySQL may complain about, being quotes, etc., then you will need to escape/sanitize your data.

Say someone entered Tim's donuts in an input:
MySQL would translate that in your values as 'Tim's donuts', in turn throwing a syntax error.

  • Using mysqli_real_escape_string() for instance, would escape the apostrophe and render it as 'Tim\'s donuts' being a valid statement since it has been escaped.

  • Better yet, using prepared statements, as outlined below.


In its present state, your present code is open to SQL injection. Use prepared statements, or PDO with prepared statements, they're much safer.


Add error reporting to the top of your file(s) which will help find errors.

<?php 
error_reporting(E_ALL);
ini_set('display_errors', 1);

// rest of your code

Sidenote: Error reporting should only be done in staging, and never production.


Footnotes:

Given that we don't know which MySQL API you are connecting with, please note that different APIs do not intermix with each other.

For example:

  • You can't connect using PDO and querying with mysqli_
  • You can't connect using mysql_ and querying with mysqli_
  • etc. etc.

You must be consistent from A to Z, meaning from connection to querying.


Final closing note(s):

As stated by Rizier123, you are best using:

if (
    !empty($_POST['food_name']) 
    && 
    !empty($_POST['food_calories']) 
)
  • It is a better solution.
Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • 1
    This is the most complete answer here! (*meaning from connection to querying* And where is the connection close call? the closing must also use the same API :) – Rizier123 Apr 29 '15 at 23:30
0

Your issue (at least one of them) might be the SQL statement itself. Depending on the columns that you have in this foods table, you'll be required to specify the columns that you're inserting into. Try this:

INSERT INTO foods (col1, col2, col3) VALUES (val1, val2, val3)

Also, if val1 is supposed to be the ID column, you can't specify a value for that if it's auto-incrementing... the db will take care of that for you.

ctb
  • 1,212
  • 1
  • 11
  • 24
  • If OP has 3 columns in his table he doesn't have to define the column names! So his code would be completely fine – Rizier123 Apr 29 '15 at 22:53
  • 1
    I think I said that... "Depending on the columns that you have in this foods table..." – ctb Apr 29 '15 at 22:55