Merge lp:~blamar/nova/negative-limits-offsets into lp:~hudson-openstack/nova/trunk

[Brian Lamar]

Very simple change checking for < 0 values in "limit" and "offset" GET parameters. If either are negative, raise a HTTPBadRequest exception. Relevant tests included.

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Rick Harris (rconradharris)]

lgtm

review: Approve

=== modified file 'nova/api/openstack/common.py'
--- nova/api/openstack/common.py	2011-02-09 18:30:40 +0000
+++ nova/api/openstack/common.py	2011-03-01 21:04:33 +0000
@@ -15,6 +15,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import webob.exc
+
 from nova import exception
 
 
@@ -27,7 +29,8 @@
                     GET variables. 'offset' is where to start in the list,
                     and 'limit' is the maximum number of items to return. If
                     'limit' is not specified, 0, or > max_limit, we default
-                    to max_limit.
+                    to max_limit. Negative values for either offset or limit
+                    will cause exc.HTTPBadRequest() exceptions to be raised.
     @kwarg max_limit: The maximum number of items to return from 'items'
     """
     try:
@@ -40,6 +43,9 @@
     except ValueError:
         limit = max_limit
 
+    if offset < 0 or limit < 0:
+        raise webob.exc.HTTPBadRequest()
+
     limit = min(max_limit, limit or max_limit)
     range_end = offset + limit
     return items[offset:range_end]

=== modified file 'nova/tests/api/openstack/test_common.py'
--- nova/tests/api/openstack/test_common.py	2011-02-23 19:56:37 +0000
+++ nova/tests/api/openstack/test_common.py	2011-03-01 21:04:33 +0000
@@ -19,6 +19,7 @@
 Test suites for 'common' code used throughout the OpenStack HTTP API.
 """
 
+import webob.exc
 
 from webob import Request
 
@@ -160,3 +161,23 @@
         self.assertEqual(limited(items, req, max_limit=2000), items[3:])
         req = Request.blank('/?offset=3000&limit=10')
         self.assertEqual(limited(items, req, max_limit=2000), [])
+
+    def test_limiter_negative_limit(self):
+        """
+        Test a negative limit.
+        """
+        def _limit_large():
+            limited(self.large, req, max_limit=2000)
+
+        req = Request.blank('/?limit=-3000')
+        self.assertRaises(webob.exc.HTTPBadRequest, _limit_large)
+
+    def test_limiter_negative_offset(self):
+        """
+        Test a negative offset.
+        """
+        def _limit_large():
+            limited(self.large, req, max_limit=2000)
+
+        req = Request.blank('/?offset=-30')
+        self.assertRaises(webob.exc.HTTPBadRequest, _limit_large)