0

I'm trying to add data to my mysql database using a php GET method. Though when open the url for test purposes T get strange errors.

this is my code: 

<?php
$servername = "bernd-mysql.php-friends.de";
$username = "521_admin";
$password = "*****";
$dbname = "521_rfid_test";

// Create connection
$conn = mysqli_connect($servername, $username, $password, $dbname);

// Check connection
if (!$conn) {
        die("Connection failed: " . mysqli_connect_error());
}
echo "Connected successfully";
echo "<br/>";

$var1=$_GET["value1"];
$var2=$_GET["value2"];

echo $var1;
echo "<br/>";
echo $var2;
echo "<br/>";

$query = "INSERT INTO `accounts` (`id`, `firstName`)
VALUES ('".$var1."','".$var2."')";

if (mysqli_query($conn, $query)) {
    echo "New record created successfully" . "<br />";
} else {
    echo "Error: " . $query . "<br>" . mysqli_error($conn);
}
#mysql_query($query,$conn);
mysql_close($conn);
?>

When i open the url with ?value1=00001&value2=9999 attached i get the following output.

Connected successfully
00001
9999
Error: INSERT INTO accounts (id, firstName) VALUES ('00001','9999') $query,$conn); mysql_close($conn); ?>

I'm pretty new to php and mysql so please bear with me :)

Alexander Hörl
  • 578
  • 4
  • 11
  • 24
  • use **mysqli_close($conn);** instead of **mysql_close($conn);** – Daniyal Mar 28 '18 at 10:29
  • What data type is `id` and `firstName`? And if you ever think of deploying your app, read about SQL Injection first. – Nkole Evans Mar 28 '18 at 10:31
  • 1
    **Danger**: You are **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that you need to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Mar 28 '18 at 10:32
  • As you connect with `mysqli`, you must close the connection using `mysqli` and not `mysql`.. – Yash Parekh Mar 28 '18 at 10:32
  • You should also not use the final closing php tag (`?>`). It is not required and _might_ cause lots of issues. Just remove it at the end of the file. – arkascha Mar 28 '18 at 10:36
  • Note: The object-oriented interface to `mysqli` is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface. Before you get too invested in the procedural style it’s worth switching over. Example: `$db = new mysqli(…)` and `$db->prepare("…")` The procedural interface is an artifact from the PHP 4 era when `mysqli` API was introduced and should not be used in new code. – tadman Mar 28 '18 at 10:57
  • If those fields are actually named `field1` and `field2` you're making life miserable for yourself. Name them with what they are. – tadman Mar 28 '18 at 10:59
  • A lot of problems can be detected and resolved by [enabling exceptions in `mysqli`](https://stackoverflow.com/questions/14578243/turning-query-errors-to-exceptions-in-mysqli) so mistakes aren't easily ignored. – tadman Mar 28 '18 at 10:59

1 Answers1

-1

This can be due to your field "id" being an integer type in your database, if it's the case you shouldn't put coma in your VALUES for it, and always cast it to an integer, like this :

$query = "INSERT INTO accounts (id, firstName) VALUES (".(int)$var1.",'".$var2."')";

Keyle
  • 58
  • 9
  • 1
    No! This even makes things worse, considering one should _never_ place client side provided data in a sql query using string concatenation. Please also read the comment @Quentin posted under the question. – arkascha Mar 28 '18 at 10:34
  • Yes, I know that it's absolutly unsafe, I'm just answering about the sql error in this context. – Keyle Mar 28 '18 at 10:47
  • The only situation where the detail you point out might lead to the described issue is _exactly_ a description of an sql injection. So what point is there in deliberately being silent in that point? – arkascha Mar 28 '18 at 10:49
  • "I know this product I'm selling you will burn your house down, but it will also kill those pesky bedbugs." – tadman Mar 28 '18 at 10:58
  • He only asked about why he is getting an sql error, I told him what could be a cause. He also said that he was a beginner so I thought it would be a bit overkill to talk about security at this point when he just want to get his php working. – Keyle Mar 28 '18 at 11:05