0

I am currently building a library website for my school and am having a problem getting the searchbar on the page to return more than one result. Here is the code:

PHP:

  <?php
    error_reporting(E_ALL);
    $cont = mysqli_connect('host','username','password', 'db') or die(mysqli_connect_error());
    $output='';
    if(isset($_POST['searchVal'])){
      $searchkey= $_POST['searchVal'];
      $searchkey=preg_replace("#[^0-9a-z]#i", "", $searchkey);

    $query = mysqli_query($cont, "SELECT * FROM books WHERE Title LIKE 
     '%$searchkey%' OR Author LIKE '%$searchkey%' OR ISBN_Other_code_no LIKE 
     '%$searchkey%' ") or die('couldnotconnnect');
    $count = mysqli_num_rows($query);

    if($count == 0){
        $output="There was no search result!";
    }else{
        while($row=mysqli_fetch_array($query)){
              $fname = $row['Title'];
              $lname = $row['Author'];
              $pname = $row['ISBN_Other_code_no'];

            $output = "$fname  $lname  $pname </br>";

        };
    };
    };

    echo ($output);
    ?>

JS:

function searchq(){
        var searchTxt = $("input[name='search']").val();

        $.post("search.php", {searchVal: searchTxt}, function(output){
            $("#output").html(output);
        });
};

The code above only posts one result in my HTML when there should be 10 or more. Can anyone see a possible bug in the code stopping it from looping through all of the statements in the SQL table? Or do I have to write a loop in the PHP?

Thanks for your help!

Qirel
  • 25,449
  • 7
  • 45
  • 62
Conor Curley
  • 17
  • 1
  • 2
  • Dont post sensitive data like database connection details. – Ravinder Reddy Aug 08 '17 at 21:05
  • I've edited out your credentials to the database - I recommend you change the password **immediately**, as it may still be a security risk! – Qirel Aug 08 '17 at 21:05
  • 1
    Actual issue is that you overwrite `$output` in each iteration. Concat or use an array instead. Change `$output = "$fname $lname $pname ";` to `$output .= "$fname $lname $pname ";` (note the change from `=` to `.=`). – Qirel Aug 08 '17 at 21:06
  • You're already using an API that supports **prepared statements** with bounded variable input, you should utilize parameterized queries with placeholders (prepared statements) to protect your database against [SQL-injection](http://stackoverflow.com/q/60174/)! Get started with [`mysqli::prepare()`](http://php.net/mysqli.prepare) and [`mysqli_stmt::bind_param()`](http://php.net/mysqli-stmt.bind-param). – Qirel Aug 08 '17 at 21:06
  • Oh my god I meant to edit that out before I posted...Thank you so much Qirel!!! – Conor Curley Aug 08 '17 at 21:07
  • 1
    It's still visible in the "revision log", so you should change it asap. – Qirel Aug 08 '17 at 21:07
  • I'm new to PHP Qirel, how do I stop it overwriting? And I'm not too worried about SQL injection because it will be a closed site only available to the school, and I don't think anyone would want a database full of books! – Conor Curley Aug 08 '17 at 21:09
  • Even so, it's not something you can ignore. It's a huge security issue, and you should never assume anything when it comes to security - always protect your back! Use prepared statements! See the links I posted. [**My comment above**](https://stackoverflow.com/questions/45577991/instant-search-only-returns-one-item#comment78115489_45577991) was since edited, and shows one (of many possible) solution. – Qirel Aug 08 '17 at 21:10
  • Okay, it's changed, thanks so much to everyone who alerted me. So how do I use parameterised queries? I'm so new to PHP, apologies. – Conor Curley Aug 08 '17 at 21:14
  • It's well documented [**in the manual for MySQLi**](http://php.net/manual/en/mysqli-stmt.bind-param.php#refsect1-mysqli-stmt.bind-param-examples), with good examples. Try it out! ;-) If you face issues from it, post a new question here with it, and we'll be sure to help you out ;-) – Qirel Aug 08 '17 at 21:15
  • Thanks so much for your help everyone! I'll get onto that Qirel. – Conor Curley Aug 08 '17 at 21:20
  • I have one more problem... When the searchbar is empty the page displays every item in the table, which is 3500 items... Any ideas? – Conor Curley Aug 08 '17 at 21:41
  • That's a different question and you should ask it separately. I am going to tip you off anyway: only fire the ajax request if the length of the search field is more than a sensible value (which might be 3 or 4 in your case). Please tell me this is an assignment and not going to production yet. – simlev Aug 08 '17 at 22:00

1 Answers1

2

Putting other issues to the side, it stands out you are overwriting $output instead of appending to it. Try:

$output .= "$fname  $lname  $pname </br>";

Note: Qirel posted this same advice while I was slowly and awkwardly typing on my mobile. Feel free to post it as an answer.

simlev
  • 919
  • 2
  • 12
  • 26