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 9c2e6cc0aa8..40179b9c70d 100644
--- a/cinder/api/contrib/volume_actions.py
+++ b/cinder/api/contrib/volume_actions.py
@@ -184,8 +184,10 @@ def _initialize_connection(self, req, id, body):
             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 @@ def _terminate_connection(self, req, id, body):
             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 3312879cf9f..45658ac04e9 100644
--- a/cinder/tests/api/contrib/test_volume_actions.py
+++ b/cinder/tests/api/contrib/test_volume_actions.py
@@ -78,6 +78,21 @@ def fake_initialize_connection(*args, **kwargs):
         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 @@ def fake_terminate_connection(*args, **kwargs):
         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',