PHP Insert Into Error

Question

I am started to learn coding start with HTML, CSS, and php. I created a basic form to test my skill. However, I got stuck with this. Can you help me on that?

I know that it is open to SQL injections, I am just trying to improve myself in coding and will use prepared statements and parameterized queries in real life.

<?php

if ($_SERVER["REQUEST_METHOD"] == "POST") {

    $mysql_host = "";
    $mysql_username = "";
    $mysql_password = "";
    $mysql_database = "";

    $conn = new mysqli ($mysql_host, $mysql_username, $mysql_password, $mysql_database);

    $c_name = $_POST["club_name"]; 
    $c_league = $_POST["league"];
    $c_rank = $_POST["ranking"];
    $c_prank = $_POST["previous_rank"];

    $sql = "INSERT INTO `club_data` (`club_name`, `league`, `ranking`, `previous_rank`)
    VALUES ('$c_name', '$c_league, $c_rank, $c_prank);";
    mysqli_query($conn, $sql); 

    if ($conn->query($sql) === TRUE) {
        echo "kayit islendi";
    }
    else {
        echo "Error". $sql ."<br>". $conn->error;
    }
    $conn->close();
}
?>

List Names

Everytime I used the form I got this error.

ErrorINSERT INTO... etc.

You're open to SQL injection!! This is a huge security risk! Don't use your code in a real life application!! — cramopy, Apr 07 '18 at 17:41
So, what error you've got? Also, look carefully to your query in `$sql`, you have a problem with quotes — Evgeny Ruban, Apr 07 '18 at 17:43
Don't put user supplied data into SQL queries like that. Use [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php), always. — Peter, Apr 07 '18 at 17:45
@Awaisfiaz We all were beginners once, and we (hopefully) all wish more people had told us how to avoid a catastrophe. If you see a new driver driving with no hands on the wheel, hopefully you would say something. This is the same kind of problem. — elixenide, Apr 07 '18 at 17:48
As others have pointed out, you are wide open to [**SQL injection**](https://www.owasp.org/index.php/SQL_Injection). You need to use prepared statements, rather than concatenating variables into your query. See [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1). — elixenide, Apr 07 '18 at 17:48
@EdCottrell i respect your views and appreciate your effort for helping him/her. — Awais fiaz, Apr 07 '18 at 17:50
`'$c_name', '$c_league, $c_rank, $c_prank` look at your quotes here. Better yet, use a prepared statement and you won't have to think about your quotes in the query. — Qirel, Apr 07 '18 at 17:50
you went and edited the question after someone posted an answer about unquoted variables, why is that and why haven't you commented under the answer(s) about it? @sinan — Funk Forty Niner, Apr 07 '18 at 23:47
I know why this failed. I'll give you a hint: think of the number 2 and I'll give you another number. — Funk Forty Niner, Apr 07 '18 at 23:48
I edited the quotes in the code because it wasnt the reason, it was just typo I did while i was writing this post. And guys why are you so aggressive on your comments, I am here just to learn from you? — sinan, Apr 08 '18 at 12:04

Paul G Mihai · Accepted Answer · 2018-04-07T18:25:06.133

You are missing quotes around your insert values, here's the fixed sql:

$sql = "INSERT INTO `club_data` (`club_name`, `league`, `ranking`, `previous_rank`)
VALUES ('$c_name', '$c_league', '$c_rank', '$c_prank');"

You were missing quotes around each value!

HOWEVER, this is an ill advised way of making database queries in production. Either use mysqli_real_escape_string to sanitize your strings(each of your variables will need this treatment) or use prepared statements.

Alternatively, and the way you should always use your DB is via the PDO wrapper. In this case you would use: PDO::quote. PDO offers a unified interface to the most popular databases there are. Here you can read more about PDO: http://php.net/manual/en/book.pdo.php

Coders prefer prepared statements to sanitizing their input. However this incurs extra communication with the mysql server vs writing a bit more code in php. Prepared statements are more involved then normal queries as they are cached on the SQL server and preprocessed waiting for data to be used, also having a miriad of question marks makes the code very hard to read especially if you start working in production and have a miriad of columns to fill. Here you can read more about the prepared statements: https://dev.mysql.com/doc/refman/5.7/en/sql-syntax-prepared-statements.html

Main takeaway:

never, EVER, EVER save unsanitized data to the DB!!Use mysqli_real_escape_string or PDO::quote or prepared statements, depending on situation.
use prepared statements for what they have been created for not just as a wholesale sanitizer tool, use them when you have to execute the same query repeatedly. Especially if this query is not an insert in which case I suggest you do mass insert like so:INSERT INTO tbl_name (a,b,c) VALUES(1,2,3),(4,5,6),(7,8,9); read more here: https://dev.mysql.com/doc/refman/5.7/en/insert.html This has a caveat in that the maximum size of the sql with inserted values should never be larger then max_allowed_packet config.

You did cover a lot here and was the reason why I upvoted, but missed something that everyone overlooked. I posted [this comment](https://stackoverflow.com/questions/49710151/php-insert-into-error#comment86439556_49710151) about it under their post. — Funk Forty Niner, Apr 07 '18 at 23:53

score 2 · Answer 2 · answered Apr 07 '18 at 17:50

You should use prepared statements. Not only does it prevent SQL injection attacks, it also avoids the pesky quoting issues you are currently facing

$sql = "INSERT INTO `club_data` (`club_name`, `league`, `ranking`, `previous_rank`)
VALUES (?, ?, ?, ?);";

$result = $conn->prepare($sql);
$result->bind_param('ssss', $c_name, $c_league, $c_rank, $c_prank);
echo $result->execute() === true ? 'kayit islendi' : 'Error'.$conn->error;

PHP Insert Into Error

2 Answers2