-1

This is my first code-question! I'm a beginner at both MySQL and PHP, so this might be really simple! Here is my code:

This is a file included in my Index.php...:

<?php
$query="SELECT * FROM wines WHERE Type='$type' AND Country='$country'";
$products=mysql_query($query);
?>

And these are the variables set with the $_GET function:

<?php
$type=$_GET['type'];
$fruit=$_GET['fruit'];
$country=$_GET['country'];
?>

And then later I'll fetch the array and work with it.

The problem is that my $query works fine with just the '$type'-variable, but not with the $country -variable - - - - or any other variable I've tried.

I use Microsoft Webmatrix and it tells me that the issue arises the very moment I type in the $-sign in the second variable...

So confused! Hope you can help a newcomer :)

EDIT: I found out the problem was with the "ticks" around my variables. The correct way to do it is with backticks ( `` ). Also, I started using PDO and MySQLi instead of mysql. For beginners, MySQLi is probably the easiest. Started sanitizing too.

  • Learn to sanitize input asap. Try echoing out the $_GET variables and see if they are set. – Sterling Archer Mar 20 '13 at 21:52
  • Do not write another line of code before you clean those inputs up. Read up on sql injection: http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php – span Mar 20 '13 at 21:53
  • 1
    You should use PDO to secure your requests, and avoid SQL injections. To know what is inside your variables before usig them, you can use `var_dump($var)` – Acuao Mar 20 '13 at 21:56
  • Since you're a beginner with PHP, it is important to tell you that the code you've written is using obsolete PHP functions. The `mysql_xx()` functions are deprecated and have been replaced by the PDO library. The old functions still work (for the time being) for the benefit of existing code, but if you're learning to program PHP for the first time, you really should learn to use the new functions; there's no point learning how to do something that's already obsolete. Some links to help you: https://phpbestpractices.org/ and http://phpmaster.com/avoid-the-original-mysql-extension-2/ – Spudley Mar 20 '13 at 22:03
  • @PRPGFerret: NEVER SANITIZE INPUT - ALWAYS SANITIZE OUTPUT! – symcbean Mar 20 '13 at 22:57
  • @Spodley: No - the mysql_ functions are deprecated - PDO does NOT replace them: it provides an alternative (non-deprecated) API, as does the mysql extension. – symcbean Mar 20 '13 at 22:58
  • I used your answers to become a better programmer. Thank you. –  Oct 15 '13 at 07:28

2 Answers2

1

Just a couple of (important) points to note...

  1. You should probably be using the mysqli_* functions in the place of the mysql_ functions, as they are going to be deprecated in a following release.

  2. start sanitizing your input before making DB queries. It's a good habit to get into early, and will save you a lot of headaches in the long run!

a good habit it to use sprintf and mysqli_real_escape_string to build your SQL before executing it on the db:

$sql = sprintf("SELECT * FROM wines WHERE Type='%s' AND Country='%s'" ,
    mysqli_real_escape_string($db_object, $type),
    mysqli_real_escape_string($db_object, $country));

$results = mysqli_query($db_object, $sql);

ps. in my example $db_object is coming from the call to mysqli_connect()

EDIT:

Using the (soon-to-be-defunct) mysql_ functions would be something like the following for the above example:

$sql = sprintf("SELECT * FROM wines WHERE Type='%s' AND Country='%s'" ,
    mysql_real_escape_string($type),
    mysql_real_escape_string($country));

$results = mysql_query($sql);
msturdy
  • 10,479
  • 11
  • 41
  • 52
  • I can't get mysqli functions to work. Might be because my MySQL installation went corrupt when I did it along with Webmatrix. Resorting to Mysql functions fixed the issues. –  Mar 20 '13 at 22:32
  • OK, I've updated my answer regarding the functions. Though it would be worth your while investing some effort in getting them working, or at least take a look at the DBO functions: http://php.net/manual/en/book.pdo.php – msturdy Mar 20 '13 at 22:56
0

If you don't send a $type in your query string, the query is

SELECT * FROM wines WHERE Type='' AND [other stuff];

which will give back only wines with type = '';

Try:

<?php
$wheres=array();
if (isset($type)){$wheres['Type']=$type;}
if (isset($type)){$wheres['Country']=$country;}
// and so forth for each parameter
$query="SELECT * FROM wines";
if (count($wheres) != 0){
    $query.=" WHERE ";
    foreach ($wheres as $k => $v){$query.="$k='$v' AND ";}
    $query.=" 1=1;"
}
$products=mysql_query($query);
?>
Lighthart
  • 3,648
  • 6
  • 28
  • 47
  • and also pay attention to msturdy's sanitize comments. – Lighthart Mar 20 '13 at 22:12
  • Cant get that piece of code to work. I'm not well enough versed yet to troubleshoot effectively, but I see the point in your code. I know that it's not safe, but it's because I'm developing a platform for one store that will run on an Ipad in guided access, so security is not important. If the concept seems to work, I'll have funding for more skillful programmers at hand :) –  Mar 20 '13 at 22:28
  • A possible quickfix to your original: $query="SELECT * FROM wines WHERE Type='%$type%' AND Country='%$country%'"; – Lighthart Mar 20 '13 at 22:43
  • as soon as $country is written, the rest of the query turns grey, meaning (in Webmatrix's eyes), that it's faulty... Dunno why... –  Mar 20 '13 at 22:47