0

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.

Improve this question
10
  • 4
    You're open to SQL injection!! This is a huge security risk! Don't use your code in a real life application!! Commented Apr 7, 2018 at 17:41
  • So, what error you've got? Also, look carefully to your query in $sql, you have a problem with quotes Commented Apr 7, 2018 at 17:43
  • 4
    Don't put user supplied data into SQL queries like that. Use parameterized queries, always. Commented Apr 7, 2018 at 17:45
  • 5
    @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. Commented Apr 7, 2018 at 17:48
  • 1
    '$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. Commented Apr 7, 2018 at 17:50

2 Answers 2

Reset to default
3

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.
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

You did cover a lot here and was the reason why I upvoted, but missed something that everyone overlooked. I posted this comment about it under their post.
2

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;
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.