Return 400 if create volume snapshot force parameter is invalid

Fixes bug #1014689

* Add is_valid_boolstr function in utils.py
* Add force parameter check in SnapshotsController.create()
* Add unittest for invalid force parameter.

Change-Id: Ie6a07a2ac589da76f52a82b126a6d66e5987edc4

5 files changed, 44 insertions, 2 deletions

diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index 41e168411..7194f64e3 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -27,6 +27,7 @@ from nova import compute
 from nova import exception
 from nova import flags
 from nova.openstack.common import log as logging
+from nova import utils
 from nova import volume
 from nova.volume import volume_types
 
@@ -595,7 +596,11 @@ class SnapshotController(object):
         LOG.audit(_("Create snapshot from volume %s"), volume_id,
                 context=context)
 
-        if force:
+        if not utils.is_valid_boolstr(force):
+            msg = _("Invalid value '%s' for force. ") % force
+            raise exception.InvalidParameterValue(err=msg)
+
+        if utils.bool_from_str(force):
             new_snapshot = self.volume_api.create_snapshot_force(context,
                                         volume,
                                         snapshot.get('display_name'),
diff --git a/nova/api/openstack/volume/snapshots.py b/nova/api/openstack/volume/snapshots.py
index 91823de79..755398369 100644
--- a/nova/api/openstack/volume/snapshots.py
+++ b/nova/api/openstack/volume/snapshots.py
@@ -24,6 +24,7 @@ from nova.api.openstack import xmlutil
 from nova import exception
 from nova import flags
 from nova.openstack.common import log as logging
+from nova import utils
 from nova import volume
 
 
@@ -154,7 +155,11 @@ class SnapshotsController(object):
         msg = _("Create snapshot from volume %s")
         LOG.audit(msg, volume_id, context=context)
 
-        if force:
+        if not utils.is_valid_boolstr(force):
+            msg = _("Invalid value '%s' for force. ") % force
+            raise exception.InvalidParameterValue(err=msg)
+
+        if utils.bool_from_str(force):
             new_snapshot = self.volume_api.create_snapshot_force(context,
                                         volume,
                                         snapshot.get('display_name'),
diff --git a/nova/tests/api/openstack/compute/contrib/test_snapshots.py b/nova/tests/api/openstack/compute/contrib/test_snapshots.py
index ad2d9b322..5a9e8dbbb 100644
--- a/nova/tests/api/openstack/compute/contrib/test_snapshots.py
+++ b/nova/tests/api/openstack/compute/contrib/test_snapshots.py
@@ -165,6 +165,19 @@ class SnapshotApiTest(test.TestCase):
         self.assertEqual(resp_dict['snapshot']['displayDescription'],
                         snapshot['display_description'])
 
+        # Test invalid force paramter
+        snapshot = {"volume_id": 12,
+                "force": '**&&^^%%$$##@@'}
+        body = dict(snapshot=snapshot)
+        req = webob.Request.blank('/v2/fake/os-snapshots')
+        req.method = 'POST'
+        req.body = jsonutils.dumps(body)
+        req.headers['content-type'] = 'application/json'
+
+        resp = req.get_response(fakes.wsgi_app())
+        LOG.debug(_("test_snapshot_create_force: param=%s"), _last_param)
+        self.assertEqual(resp.status_int, 400)
+
     def test_snapshot_delete(self):
         global _last_param
         _last_param = {}
diff --git a/nova/tests/api/openstack/volume/test_snapshots.py b/nova/tests/api/openstack/volume/test_snapshots.py
index ef4724338..c65182cb7 100644
--- a/nova/tests/api/openstack/volume/test_snapshots.py
+++ b/nova/tests/api/openstack/volume/test_snapshots.py
@@ -114,6 +114,16 @@ class SnapshotApiTest(test.TestCase):
         self.assertEqual(resp_dict['snapshot']['display_description'],
                         snapshot['display_description'])
 
+        # Test invalid force paramter
+        snapshot = {"volume_id": 12,
+                "force": '**&&^^%%$$##@@'}
+        body = dict(snapshot=snapshot)
+        req = fakes.HTTPRequest.blank('/v1/snapshots')
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.controller.create,
+                          req,
+                          body)
+
     def test_snapshot_delete(self):
         self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get)
         self.stubs.Set(volume.api.API, "delete_snapshot", stub_snapshot_delete)
diff --git a/nova/utils.py b/nova/utils.py
index 31d62a611..41b268c2e 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -941,6 +941,15 @@ def bool_from_str(val):
                val.lower() == 'y'
 
 
+def is_valid_boolstr(val):
+    """Check if the provided string is a valid bool string or not. """
+    val = str(val).lower()
+    return val == 'true' or val == 'false' or \
+           val == 'yes' or val == 'no' or \
+           val == 'y' or val == 'n' or \
+           val == '1' or val == '0'
+
+
 def is_valid_ipv4(address):
     """valid the address strictly as per format xxx.xxx.xxx.xxx.
     where xxx is a value between 0 and 255.