Search script returning incorrect results

[stualk]

Hello folks,

 

I've got a problem with a search script. It's searching a db containing various properties (as in buildings) and it's not quite returning the correct results. The search form asks that you select a house type (4 options, all of which are tick boxes) and then the last part of the form is select how many bedrooms you'd like (which is a drop-down list featuring numbers 2-5)

 

What should happen is someone selects one or more of the tick boxes and then the number of bedrooms and it returns all the house types that were ticked but only those with the number of bedrooms that were specified in the original search. Currently the results are correct in that they're showing the correct house types, but the number of bedrooms isn't right. e.g. if you chose 2 bedrooms in the search it shows those, as well as 3, 4 and 5 bedrooms in the results.

 

Can't figure it out but it will be something to do with && and OR in my search script. Can anyone suggest a tweak to this that might work? (I'm using the $_GET function because the search form is in a Flash movie)

 

$Result = mysql_query("select * from property_names where TownHouse = '$_GET[TownHouse]' OR Apartment = '$_GET[Apartment]' OR Detached = '$_GET[Detached]' OR SemiDetached = '$_GET[semiDetached]' && visible= 'Yes' && bedrooms = '$_GET[bedrooms]' && bedrooms = '$_GET[bedrooms]'")

 

[monkeytooth]

&& I don't think is valid for mySql AND would be more appropriate. also not sure about the newer version of mySQL but pretty sure you should capitalize the mysql statements like the words select, where. Also your searching for Bedrooms twice..

 

Then another thing to consider is going outside the box and building your query as its needed.

 

$myQuery = "SELECT * FROM mytablename WHERE ";
if($tickbox1 == "checked"){$myQuery .= "tickbox1Row = '".mysql_real_escape_string($value1)."'";
if($tickbox2 == "checked"){$myQuery .= "tickbox2Row = '".mysql_real_escape_string($value2)."'";

echo $myQuery; //shows you what it built for the query

 

Now this is a piss poor excuse for an example. You will have to modify the concept to include the AND's and OR's but at least it gives you an idea of what might be an easier way to handle your query strings.

 

and lastly. notice the use of mysql_real_escape_string that is going to be your best friend. Never ever ever ever allow raw user input from a session, cookie, get, post, request variable to be used in a query. Your just asking for an injection attack that way. I suggest reading up on it mysql_real_escape_string and other injection prevention and sanitization techniques before continuing.

 

[stualk]

Thanks for this. Some useful info and advice there. Always good to get a second opinion on the method etc, especially considering you spotted that bedrooms was in there twice and I didn't! (Although removing it hasn't altered the search results) I'll try to post back to let you know how I got on using your method.

 

Thanks again for the assistance.

[MMDE]

try to do it in php, without using ( or ) to group things. see if you get the results you want.

[PFMaBiSmAd]

Your query needs to have () around the OR'ed terms to make an expression that is then AND'ed with the rest of the logical conditions (and AND and && are equivalent in mysql) -

"select * from property_names where (TownHouse = '$_GET[TownHouse]' OR Apartment = '$_GET[Apartment]' OR Detached = '$_GET[Detached]' OR SemiDetached = '$_GET[semiDetached]') && visible= 'Yes' && bedrooms = '$_GET[bedrooms]' && bedrooms = '$_GET[bedrooms]'"

[stualk]

Just wanted to say thanks for all the great tips on this. Grouping the 'OR' items within brackets solved the issue with the incorrect search results. Now I get results which are 100% accurate. Later I'll be looking at mysql_real_escape_string to make it better still but ultimately the search is now working perfectly, so thanks to all for your help.

[monkeytooth]

Definitely The way the original  look into the mysql_real_escape thing, and a few other things worth embedded in the code to prevent SQL injection. The way the original code is standing, with the post/get variables directly in the queries with no prevention measures at all in the query or surrounding code. That's just asking for someone to come along run an injection method which if they hate you that can do something that would drop the entire database, if they don't they can inject it with whatever pretty much. So its something very much worth looking into. It would suck to have someone come along months after your hard work, months after getting so many things into the database and then have someone issue a drop command or do something that would rewrite a mass of the data you have or whatever the case or intention is of said person. Ultimately messing up everything in a way that would be seriously hard to recover from.