From d3bf010d3d66ad880ff65259119745dfb333a32a Mon Sep 17 00:00:00 2001
From: Chris Dent <cdent@anticdent.org>
Date: Mon, 20 Mar 2017 17:46:27 +0000
Subject: [PATCH] [placement] Allow PUT and POST without bodies

We plan to allow PUT requests to create/update both custom traits
and custom resource classes, without bodies. Prior to this change,
the code would not all a PUT, POST or PATCH to not have a body. This was
added in I6e7dffb5dc5f0cdc78a57e8df3ae9952c55163ae which was fixing an
issue with how webob handles exceptions.

This change does two things:

* It address the problem from bug #1623517, fixed in the change id'd
  above, in a more narrow fashion, making sure the data source that
  causes the KeyError is non-empty right before it is used. This allows
  simplifying the following change.
* If a request has a content-length (indicating the presence of a body),
  verify that there is also a content-type. If not, raise a 400.

basic-http.yaml has been change to modify one gabbi test to check a
response body is correct and to add another test to confirm that the
code that is doing the content-length check is passed through.

Change-Id: Ifb7446fd02ba3e54bbe2676dfd38e5dfecd15f98
Closes-Bug: #1674392
Related-Bug: #1623517
---
 nova/api/openstack/placement/handler.py            | 14 +++-----------
 nova/api/openstack/placement/util.py               |  8 ++++++--
 .../openstack/placement/gabbits/basic-http.yaml    | 14 ++++++++++++++
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/nova/api/openstack/placement/handler.py b/nova/api/openstack/placement/handler.py
index 6c9af91c0..8b0da199a 100644
--- a/nova/api/openstack/placement/handler.py
+++ b/nova/api/openstack/placement/handler.py
@@ -177,17 +177,9 @@ 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'):
+        # Check that an incoming request with a content-length
+        # header also has a content-type header. If not raise a 400.
+        if int(environ.get('CONTENT_LENGTH', 0)):
             if 'CONTENT_TYPE' not in environ:
                 raise webob.exc.HTTPBadRequest(
                     _('content-type header required'),
diff --git a/nova/api/openstack/placement/util.py b/nova/api/openstack/placement/util.py
index e6476e404..71ba4aa2e 100644
--- a/nova/api/openstack/placement/util.py
+++ b/nova/api/openstack/placement/util.py
@@ -123,11 +123,15 @@ def require_content(content_type):
             if req.content_type != content_type:
                 # webob's unset content_type is the empty string so
                 # set it the error message content to 'None' to make
-                # a useful message in that case.
+                # a useful message in that case. This also avoids a
+                # KeyError raised when webob.exc eagerly fills in a
+                # Template for output we will never use.
+                if not req.content_type:
+                    req.content_type = 'None'
                 raise webob.exc.HTTPUnsupportedMediaType(
                     _('The media type %(bad_type)s is not supported, '
                       'use %(good_type)s') %
-                    {'bad_type': req.content_type or 'None',
+                    {'bad_type': req.content_type,
                      'good_type': content_type},
                     json_formatter=json_error_formatter)
             else:
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 fae0d2c24..1c0dd6fcc 100644
--- a/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml
+++ b/nova/tests/functional/api/openstack/placement/gabbits/basic-http.yaml
@@ -84,9 +84,23 @@ tests:
   status: 415
 
 - name: post resource provider missing content-type
+  desc: because content-length is set, we should have a content-type
   POST: /resource_providers
   data: I want a resource provider please
   status: 400
+  response_strings:
+      - content-type header required
+
+# NOTE(cdent): This is an awkward test. It is not actually testing a
+# PUT of a resource provider. It is confirming that a PUT with no
+# body, no content-length header and no content-type header will
+# reach the desired handler.
+- name: PUT resource provider no body
+  desc: different response string from prior test indicates past content-length requirement
+  PUT: /resource_providers/d3a64825-8228-4ccb-8a6c-1c6d3eb6a3e8
+  status: 415
+  response_strings:
+      - The media type None is not supported, use application/json
 
 - name: post resource provider schema mismatch
   POST: /resource_providers