Merge pull request #14 from zopefoundation/adamg-input-protect

Protect against bad input in request parameters

CHANGES.rst

@@ -9,6 +9,8 @@ CHANGES
 
 - Drop support for Python 2.7, 3.5, 3.6.
 
+- Protect against bad input in request parameters -- don't fail hard, use defaults
+
 
 2.2 (2022-02-11)
 ----------------

src/z3c/table/batch.rst

@@ -650,3 +650,187 @@ Update the table and render the batch:
   <a href="...html?table-batchSize=5&table-batchStart=100&table-sortOn=table-number-1" class="current">21</a>
   xxx
   <a href="...html?table-batchSize=5&table-batchStart=1015&table-sortOn=table-number-1" class="last">204</a>
+
+Edge cases, do not fail hard when someone tries some weird batching values:
+
+  >>> batchingRequest = TestRequest(form={'table-batchStart': '11',
+  ...                                     'table-batchSize': 'foobar',
+  ...                                     'table-sortOn': 'table-number-1'})
+  >>> requestBatchingTable = SimpleTable(container, batchingRequest)
+  >>> requestBatchingTable.cssClassSortedOn = None
+
+  >>> requestBatchingTable.__parent__ = container
+  >>> requestBatchingTable.__name__ = u'requestBatchingTable.html'
+  >>> requestBatchingTable.batchSize = 3
+
+  >>> requestBatchingTable.startBatchingAt = 5
+  >>> requestBatchingTable.update()
+  >>> print(requestBatchingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th>My items</th>
+        <th>Number</th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>Twelfth item</td>
+        <td>number: 12</td>
+      </tr>
+      <tr>
+        <td>Thirteenth item</td>
+        <td>number: 13</td>
+      </tr>
+      <tr>
+        <td>Fourteenth item</td>
+        <td>number: 14</td>
+      </tr>
+    </tbody>
+  </table>
+
+
+  >>> batchingRequest = TestRequest(form={'table-batchStart': '0',
+  ...                                     'table-batchSize': '-10',
+  ...                                     'table-sortOn': 'table-number-1'})
+  >>> requestBatchingTable = SimpleTable(container, batchingRequest)
+  >>> requestBatchingTable.cssClassSortedOn = None
+
+  >>> requestBatchingTable.__parent__ = container
+  >>> requestBatchingTable.__name__ = u'requestBatchingTable.html'
+
+  >>> requestBatchingTable.startBatchingAt = 5
+  >>> requestBatchingTable.batchSize = 3
+  >>> requestBatchingTable.update()
+  >>> print(requestBatchingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th>My items</th>
+        <th>Number</th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>Zero item</td>
+        <td>number: 0</td>
+      </tr>
+      <tr>
+        <td>First item</td>
+        <td>number: 1</td>
+      </tr>
+      <tr>
+        <td>Second item</td>
+        <td>number: 2</td>
+      </tr>
+    </tbody>
+  </table>
+
+
+  >>> batchingRequest = TestRequest(form={'table-batchStart': 'foobar',
+  ...                                     'table-batchSize': '3',
+  ...                                     'table-sortOn': 'table-number-1'})
+  >>> requestBatchingTable = SimpleTable(container, batchingRequest)
+  >>> requestBatchingTable.cssClassSortedOn = None
+
+  >>> requestBatchingTable.__parent__ = container
+  >>> requestBatchingTable.__name__ = u'requestBatchingTable.html'
+
+  >>> requestBatchingTable.startBatchingAt = 5
+  >>> requestBatchingTable.update()
+  >>> print(requestBatchingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th>My items</th>
+        <th>Number</th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>Zero item</td>
+        <td>number: 0</td>
+      </tr>
+      <tr>
+        <td>First item</td>
+        <td>number: 1</td>
+      </tr>
+      <tr>
+        <td>Second item</td>
+        <td>number: 2</td>
+      </tr>
+    </tbody>
+  </table>
+
+
+  >>> batchingRequest = TestRequest(form={'table-batchStart': '99999',
+  ...                                     'table-batchSize': '3',
+  ...                                     'table-sortOn': 'table-number-1'})
+  >>> requestBatchingTable = SimpleTable(container, batchingRequest)
+  >>> requestBatchingTable.cssClassSortedOn = None
+
+  >>> requestBatchingTable.__parent__ = container
+  >>> requestBatchingTable.__name__ = u'requestBatchingTable.html'
+
+  >>> requestBatchingTable.startBatchingAt = 5
+  >>> requestBatchingTable.update()
+  >>> print(requestBatchingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th>My items</th>
+        <th>Number</th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>1017 item</td>
+        <td>number: 1017</td>
+      </tr>
+      <tr>
+        <td>1018 item</td>
+        <td>number: 1018</td>
+      </tr>
+      <tr>
+        <td>1019 item</td>
+        <td>number: 1019</td>
+      </tr>
+    </tbody>
+  </table>
+
+
+  >>> batchingRequest = TestRequest(form={'table-batchStart': '-10',
+  ...                                     'table-batchSize': 'foobar',
+  ...                                     'table-sortOn': 'table-number-1'})
+  >>> requestBatchingTable = SimpleTable(container, batchingRequest)
+  >>> requestBatchingTable.cssClassSortedOn = None
+
+  >>> requestBatchingTable.__parent__ = container
+  >>> requestBatchingTable.__name__ = u'requestBatchingTable.html'
+  >>> requestBatchingTable.batchSize = 3
+
+  >>> requestBatchingTable.startBatchingAt = 5
+  >>> requestBatchingTable.update()
+  >>> print(requestBatchingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th>My items</th>
+        <th>Number</th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>Zero item</td>
+        <td>number: 0</td>
+      </tr>
+      <tr>
+        <td>First item</td>
+        <td>number: 1</td>
+      </tr>
+      <tr>
+        <td>Second item</td>
+        <td>number: 2</td>
+      </tr>
+    </tbody>
+  </table>

src/z3c/table/header.py

@@ -21,6 +21,7 @@
 
 import z3c.table.interfaces
 from z3c.table.i18n import _
+from z3c.table.table import getCurrentSortID
 
 
 @zope.interface.implementer(z3c.table.interfaces.IColumnHeader)
@@ -68,20 +69,14 @@ def render(self):
         prefix = table.prefix
         colID = self.column.id
 
-        # this may return a string 'id-name-idx' if coming from request,
-        # otherwise in Table class it is intialised as a integer string
-        currentSortID = table.getSortOn()
-        try:
-            currentSortID = int(currentSortID)
-        except ValueError:
-            currentSortID = currentSortID.rsplit("-", 1)[-1]
+        currentSortID = getCurrentSortID(table.getSortOn())
 
         currentSortOrder = table.getSortOrder()
 
         sortID = colID.rsplit("-", 1)[-1]
 
         sortOrder = table.sortOrder
-        if int(sortID) == int(currentSortID):
+        if int(sortID) == currentSortID:
             # ordering the same column so we want to reverse the order
             if currentSortOrder in table.reverseSortOrderNames:
                 sortOrder = "ascending"

src/z3c/table/sort.rst

@@ -379,3 +379,42 @@ Let's see the `title` column but descending:
       </tr>
     </tbody>
   </table>
+
+
+Edge case, do not fail hard when someone tries some weird sortOn value:
+
+  >>> sortingTable.sortOn = u'table-title-foobar'
+  >>> sortingTable.sortOrder = 'ascending'
+
+  >>> sortingTable.update()
+  >>> print(sortingTable.render())
+  <table>
+    <thead>
+      <tr>
+        <th><a href="?table-sortOn=table-title-0&table-sortOrder=ascending" title="Sort">Title</a></th>
+        <th><a href="?table-sortOn=table-number-1&table-sortOrder=ascending" title="Sort">Number</a></th>
+      </tr>
+    </thead>
+    <tbody>
+      <tr>
+        <td>Title: First</td>
+        <td>number: 1</td>
+      </tr>
+      <tr>
+        <td>Title: Fourth</td>
+        <td>number: 4</td>
+      </tr>
+      <tr>
+        <td>Title: Second</td>
+        <td>number: 2</td>
+      </tr>
+      <tr>
+        <td>Title: Third</td>
+        <td>number: 3</td>
+      </tr>
+      <tr>
+        <td>Title: Zero</td>
+        <td>number: 0</td>
+      </tr>
+    </tbody>
+  </table>

src/z3c/table/table.py

@@ -42,6 +42,20 @@ def getColumnSortKey(sublist):
     return getSortKey
 
 
+def getCurrentSortID(sortOn):
+    # this may return a string 'id-name-idx' if coming from request,
+    # otherwise in Table class it is intialised as a integer string
+    try:
+        currentSortID = int(sortOn)
+    except ValueError:
+        currentSortID = sortOn.rsplit("-", 1)[-1]
+    try:
+        currentSortID = int(currentSortID)
+    except ValueError:
+        return None
+    return currentSortID
+
+
 def nameColumn(column, name):
     """Give a column a __name__."""
     column.__name__ = name
@@ -110,13 +124,10 @@ def getCSSHighlightClass(self, column, item, cssClass):
     def getCSSSortClass(self, column, cssClass):
         """Add CSS class based on current sorting"""
         if self.cssClassSortedOn and self.sortOn is not None:
-            try:
-                currentSortID = int(self.sortOn)
-            except ValueError:
-                currentSortID = self.sortOn.rsplit("-", 1)[-1]
+            currentSortID = getCurrentSortID(self.sortOn)
             sortID = column.id.rsplit("-", 1)[-1]
 
-            if int(sortID) == int(currentSortID):
+            if int(sortID) == currentSortID:
                 klass = self.cssClassSortedOn + " " + self.sortOrder
                 if cssClass:
                     cssClass += " " + klass
@@ -213,17 +224,32 @@ def sortRows(self):
     # batch
 
     def getBatchSize(self):
-        return int(
-            self.request.get(self.prefix + "-batchSize", self.batchSize)
-        )
+        try:
+            newSize = int(
+                self.request.get(self.prefix + "-batchSize", self.batchSize)
+            )
+        except ValueError:
+            newSize = self.batchSize
+        if newSize < 1:
+            newSize = self.batchSize
+        return newSize
 
     def getBatchStart(self):
-        return int(
-            self.request.get(self.prefix + "-batchStart", self.batchStart)
-        )
+        try:
+            return int(
+                self.request.get(self.prefix + "-batchStart", self.batchStart)
+            )
+        except ValueError:
+            return self.batchStart
 
     def batchRows(self):
-        if len(self.rows) > self.startBatchingAt:
+        length = len(self.rows)
+        if length > self.startBatchingAt:
+            if self.batchStart >= length:
+                self.batchStart = length - self.batchSize
+            if self.batchStart < 0:
+                self.batchStart = 0
+
             self.rows = Batch(
                 self.rows, start=self.batchStart, size=self.batchSize
             )