Searching a MySQL table using PHP

Question

I am trying to create a PHP file to help search a table built in MySQL from a webpage. I have built the form, which allows the user to enter keywords into two of the search criteria and a drop-down menu for the third. However, I am having trouble with the PHP file itself. I have appeared to do something wrong and cant quite figure out what is going wrong. If anyone can spot an error in the code below I'd really appreciate the help. Thanks.

// define variables and set to empty values
$Location = $Commemorating =  "";
if (isset($_GET['Region']) && !empty($_GET['Region'])) 
{
    $Region_name = $_GET['Region'];

    if (empty($_GET["Location"])) 
    {
        $Location = "";
    } 
    else 
    {
        $Location = ($_GET["Location"]);
    }

    if (empty($_GET["Commemorating"])) 
    {
        $Commemorating = "";
    } 
    else 
    {
        $Commemorating = ($_GET["Commemorating"]);
    }

$query = "SELECT Monument,
                Location,
                Commemorating,
                Region,

        FROM MONUMENTS 
        WHERE Region = '$Region'";
        //..if a location is specified run this query
        if ($Location != "") 
        {
            $query .= " AND Location LIKE '%$Location%'";
        }
        //..and if a name is entered run this query
        if ($Commemorating != "") 
        {
            $query .= " AND Commemorating LIKE '%$Commemorating%'";
        } 
        //..and if a region is specified run this query
        if ($Region != "All") 
        {
            $query .= " AND Region LIKE '$Region'";
        }
$query_run = mysql_query($query);
}

One important thing I need to point out (even though it doesn't fix your issue) is that you have no security mechanism against sql injection; I would highly recommend reading the selected answer at " stackoverflow.com/questions/60174/… ". That page will also introduce you to a more modern way of reading from the database, as mysql_query is depreciated and will not work in future PHP versions. — Kver
– Kver, Commented Jun 4, 2014 at 14:30

Alexander · Accepted Answer · 2014-06-04 17:24:32Z

1

$query = "SELECT Monument,
            Location,
            Commemorating,
            Region,

Looks like you should strip list comma in field list from the query:

$query = "SELECT Monument,
            Location,
            Commemorating,
            Region

Like this.

There is a bit misunderstanding since you check is Region is not empty, then query for items in given Region and then add another cause in case of Region is not 'All'. So if I run your code with Region = 'All' then the query will return only the items that have Region set to 'All', which sounds a bit odd (I'd say monuments are at a single region, isn't it?).

You also use LIKE while may simple use = since you add sibgle quotes (') around strings so it won't give you any 'wildcard' match but slow down the query. Another thing to do is to do some mysql escape function to be sure you won't get SQL code in one of your GET query.

May I also suggest to short your code a bit:

$Region_name = isset($_GET['Region']) ? trim($_GET['Region']) : '';
if ($Region_name) {
    $Location = isset($_GET['Location']) ? trim($_GET['Location']) : '';
    $Commemorating = isset($_GET['Commemorating']) ? trim($_GET['Commemorating']) : '';
    $query = sprintf("SELECT 
                    Monument,
                    Location,
                    Commemorating,
                    Region
            FROM MONUMENTS
            WHERE 1=1%s%s%s",
                    $Region!='All' ? "AND Region='".mysql_real_escape_string($Region)."'",
                    $Location ? "AND Location='".mysql_real_escape_string($Location)."'",
                    $Commemorating ? "AND Region = '".mysql_real_escape_string($Region)."'",
    );
    ...etc...

I add 1=1 so I can easily add AND to the following causes without worry.

edited Jun 4, 2014 at 17:24

answered Jun 4, 2014 at 14:30

Alexander

4941 gold badge6 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user3707442 Over a year ago

Thanks, it has stopped giving me an error message after doing this.

Alexander Over a year ago

Nice to know that! :)

Community · Accepted Answer · 2017-05-23 12:27:25Z

0

Use $Region_name instead of $Region in your query. I see you depend on user input (via $_GET). Make sure you sanitize user input: https://stackoverflow.com/a/3126175/1071063

edited May 23, 2017 at 12:27

CommunityBot

11 silver badge

answered Jun 4, 2014 at 14:32

Bas

612 bronze badges

Collectives™ on Stack Overflow

Searching a MySQL table using PHP

2 Answers 2

2 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related