magento2: Pagination returning wrong results

Preconditions

  1. XAMPP, Windows 10
  2. Magento 2.1

Steps to reproduce

  1. Installed Magento 2.1 with Sample Data, no other extension installed. Except this custom module.
  2. The code, assuming the setCurPage increases each time
$categoryProducts = $categoryFactory->create()
							      ->load($categoryId)
			    				      ->getProductCollection()
							      ->setPageSize(5)
							      ->setCurPage(1); 
  1. This category should have 10 products, Page 1 -> 5 products shown Page 2 -> 5 products shown Page 3 -> Same products from Page 2.

Expected result

  1. Page 3 shouldn’t have shown any products at all since there is only 10 products.

Actual result

  1. This category should have 10 products, Page 1 -> 5 products shown Page 2 -> 5 products shown Page 3 -> Same products from Page 2.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 5
  • Comments: 16 (10 by maintainers)

Most upvoted comments

I don’t understand the explanation for why this isn’t an issue and it’s intended behavior. As is, how is a developer supposed to know that the results are the last page or they are on a page that has no results. From my tests, you can requests infinitely a number of pages and the application will always return the results from the last page. The api response does not indicate also the total number of pages, seems like this functionality isn’t quite finished but judging from the time this has been reported and the fact it is closed clearly indicates that Magento core team has no intention to service this issue so a manual work around is required.

As explained above,

For once, you can type it directly, this can be reproduced on stock magento: http://magento2-demo.nexcess.net/women/tops-women/tanks-women.html?p=-10 http://magento2-demo.nexcess.net/women/tops-women/tanks-women.html?p=100500 Or even: http://magento2-demo.nexcess.net/women/tops-women/tanks-women.html?p=asdasdasd

is the only thing which may be considered as a bug since it may lead to DoS attack. Collection in it’s current implementation works as intended.

Closing as original scenario is not an issue but rather a bug in described custom implementation. Feel free to report a new issue describing a probable DoS attack (note https://github.com/magento/magento2#reporting-security-issues) or propose a pull request containing suggested changes.

I met this problem today and resolved it by using this: $coll->setCurPage($page)->setPageSize($pageSize); if(($coll->getSize()+$pageSize) >= ($page*$pageSize)) { $info= $coll->getData(); } else { $info= []; }

I confirm this issue, I happened to see this effect myself recently, thus I shall provide some additional background information about the issue: The limit seems to get applied to the collection here and the getCurPage() method has some extra logic which always provides a valid (wrong, if the original page number you’ve sent in is negative or too large).

\Magento\Framework\Data\Collection gives out first or last page’s result when the requested page number is invalid. It shouldn’t be the default behavior because it is completely unreasonable. This is clearly a bug!

As a workaround for your custom code, I’d advise to add an extra check just like the Magento’s own Pager block does. In your example, on the page 2 (last page) it will prevent the “next page” button from appearing.

Back to the collection’s implementation, I see the following possible solutions: a) move those checks from getCurPage() to setCurPage() forcing it to throw an \InvalidArgumentException if the requested page number is not available b) catch it when loading the collection and return an empty array instead.

@korostii

Updated as requested.