0

So when someone press this link, it should insert all the data from that text id to a new table but with the username who clicked it and the id of the text the user pressed.

The problem is, when a user clicks the link, it doesn't insert the data, what could be wrong?

The session works, so it must be something with the GET?

<?php
if(isset($_GET['collect'])) {
    $perman = $_GET['collect'];
    $username = $_SESSION['username'];

    $query = $dbh->query("INSERT INTO collections (id, ad, user) VALUES ('', $perman, $username)");
    echo 'Saving';
    echo $perman;
    header ('Refresh: 1; URL=http://localhost/de/collect.php');
}

?>

user2328659
  • 71
  • 1
  • 6
  • Are you connected to the database? Do you get any error messages? You should really use prepared statements to avoid SQL injections. –  Jun 19 '13 at 16:39
  • I'm connected to the database, yes. The error message I get is "Unknown column "patrik" in "field list". That's the user who is logged in. If I login as someone else, it will say "Unknown column "that user". I use prepared statements usually. Just tried with query in this case and I will change it back once I get this fixed. – user2328659 Jun 19 '13 at 16:42

3 Answers3

0

First, inserting '' for ID isn't very good (don't know if it works), don't use it (uses default), or insert NULL (uses default too, if NOT NULL). Second, to insert values it's good practice to enquote it and use escape_string on it. I think that's your problem.

$query = $dbh->query("INSERT INTO collections (ad, user) VALUES ('" . $dbh->escape_string($perman) . "', '" . $dbh->escape_string($username) . "')");
Lorenz
  • 2,179
  • 3
  • 19
  • 18
0

You tagged your Question with "PDO". Are you using PDO? If yes, why are you not using bindParam() or bindValue()?

If $perman and $username are strings, you've to escape them:

$query = $dbh->query("INSERT INTO `collections` (`id`, `ad`, `user`) VALUES ('', '{$perman}', '{$username}')");

That query should work, but there are still security issues. You've to escape the values. With PDO it's very simple.

General: use http://php.net/manual/en/function.mysql-error.php

Your column "id" should be Integer and have an auto_increment. Of course some IDs are Strings, but if you're able to avoid it, avoid it!

You could print out the $_GET params by using

print_r($_GET);

Edit Example with PDOStatement::bindValue():

$stmt = $dbh->prepare("INSERT INTO `collections` (`id`, `ad`, `user`) VALUES (:id, :ad, :user)");
$stmt->bindValue(":id", 123);
$stmt->bindValue(":ad", "ad");
$stmt->bindValue(":user", "username");
$stmt->execute();
Mr. B.
  • 8,041
  • 14
  • 67
  • 117
  • Thanks, it works. But what shall I bindParam in this case? Thanks again – user2328659 Jun 19 '13 at 16:48
  • Your variables $perman and $username are not escaped yet. If you bind them with PDO, you don't have to do it by your own. If you don't escape your values, hackers will be able to attack your code by simple injections. – Mr. B. Jun 19 '13 at 16:50
  • @user2328659 see [How to prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php) – Alvin Wong Jun 19 '13 at 16:51
0

You should be doing it like this...if you're using PDO

Much safer, with prepared statements

 $sql = "INSERT INTO books (id,ad,user) VALUES (:id,:ad,:user)";
 $q = $conn->prepare($sql);
 $q->execute(array(':id'=>null,':ad'=>$perman,':user'=>$username));
Kylie
  • 11,421
  • 11
  • 47
  • 78