From 60f5b775eb1306c2460a73940ef709ed62eae4e2 Mon Sep 17 00:00:00 2001
From: huangtianhua <huangtianhua@huawei.com>
Date: Fri, 22 Nov 2013 17:02:32 +0800
Subject: [PATCH] Initialize and terminate connection raise 500 err

Initialize and terminate connection should check whether "connector" is
in request body. It throws 500 error if "connector" is not present. We
should catch the KeyError and transfer it to 400 (HTTPBadRequest)
instead of.

Closes-Bug: #1253944
Change-Id: If38419592701c8a14df52f94fd46ed0fc7a17e04
---
 cinder/api/contrib/volume_actions.py          | 12 +++++---
 .../tests/api/contrib/test_volume_actions.py  | 30 +++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py
index 9c2e6cc0a..40179b9c7 100644
--- a/cinder/api/contrib/volume_actions.py
+++ b/cinder/api/contrib/volume_actions.py
@@ -184,8 +184,10 @@ class VolumeActionsController(wsgi.Controller):
             volume = self.volume_api.get(context, id)
         except exception.VolumeNotFound as error:
             raise webob.exc.HTTPNotFound(explanation=error.msg)
-
-        connector = body['os-initialize_connection']['connector']
+        try:
+            connector = body['os-initialize_connection']['connector']
+        except KeyError:
+            raise webob.exc.HTTPBadRequest("Must specify 'connector'")
         info = self.volume_api.initialize_connection(context,
                                                      volume,
                                                      connector)
@@ -199,8 +201,10 @@ class VolumeActionsController(wsgi.Controller):
             volume = self.volume_api.get(context, id)
         except exception.VolumeNotFound as error:
             raise webob.exc.HTTPNotFound(explanation=error.msg)
-
-        connector = body['os-terminate_connection']['connector']
+        try:
+            connector = body['os-terminate_connection']['connector']
+        except KeyError:
+            raise webob.exc.HTTPBadRequest("Must specify 'connector'")
         self.volume_api.terminate_connection(context, volume, connector)
         return webob.Response(status_int=202)
 
diff --git a/cinder/tests/api/contrib/test_volume_actions.py b/cinder/tests/api/contrib/test_volume_actions.py
index 3312879cf..45658ac04 100644
--- a/cinder/tests/api/contrib/test_volume_actions.py
+++ b/cinder/tests/api/contrib/test_volume_actions.py
@@ -78,6 +78,21 @@ class VolumeActionsTest(test.TestCase):
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 200)
 
+    def test_initialize_connection_without_connector(self):
+        def fake_initialize_connection(*args, **kwargs):
+            return {}
+        self.stubs.Set(volume.API, 'initialize_connection',
+                       fake_initialize_connection)
+
+        body = {'os-initialize_connection': {}}
+        req = webob.Request.blank('/v2/fake/volumes/1/action')
+        req.method = "POST"
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 400)
+
     def test_terminate_connection(self):
         def fake_terminate_connection(*args, **kwargs):
             return {}
@@ -93,6 +108,21 @@ class VolumeActionsTest(test.TestCase):
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 202)
 
+    def test_terminate_connection_without_connector(self):
+        def fake_terminate_connection(*args, **kwargs):
+            return {}
+        self.stubs.Set(volume.API, 'terminate_connection',
+                       fake_terminate_connection)
+
+        body = {'os-terminate_connection': {}}
+        req = webob.Request.blank('/v2/fake/volumes/1/action')
+        req.method = "POST"
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 400)
+
     def test_attach_to_instance(self):
         body = {'os-attach': {'instance_uuid': 'fake',
                               'mountpoint': '/dev/vdc',
-- 
2.45.2