From fc40a6504b78e604a6ba90a3b85b1ed54ee238a2 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 9 Jun 2012 17:55:24 +0200 Subject: [PATCH] Fixed #17159 -- Validated returned number of next|previous_page_number Thanks mehta.apurva at gmail.com for the report and the initial patch and neaf for the complete patch. --- django/core/paginator.py | 4 ++-- docs/releases/1.5.txt | 10 ++++++++++ docs/topics/pagination.txt | 14 ++++++++++---- tests/modeltests/pagination/tests.py | 26 +++++++++++++------------- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/django/core/paginator.py b/django/core/paginator.py index 8b4d2891b2..6b0b3542f8 100644 --- a/django/core/paginator.py +++ b/django/core/paginator.py @@ -132,10 +132,10 @@ class Page(object): return self.has_previous() or self.has_next() def next_page_number(self): - return self.number + 1 + return self.paginator.validate_number(self.number + 1) def previous_page_number(self): - return self.number - 1 + return self.paginator.validate_number(self.number - 1) def start_index(self): """ diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 6512560b50..3e274b5d98 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -154,6 +154,16 @@ Unicode parameters (``password``, ``salt`` or ``encoded``). If any of the hashing methods need byte strings, you can use the :func:`~django.utils.encoding.smart_str` utility to encode the strings. +Validation of previous_page_number and next_page_number +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When using :doc:`object pagination `, +the ``previous_page_number()`` and ``next_page_number()`` methods of the +:class:`~django.core.paginator.Page` object did not check if the returned +number was inside the existing page range. +It does check it now and raises an :exc:`InvalidPage` exception when the number +is either too low or too high. + Features deprecated in 1.5 ========================== diff --git a/docs/topics/pagination.txt b/docs/topics/pagination.txt index 566319ff0a..72560469a8 100644 --- a/docs/topics/pagination.txt +++ b/docs/topics/pagination.txt @@ -253,13 +253,19 @@ Methods .. method:: Page.next_page_number() - Returns the next page number. Note that this is "dumb" and will return the - next page number regardless of whether a subsequent page exists. + Returns the next page number. + + .. versionchanged:: 1.5 + + Raises :exc:`InvalidPage` if next page doesn't exist. .. method:: Page.previous_page_number() - Returns the previous page number. Note that this is "dumb" and will return - the previous page number regardless of whether a previous page exists. + Returns the previous page number. + + .. versionchanged:: 1.5 + + Raises :exc:`InvalidPage` if previous page doesn't exist. .. method:: Page.start_index() diff --git a/tests/modeltests/pagination/tests.py b/tests/modeltests/pagination/tests.py index 9ced9edff2..4d5d8680e4 100644 --- a/tests/modeltests/pagination/tests.py +++ b/tests/modeltests/pagination/tests.py @@ -45,7 +45,7 @@ class PaginationTests(TestCase): self.assertFalse(p.has_previous()) self.assertTrue(p.has_other_pages()) self.assertEqual(2, p.next_page_number()) - self.assertEqual(0, p.previous_page_number()) + self.assertRaises(InvalidPage, p.previous_page_number) self.assertEqual(1, p.start_index()) self.assertEqual(5, p.end_index()) @@ -63,7 +63,7 @@ class PaginationTests(TestCase): self.assertFalse(p.has_next()) self.assertTrue(p.has_previous()) self.assertTrue(p.has_other_pages()) - self.assertEqual(3, p.next_page_number()) + self.assertRaises(InvalidPage, p.next_page_number) self.assertEqual(1, p.previous_page_number()) self.assertEqual(6, p.start_index()) self.assertEqual(9, p.end_index()) @@ -104,20 +104,20 @@ class PaginationTests(TestCase): def test_paginate_list(self): # Paginators work with regular lists/tuples, too -- not just with QuerySets. - paginator = Paginator([1, 2, 3, 4, 5, 6, 7, 8, 9], 5) + paginator = Paginator([1, 2, 3, 4, 5, 6, 7, 8, 9], 3) self.assertEqual(9, paginator.count) - self.assertEqual(2, paginator.num_pages) - self.assertEqual([1, 2], paginator.page_range) - p = paginator.page(1) - self.assertEqual("", unicode(p)) - self.assertEqual([1, 2, 3, 4, 5], p.object_list) + self.assertEqual(3, paginator.num_pages) + self.assertEqual([1, 2, 3], paginator.page_range) + p = paginator.page(2) + self.assertEqual("", unicode(p)) + self.assertEqual([4, 5, 6], p.object_list) self.assertTrue(p.has_next()) - self.assertFalse(p.has_previous()) + self.assertTrue(p.has_previous()) self.assertTrue(p.has_other_pages()) - self.assertEqual(2, p.next_page_number()) - self.assertEqual(0, p.previous_page_number()) - self.assertEqual(1, p.start_index()) - self.assertEqual(5, p.end_index()) + self.assertEqual(3, p.next_page_number()) + self.assertEqual(1, p.previous_page_number()) + self.assertEqual(4, p.start_index()) + self.assertEqual(6, p.end_index()) def test_paginate_misc_classes(self): # Paginator can be passed other objects with a count() method.