From 281a78e0af3819e7de3ed84ddb83ec93ac0cc281 Mon Sep 17 00:00:00 2001
From: Chris Dent <cdent@anticdent.org>
Date: Wed, 14 Sep 2016 15:18:30 +0100
Subject: [PATCH] [placement] prevent a KeyError in webob.dec.wsgify

If a PUT, POST or PATCH is sent without a content-type header,
webob.dec.wsgify will raise a KeyError. Avoid this by checking for
the content-type header before reaching any wsgify calls. As noted
in the TODO within this is not the most elegant solution, but
prevents an inadvertent 500 and returns a reasonable 400.

Change-Id: I6e7dffb5dc5f0cdc78a57e8df3ae9952c55163ae
Closes-Bug: #1623517
---
 nova/api/openstack/placement/handler.py         | 17 ++++++++++++++++-
 .../openstack/placement/gabbits/basic-http.yaml |  5 +++++
 .../placement/gabbits/microversion.yaml         |  3 +++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/nova/api/openstack/placement/handler.py b/nova/api/openstack/placement/handler.py
index e0070769ebc4..8a6d8ad5f9bd 100644
--- a/nova/api/openstack/placement/handler.py
+++ b/nova/api/openstack/placement/handler.py
@@ -35,7 +35,7 @@ from nova.api.openstack.placement.handlers import root
 from nova.api.openstack.placement.handlers import usage
 from nova.api.openstack.placement import util
 from nova import exception
-from nova.i18n import _LE
+from nova.i18n import _, _LE
 
 LOG = logging.getLogger(__name__)
 
@@ -154,6 +154,21 @@ class PlacementHandler(object):
                 raise webob.exc.HTTPForbidden(
                     'admin required',
                     json_formatter=util.json_error_formatter)
+        # Check that an incoming write-oriented request method has
+        # the required content-type header. If not raise a 400. If
+        # this doesn't happen here then webob.dec.wsgify (elsewhere
+        # in the stack) will raise an uncaught KeyError. Since that
+        # is such a generic exception we cannot merely catch it
+        # here, we need to avoid it ever happening.
+        # TODO(cdent): Move this and the auth checking above into
+        # middleware. It shouldn't be here. This is for dispatch not
+        # validation or authorization.
+        request_method = environ['REQUEST_METHOD'].upper()
+        if request_method in ('POST', 'PUT', 'PATCH'):
+            if 'CONTENT_TYPE' not in environ:
+                raise webob.exc.HTTPBadRequest(
+                    _('content-type header required'),
+                    json_formatter=util.json_error_formatter)
         try:
             return dispatch(environ, start_response, self._map)
         # Trap the small number of nova exceptions that aren't
diff --git a/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml b/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml
index 91efd558551d..00757ebba5c5 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml
@@ -73,6 +73,11 @@ tests:
   data: I want a resource provider please
   status: 415
 
+- name: post resource provider missing content-type
+  POST: /resource_providers
+  data: I want a resource provider please
+  status: 400
+
 - name: post resource provider schema mismatch
   POST: /resource_providers
   request_headers:
diff --git a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml
index 4fe986ea5529..a23effcabfcf 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/microversion.yaml
@@ -73,7 +73,10 @@ tests:
       - "invalid version string: 1.2.3"
 
 - name: error in application produces microversion headers
+  desc: we do not want xml
   POST: /
+  request_headers:
+      content-type: application/xml
   status: 405
   response_headers:
       openstack-api-version: placement 1.0