Pagination doesn't work properly

[vinsb]

Hello, 

I have one search form and when I search something the result is whole records from the database not only the search term. Any help is appreciated. I will post only the part where is calculating the rows from db but if need I will post whole pagination script. 

$searchTerm = trim($_POST['term']);
$adjacents = 3;

$sql1 = mysql_query("SELECT * FROM images ORDER BY id ASC");
$nr = mysql_num_rows($sql1);
$limit = 6;
$targetpage = "search.php";
$page = $_GET['page'];
if($page)
    $start = ($page - 1) * $limit;
else
    $start = 0;

if ($page == 0) $page = 1;
$prev = $page - 1;	 // $prev = $page - 1
$next = $page + 1;	 // $next = $page + 1
$lastpage = ceil($nr/$limit);   //lastpage = total pages / items per page.
$lpm1 = $lastpage - 1;
$pagination = "";
....
....
// pagination
...
...
// while loop for the results
$sql = "SELECT id, name, caption FROM images WHERE caption LIKE '%".$searchTerm."%' ORDER BY id DESC LIMIT $start, $limit";
$result = mysql_query($sql);
while($row = mysql_fetch_array($result))
{
    $output.= "<div class=\"container_image\">";
    $output.= "<a href=\"/pic-".$row['id'].".html\"><img src=\"/upload/".$row['name']."\" width=\"210\" height=\"150\"/></a>";
    $output.= "</div>";
}

Edited July 21, 2014 by vinsb

[mac_gyver]

the WHERE clause you are forming needs to be in both the query that gets a count of matching rows and in the query that retrieves the data. so, it is best if you form the WHERE clause in a php variable, then just put that variable in both queries.

 

next, the search term determines what the page is gong to display and should use method='get'. a second reason for using get is to make it easier to build the pagination links, since they must have both the pagination and search term in them for this to work.

 

i'm pretty sure i have posted an example somewhere. i will either find a link or re-post something.

[vinsb]

Thank you for the replay. So with WHERE clause in other query the problem is partially resolved. It show the proper number of pages in pagination but when I click next and go on second page show again whole data from db. This is the piece of code which I didn't post first time

if($lastpage > 1)
{
    $pagination .= "<div class=\"pagination\">";
    //previous button
    if ($page > 1)
        $pagination.= "<a href=\"$targetpage?page=$prev\"> previous</a>";
    else
        $pagination.= "<span class=\"disabled\"> previous</span>";

    //pages
    if ($lastpage < 7 + ($adjacents * 2))
    {
        for ($counter = 1; $counter <= $lastpage; $counter++)
        {
            if ($counter == $page)
                $pagination.= "<span class=\"current\">$counter</span>";
            else
                $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
        }
    }
    elseif($lastpage > 5 + ($adjacents * 2))
    {
        if($page < 1 + ($adjacents * 2))
        {
            for ($counter = 1; $counter < 4 + ($adjacents * 2); $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"$targetpage?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"$targetpage?page=$lastpage\">$lastpage</a>";
        }
        elseif($lastpage - ($adjacents * 2) > $page && $page > ($adjacents * 2))
        {
            $pagination.= "<a href=\"$targetpage?page=1\">1</a>";
            $pagination.= "<a href=\"$targetpage?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $page - $adjacents; $counter <= $page + $adjacents; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"$targetpage?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"$targetpage?page=$lastpage\">$lastpage</a>";
        }
        else
        {
            $pagination.= "<a href=\"$targetpage?page=1\">1</a>";
            $pagination.= "<a href=\"$targetpage?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $lastpage - (2 + ($adjacents * 2)); $counter <= $lastpage; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
        }
    }

    if ($page < $counter - 1)
        $pagination.= "<a href=\"$targetpage?page=$next\">next </a>";
    else
        $pagination.= "<span class=\"disabled\">next </span>";
    $pagination.= "</div>\n";
}

[mac_gyver]

the easiest way of building the query-string part of links, with any existing values (the search term), and any specific values (the page number) is to use http_build_query(). see the example code in the following reply - 

http://forums.phpfreaks.com/topic/288934-pagination-question-using-array-to-get-data/?hl=%2Bhttp_build_query&do=findComment&comment=1481655

[vinsb]

Thank you again. I'm trying to implement http_build_query() in my code but no success. Here is how I try and I guess is not ok since doesn't work.

if($lastpage > 1)
{
    $pagination .= "<div class=\"pagination\">";
    //previous button
    if ($page > 1) {
       
--->        $_GET['page'] = 1;                 
--->        $pagination = http_build_query($_GET, '', '&');
        $prevlink =  "<a href='?$pagination' title='First page'>«</a>";
--->        $_GET['page'] = $page - 1;
--->        $pagination = http_build_query($_GET, '', '&');
        $prevlink .= " <a href='?$pagination' title='Previous page'>‹</a>";
    }
    else
        $pagination.= "<span class=\"disabled\"> previous</span>";

    //pages
    if ($lastpage < 7 + ($adjacents * 2))
    {
        for ($counter = 1; $counter <= $lastpage; $counter++)
        {
            if ($counter == $page)
                $pagination.= "<span class=\"current\">$counter</span>";
            else
                $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
        }
    }
    elseif($lastpage > 5 + ($adjacents * 2))
    {
        if($page < 1 + ($adjacents * 2))
        {
            for ($counter = 1; $counter < 4 + ($adjacents * 2); $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"$targetpage?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"$targetpage?page=$lastpage\">$lastpage</a>";
        }
        elseif($lastpage - ($adjacents * 2) > $page && $page > ($adjacents * 2))
        {
            $pagination.= "<a href=\"$targetpage?page=1\">1</a>";
            $pagination.= "<a href=\"$targetpage?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $page - $adjacents; $counter <= $page + $adjacents; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"$targetpage?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"$targetpage?page=$lastpage\">$lastpage</a>";
        }
        else
        {
            $pagination.= "<a href=\"$targetpage?page=1\">1</a>";
            $pagination.= "<a href=\"$targetpage?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $lastpage - (2 + ($adjacents * 2)); $counter <= $lastpage; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"$targetpage?page=$counter\">$counter</a>";
            }
        }
    }

    if ($page < $counter - 1) {
--->        $_GET['page'] = $page + 1;
--->        $pagination .= http_build_query($_GET, '', '&');
        $pagination.= "<a href=\"$targetpage?page=$next\">next </a>";
    }
    else
        $pagination.= "<span class=\"disabled\">next </span>";
    $pagination.= "</div>\n";
}

I've added this lines ( '--->' in front of them)

Edited July 21, 2014 by vinsb

[mac_gyver]

every occurrence of ?page=_______ in the links would need to be changed.

[vinsb]

Ok, I've changed them to

 if ($page > 1) {

        $_GET['page'] = 1;
        $pagination = http_build_query($_GET, '', '&');
        $prevlink =  "<a href='?$pagination' title='First page'>«</a>";
        $_GET['page'] = $page - 1;
        $pagination = http_build_query($_GET, '', '&');
        $prevlink .= " <a href='?$pagination' title='Previous page'>‹</a>";
    }
    else
        $pagination.= "<span class=\"disabled\"> previous</span>";
    if ($lastpage < 7 + ($adjacents * 2))
    {
        for ($counter = 1; $counter <= $lastpage; $counter++)
        {
            if ($counter == $page)
                $pagination.= "<span class=\"current\">$counter</span>";
            else
                $pagination.= "<a href=\"?page=$counter\">$counter</a>";
        }
    }
    elseif($lastpage > 5 + ($adjacents * 2))
    {
        if($page < 1 + ($adjacents * 2))
        {
            for ($counter = 1; $counter < 4 + ($adjacents * 2); $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"?page=$lastpage\">$lastpage</a>";
        }
        elseif($lastpage - ($adjacents * 2) > $page && $page > ($adjacents * 2))
        {
            $pagination.= "<a href=\"?page=1\">1</a>";
            $pagination.= "<a href=\"?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $page - $adjacents; $counter <= $page + $adjacents; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"?page=$counter\">$counter</a>";
            }
            $pagination.= "...";
            $pagination.= "<a href=\"?page=$lpm1\">$lpm1</a>";
            $pagination.= "<a href=\"?page=$lastpage\">$lastpage</a>";
        }
        else
        {
            $pagination.= "<a href=\"?page=1\">1</a>";
            $pagination.= "<a href=\"?page=2\">2</a>";
            $pagination.= "...";
            for ($counter = $lastpage - (2 + ($adjacents * 2)); $counter <= $lastpage; $counter++)
            {
                if ($counter == $page)
                    $pagination.= "<span class=\"current\">$counter</span>";
                else
                    $pagination.= "<a href=\"?page=$counter\">$counter</a>";
            }
        }
    }

    if ($page < $counter - 1) {
        $_GET['page'] = $page + 1;
        $pagination .= http_build_query($_GET, '', '&');
        $pagination.= "<a href=\"?page=$next\">next </a>";
    }
    else
        $pagination.= "<span class=\"disabled\">next </span>";

And I receive this

 

[fastsol]

I have a pagination page generator here http://amecms.com/article/Pagination-Page-Generator .

The function that outputs the links will also take a parameter for the extra $_GET vars you need in the links, you just need to make a string to feed the function for those added vars.

[vinsb]

I have a pagination page generator here http://amecms.com/article/Pagination-Page-Generator .

The function that outputs the links will also take a parameter for the extra $_GET vars you need in the links, you just need to make a string to feed the function for those added vars.

Great tool!!

Working like above. When I click on Next link I get 240 pages instead 2

 

Here is what your tool produce to me. I've just put my while loop

// Makes a CONSTANT to use in the paginate function that defines the page the name to be placed in the pagination links href.
define ("body_id", basename($_SERVER['SCRIPT_NAME'], ".php"));

// Sets the page number for pagination
if(!isset($_GET['page']) || (int)$_GET['page'] < 0)
{$page = 1;}
else {$page = (int)$_GET['page'];}

// This is the number of items displayed per page.
$pp = 12;

// This is number of links to display before ... and then the last number of pages available.
$display_links = 4;

// Sets up the start value for pagination queries
function startValue($page, $pp)
{
    return ($page > 0) ? ($page * $pp) - $pp: 0;
}

// Standard Next and Previous setup for pagination
// $pp tells it how many items per page
// $cp is the $_GET['page'] for the current page.
// $p_type allows you to specify additional $_GET items for the page you are using.
// $lpp allows you to specify the number of links to show before it starts to split them up.
// Define your own class called current-page to destinguish what page you are on in the links.

function paginate($rc, $pp, $cp, $lpp = NULL, $p_type = NULL)
{
    $html='';

    if($p_type !== NULL)
    { $page_type = '&'.$p_type; }
    else{$page_type='';}

    if ($rc > $pp)
    {
        $html .= '<div class="pagination">';

        $prev = $cp - 1;
        $next = $cp + 1;
        $page_count = ceil($rc / $pp); // Total number of pages needed to paginate.
        $linkspp = ($lpp !== NULL) ? $lpp : 10; // Number of links to show in between the first and last page numbers if the number of pages exceeds the $linkspp.
        $median = floor($linkspp / 2); // Number of links to display on either side of the current page.
        $multiplier1 = ceil($linkspp / 2);
        $multiplier2 = $linkspp - 1;

        // Starting page number to display.
        $start_count = (($cp - $median) <=1) ? 1 : $cp - $median;

        // Checks if the we have correct number of page links being displayed.
        $start_count = (($cp + $median) >= $page_count) ? $start_count - (($cp + $median) - $page_count) : $start_count;

        // Checks if we are at the end of the page numbering.
        $start_count = (($start_count + $multiplier1) >= $page_count) ? $page_count - $multiplier2 : $start_count;

        //Checks if the starting page number is greater than the page count.
        $start_count = ($linkspp > $page_count) ? $linkspp = $page_count : $start_count;

        // Checks if the page count is equal to the number of pages being displayed.
        $start_count = ($linkspp == $page_count) ? 1: $start_count;

        for($t=0; $t<$linkspp; $t++)
        {
            $page_links[$start_count] = $start_count;
            $start_count++;
        }

        if($cp == $page_count)
        {$page_links[$page_count] = $page_count;}

        if ($cp > 1)
        {$html .= '<a href="'.body_id.'.php?page='.$prev.$page_type.'">Prev</a> ';}

        if(!in_array(1, $page_links))
        {$html .= '<a href="'.body_id.'.php?page=1'.$page_type.'">1</a> ... ';}

        foreach($page_links as $key => $val)
        {
            if($key > $page_count){}
            else
            {
                if ($cp!=$val || $cp == 1)
                {$html .= '<a href="'.body_id.'.php?page='.$val.$page_type.'">'.$val.'</a> ';}
                else
                {$html .= '<span class="current-page"><a href="'.body_id.'.php?page='.$val.$page_type.'">'.$val.'</a></span> ';}
            }
        }

        if(array_key_exists("$page_count", $page_links) === FALSE)
        {$html .= '... <a href="'.body_id.'.php?page='.$page_count.$page_type.'">'.$page_count.'</a> ';}

        if ($cp < $page_count)
        {$html .= ' <a href="'.body_id.'.php?page='.$next.$page_type.'">Next</a>';}

        $html .= '<br/><br/></div>';
    }
    else {}

    return $html;
}

$start = startValue($page, $pp);
$searchTerm = trim($_POST['term']);
$get = mysql_query("SELECT SQL_CALC_FOUND_ROWS id, name, caption FROM images WHERE caption LIKE '%".$searchTerm."%' ORDER BY id DESC LIMIT $start, $pp") or die(mysql_error());
$result_count = mysql_query("SELECT FOUND_ROWS();");
$rowResult = mysql_fetch_array($result_count);
$total_rows = $rowResult[0];


$output = '';
while($rows = mysql_fetch_assoc($get)){

    //$results[] = $rows;
    $output.= "<div class=\"container_image\">";
    $output.= "<a href=\"/pic-".$rows['id'].".html\"><img src=\"/upload/".$rows['name']."\" width=\"210\" height=\"150\"/></a>";
    $output.= "</div>";
}

Edited July 21, 2014 by vinsb

[fastsol]

I don't see the paginate() being called in the code you posted.  It also sounds like you didn't put the additional $_GET vars in the paginate parameters for the search term which is why it all the sudden grows to 240 pages.  As it's already been suggested, you need to change your search form to GET instead of POST.  This makes it much easier to paginate and that way when someone hits the back button in the browser the search term is reatined and the form doesn't have to be submitted again.

[vinsb]

I don't see the paginate() being called in the code you posted.  It also sounds like you didn't put the additional $_GET vars in the paginate parameters for the search term which is why it all the sudden grows to 240 pages.  As it's already been suggested, you need to change your search form to GET instead of POST.  This makes it much easier to paginate and that way when someone hits the back button in the browser the search term is reatined and the form doesn't have to be submitted again.

With method="get" on my form doens't search at all. Just reloading the entire page..

<form action="../search.php" class="search-wrapper cf" method="get">
    <input type="text" placeholder="Search here..." required="" name="term">
    <button type="submit">Search</button>
</form>

Also I called the pagination later in my html

<div id="main">
            <div id="column_left">
                <?php echo $output; ?>

                <div style=" margin-left:100px; margin-top: 5px; height: 30px; width: 400px;"><?php echo paginate($total_rows, $pp, $page, $display_links); ?></div>
            </div>
            <?php include 'include/right.php'; ?>
            <?php include 'include/footer.php'; ?><br/>
</div>

Edited July 21, 2014 by vinsb

[fastsol]

You would also need to change this line in the code to pickup the GET instead

$searchTerm = trim($_POST['term']);

// Now needs to be this
$searchTerm = htmlentities(trim($_GET['term'])); // I would add the htmlentities just incase you are displaying the search term anywhere on the page.  This will prevent XSS attack.

Then change the paginate call to this

paginate($total_rows, $pp, $page, $display_links, 'term='.$searchTerm);

[vinsb]

 

You would also need to change this line in the code to pickup the GET instead

$searchTerm = trim($_POST['term']);

// Now needs to be this
$searchTerm = htmlentities(trim($_GET['term'])); // I would add the htmlentities just incase you are displaying the search term anywhere on the page.  This will prevent XSS attack.

Then change the paginate call to this

paginate($total_rows, $pp, $page, $display_links, 'term='.$searchTerm);

Now that's working perfectly. Just last question because I'm not very familiar with sql injections and etc. 

Is it secure enought this code? 

Also what can I make if someone enter something like "' OR 1=1 #" to show some message for error because if I search for this is load whole images from database without pagination.. and they are ~5000 images so far. It's loading them on the page and this slowing the entire site.

Is this ok for major sql injections?

if (strstr($queryString,"<") || 
strstr($queryString,">") || 
strstr($queryString,"(") || 
strstr($queryString,")") || 
strstr($queryString,"..") || 
strstr($queryString,"%") || 
strstr($queryString,"*") || 
strstr($queryString,"+") || 
strstr($queryString,"!") || 
strstr($queryString,"@") || 
preg_match('/union.*select/i', $queryString) || 
preg_match('/exec/i', $queryString) || 
preg_match('/declare/i', $queryString) || 
preg_match('/drop/i', $queryString) )
 { 
$filename = '/logs/sql_injection_log_'.date('Y-m-d').'.txt'; 
$fh = fopen($filename, 'a'); 
fwrite($fh, $data); fclose($fh); // Send an email to the administrator 
$message = 'SqlInjectionAlarm function '.wordwrap($data, 70); 
$headers = 'From: site' . "\r\n" . 'Reply-To: '.$email. "\r\n" . 'X-Mailer: PHP/' . phpversion(); // Send mail('', 'Possible Hack Attempt', $message, $headers); 
header('Location: http://www.google.com'); exit();}

 

 

edit:

 

Also if I enter in search bar this:

<script>alert('Some JavaScript code')</script>

I get this massages

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 'Some JavaScript code')</script>%' ORDER BY id DESC LIMIT 0, 6' at line 1

I think I must hide this somehow.

Edited July 21, 2014 by vinsb

[fastsol]

It's been so long since I have used the old mysql extensions, I forgot that you are totally open to sql injection in your query.  Add this line after the $searchTerm line I provided above.

$searchTerm = mysql_real_escape_string($searchTerm);

That will prevent sql injection in the query.  You really should consider updating your code to PDO or mysqli.  The mysql extensions have been deprecated and will likely be removed from php in the future.

[vinsb]

It's been so long since I have used the old mysql extensions, I forgot that you are totally open to sql injection in your query.  Add this line after the $searchTerm line I provided above.

$searchTerm = mysql_real_escape_string($searchTerm);

That will prevent sql injection in the query.  You really should consider updating your code to PDO or mysqli.  The mysql extensions have been deprecated and will likely be removed from php in the future.

Yes, it"s better now. Doesn't show any massages or something. Just reloading the page. I will convert this to mysli_* because I know nothing from PDO yet. 

Thank's for your help!

[Jacques1]

I will convert this to mysli_* because I know nothing from PDO yet.

 

This makes no sense. MySQLi is much more difficult to use than PDO. And of course it's limited to MySQL, which is another downside. So if you rewrite your database code, do it properly and go with PDO.

 

If you were planning to cheat and merely add an “i” to all mysql_* functions, this doesn't get you anywhere. It's still the same insecure code. Sure, you'll get rid of the deprecation warnings, but that's not worth the effort.

[vinsb]

This makes no sense. MySQLi is much more difficult to use than PDO. And of course it's limited to MySQL, which is another downside. So if you rewrite your database code, do it properly and go with PDO.

 

If you were planning to cheat and merely add an “i” to all mysql_* functions, this doesn't get you anywhere. It's still the same insecure code. Sure, you'll get rid of the deprecation warnings, but that's not worth the effort.

Thank you for your replay. I must confess I'm kinda new in php&mysql at all.. not complete beginner but still learning. I have writen few codes in mysqli_* but never in PDO that's why I think PDO is harder for newbie. Also OOP at all..

Edited July 21, 2014 by vinsb

[fastsol]

I understand the concern you have with PDO, but honestly it's pretty easy to use.  I learned on mysql too and decided to switch a while back when I was still learning a lot of php, so I too didn't really know what I was getting in to.  But I converted my whole cms in a matter of hours, so it was actually pretty easy to learn.  If you decide to change, the pagination tool can do PDO also.

[vinsb]

I want and I must change one day to PDO. But I think I must first learn php&mysql and then switch to PDO or I can go directly to PDO. As I said I'm not totaly beginner I have some background. Anyway will do it sooner or later. Thank you guys again for your help!

[Jacques1]

It makes no sense to learn the obsolete MySQL extension and then start over with PDO. Just use PDO from the start. The old extension is dead since more than a decade; the only reason why it's still around is because of legacy applications, old tutorials and clueless teachers.

 

The old MySQL extension reflects the technology of the 90s. A lot has changed since. For example, we use prepared statements to securely pass values to a query. We employ exceptions to represent errors in a smart way. And we make use of object-oriented programming. You should start right in the 21st century instead of going back in time. What do you expect to find there?

Edited July 21, 2014 by Jacques1

[vinsb]

I will try to go directly to PDO and OOP. In fact I have upload form for images/gifs/videos .. and it's with prepared statements.

$query = "INSERT INTO images (caption, name, size, type, file_path, alt, img_category) VALUES (?,?,?,?,?,?,?)";
            $conn = $db->prepare($query);
            if ($conn == TRUE) {
                $conn->bind_param("ssissss",$caption, $myFile, $fileSize, $fileType, $path, $alt, $category);
                if (!$conn->execute()) {
                    echo 'error insert';
                } else {