4

I'm building a website to learn PHP and am making an autosuggest from Jquery Ui.

Here's my code ( I took this code from a post on SO a long time ago, and I'm not 100% what it does, so if anyone could explain, it would be helpful!) This code is from suggest.php, which I call to from my jquery code (which I think is working, so I didn't post it, but can If you need it!)

<?
include("config.php");

$queryString = strtolower($_GET["q"]);

$return = array();
$query = mysql_query("SELECT name FROM company WHERE name LIKE '$queryString%' UNION SELECT cat FROM cat WHERE cat LIKE '$queryString%' UNION SELECT subcat FROM subcat WHERE subcat LIKE '$queryString%' LIMIT 10");
while ($row = mysql_fetch_array($query)) {
     array_push($return,array('label'=>$row['name'],'value'=>$row['name']));
}
echo(json_encode($return));

?>

Right now this is making the autosuggest work, but only with the same results (example, if I type "Johns" it comes up with "Johns Hot Dogs" as a suggestion, but If I type "fjfjdjf669959" then it comes up with "Johns Hot Dogs" as well.

I'm doing a Mysql Union because I'm trying to populate my autosuggest with the name row from company table, the cat row from cat table, and the subcat row from subcat table.

Why is this not working?

Thanks for any and all help!!

My JQUERy code looks like this:

<script>
    $(function() {
        $( "#search" ).autocomplete({
            source: "suggest.php"
        });
    });
</script>
Muhambi
  • 3,472
  • 6
  • 31
  • 55
  • Not sure why "Johns Hot Dogs" comes up with `fjfjf` .. what happens when you run the query manually? You also need to alias `cat` and `subcat` as `name` if you want them to be added to `return`. – Explosion Pills Aug 10 '12 at 17:40
  • @Explosion, thanks for the help. Does running the query manually mean going to suggest.php?q= ? If it does then it echoes out `[["Johns Hot Dogs","Johns Hot Dogs"],` and it repeats for all of my rows in my tables – Muhambi Aug 10 '12 at 17:45
  • well I meant running it with the `mysql` program on the command line, but that way would work too I guess. It does this even if `q=fjfjfj`? – Explosion Pills Aug 10 '12 at 17:49
  • @ExplosionPills, no if q=fjfjfj, then its just echoes " [] " – Muhambi Aug 10 '12 at 17:55
  • 1
    it's possible that the ajax request is cached – Explosion Pills Aug 10 '12 at 17:59
  • @ExplosionPills, ok so should I post my jquery code? – Muhambi Aug 10 '12 at 18:00
  • You'll be hacked if you publish this code. Google "SQL Injection". – nalply Aug 10 '12 at 19:08
  • Please, don't use `mysql_*` functions to write new code. They are no longer maintained and the community has begun [deprecation process](http://goo.gl/KJveJ). See the *[red box](http://goo.gl/GPmFd)*? Instead you should learn about [prepared statements](http://goo.gl/vn8zQ) and use either [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli). If you can't decide which, [this article](http://goo.gl/3gqF9) will help you. If you pick PDO, [here is good tutorial](http://goo.gl/vFWnC). – Madara's Ghost Aug 12 '12 at 21:06
  • @Truth, thanks. Definitely will change! – Muhambi Aug 12 '12 at 21:28

1 Answers1

4

First of all your php code is vulnerable to SQL injection attacks. Furthermore, the mysql_* functions are deprecated. Instead, use PDO.

Your code fails because you're reading the wrong query variable. $_GET['q'] is empty since the jQuery UI autocomplete plugin uses the parameter term for the search query. With an empty $queryString, you execute the SQL query

SELECT name FROM company WHERE name LIKE '%'   -- UNION ...

which of course just returns everything. You want:

<?php
include("config.php");
$db = new PDO('mysql:host=localhost;dbname=database', 'user', 'password');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

if (!isset($_GET['term'])) {
    header('HTTP/1.0 400 Bad Request');
    echo 'Missing term parameter in request';
    exit();
}
$queryString = strtolower($_GET["term"]);
$query = $db->prepare("SELECT name FROM company WHERE name LIKE :qs" .
  " UNION SELECT cat AS name FROM cat WHERE cat LIKE :qs" .
  " UNION SELECT subcat AS name FROM subcat WHERE subcat LIKE :qs " .
  " LIMIT 10");
$query->execute(array(':qs' => $queryString . '%'));
$query->setFetchMode(PDO::FETCH_NAMED);
$result = array_map(function($row) {
    return array('label'=>$row['name'],'value'=>$row['name']);
}, $query->fetchAll());

header('Content-Type: application/json');
echo(json_encode($result));

Here is a live, downloadable (incl. database) demo.

phihag
  • 278,196
  • 72
  • 453
  • 469
  • Make sure to set prepared statement emulation to false too. – Madara's Ghost Aug 12 '12 at 21:07
  • Also, I'm pretty sure you can't use the same placeholder name twice in the same prepared statement. – Madara's Ghost Aug 12 '12 at 21:08
  • @Truth What's the problem with prepared statement emulation? And I'm testing it with multiple tables, updating ... – phihag Aug 12 '12 at 21:10
  • Setting it to false solves some [edge cases](http://stackoverflow.com/a/11693505/871050). – Madara's Ghost Aug 12 '12 at 21:11
  • @Truth That looks to be like a workaround for a bug in php or PDO. I think someone may have tested that strings work ;) – phihag Aug 12 '12 at 21:15
  • @phihag, thanks for the help. Two questions: 1. When I put in the php code, it gives me an error: `Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '% UNION SELECT subcat AS name FROM subcat WHERE subcat LIKE '%' LIMIT 10' at line 1' in suggest.php:10 Stack trace: #0 suggest.php(10): PDOStatement->execute(Array) #1 {main} thrown in suggest.php on line 10'` – Muhambi Aug 12 '12 at 21:25
  • and the 2nd question is: Do I have to change name='q' in my searchbox to name='term', or does the jquery do that for me? – Muhambi Aug 12 '12 at 21:27
  • @Jake Sorry, that was an error in the earlier version. I updated the answer, and included a link where you can download a zipfile of a full demo. And the value of the `name` attribute doesn't matter for autocomplete. You are free to set any value. – phihag Aug 12 '12 at 21:39
  • @phihag, I'd award you the bounty now, but it wont allow me for 22 hours. – Muhambi Aug 12 '12 at 21:45