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.

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 </topics/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("<Page 1 of 2>", 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("<Page 2 of 3>", 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.