1

I've been on this for some days now. At first I thought the problem was in binding the parameters but I've simplified back to a basic mysqli page and still can't find the error. I'm passing the key for one of the rows in the search page before this onto this page so that I can show more details of the item which was selected.

I added an echo to test the the isset which prints correctly, also it puts the Key into the URL. If I leave out the WHERE Key = '$Key' it prints out the entire dataset. If I replace $row['Key'] with $Key it prints the whole dataset but with the selected key on every row.

This tells me that it is passing the key correctly and the print function is correct. I've tried using WHERE Key = $_GET['Key'] as well as $Key but neither work. I must be doing something basicly wrong here but after three days of trying every variation on the code I can think of, I have no more ideas.

<?php

$mysqli = new mysqli('localhost','user','password','database');

if ($mysqli->connect_error) {
die('Error : ('. $mysqli->connect_errno .') '. $mysqli->connect_error);
}
if(isset($_GET['Key'])){
$Key = $_GET['Key'];
echo "Got it";
    }else{
    echo "No input";
    }

$results = $mysqli->query("SELECT * FROM engravers WHERE Key ='$Key'");
$img_url = "http://www.xxxxx.net/images/"; 
print '<table border="1" >';    
while($row = $results->fetch_assoc()) {

print '<tr>';
print '<td>'.$row["Key"].'</td>';
print '<td>'.$row["Country"].'</td>';
print '<td>'.$row["Year"].'</td>';
print '<td>'.$row["Description"].'</td>';
print '<td>'.$row["Engraver1Surname"].'</td>';
print '<td>'.$row["Designer1Surname"].'</td>';
print '<td>'.$row["Printer"].'</td>';
print  '<td>'.'<img src="'.$img_url.$row['Images'].'" />'.'</td>'; 
print '</tr>';
}  
print '</table>';


$results->free();

$mysqli->close();
?>
</body>

jjarmstrong47
  • 55
  • 6
  • Possibly because `$Key` isn't always set. That said, if the `$_GET['Key']` parameter holds a value like `1'; DROP TABLE engravers; -- `, you're in for a nasty surprize... google injection attacks and prepared statements if you care at all about data integrity. Also do some digging concerning best-practices when writing queries: `SELECT * FROM` is not a good idea... honest – Elias Van Ootegem Aug 20 '14 at 11:41
  • 1
    That is the `mysqli` extension telling you to stop writing code vulnerable to sql injection – PeeHaa Aug 20 '14 at 11:41
  • 1
    First of all you should end your script if there's no 'Key' GET value. Do this by putting all the query and table code inside an `if(isset($_GET['Key']){ ... }` Then you can just show a message in the `else` clause. – Marc Aug 20 '14 at 11:44
  • 1
    @Elias While the code is vulnerable to SQL injection, your example is not a valid attack. `mysqli::query` can only execute one query. – Mitch Satchwell Aug 20 '14 at 11:46
  • Thanks @Hule. I'll do that. I have the page written in safe code but had the same problem. I've simplified things to try to track down the problem and found it is in the WHERE statement. When I find out why, I can try again with the prepared statements. – jjarmstrong47 Aug 20 '14 at 11:55

3 Answers3

1

There are many SQL column names you should avoid. Please read: http://technet.microsoft.com/en-us/library/ms189822.aspx

Same Reserved Keywords are in MySQL.

If you use one of those just cover it with ``

$results = $mysqli->query("SELECT * FROM `engravers` WHERE `Key`='$Key'");
MacEncrypted
  • 184
  • 9
  • Eureka! Thanks! This works! Now to go back to the other code. Are you able to explain why it works? – jjarmstrong47 Aug 20 '14 at 12:06
  • Just to confirm, you asking me why your code works in way it does before, right? – MacEncrypted Aug 20 '14 at 12:11
  • Yes. Is it a bad idea to have a column called Key for the id? – jjarmstrong47 Aug 20 '14 at 12:14
  • Thanks again. I've just read that article. As there are only three simple pages in my site it sounds like it would be worth the effort to change my column name to ID which doesn't seem to be reserved. I appreciate your help. – jjarmstrong47 Aug 20 '14 at 12:27
  • @jjarmstrong47 Sort of. In mysql `KEY` means `INDEX`. It is equal. `{INDEX|KEY}`. It is customary to use column name `id` for `PRIMARY KEY` in table and for example `id_user` or `user_id` for `FOREIGN KEY`. More info in this thread: http://stackoverflow.com/questions/924265/what-does-the-key-keyword-mean – MacEncrypted Aug 20 '14 at 12:29
0

In your query you are using column key which is not allowed because this is a reserved keyword.

Peeyush Budhia
  • 21
  • 3
0

Man, you're already using mysqli, why are you still interpolating variables in your query? This is the way to pass a string $Key to the parser

$results = $mysqli->prepare("SELECT * FROM engravers WHERE `Key`= ?");
   $results->bind_param("s", $Key);
   while($row = $results->fetch_assoc()) {
       ... stuff ...
   }

If that doesn't work, I'm sure it's not because the prepared statement is the problem.

PD: key is a reserved word, so please note the fieldname is wrapped in backticks.

ffflabs
  • 17,166
  • 5
  • 51
  • 77
  • Thanks. That's pretty much the same as the page I have written. I started writing this in mysql and in the tutorial I used, the id was called Key so I stuck with it. Thanks everyone. I hadn't come across the reserved keywords and backticks before. I live in a fairly remote area in Australia so online tutorials are my best option and most are either out of date or don't go far enough. The people here are really great. – jjarmstrong47 Aug 20 '14 at 12:20