Improve content-type handling
According to the OCCI specification, the accept header MUST be handled as follows: - If Accept is empty, the returned content-type should be text/plain - If Accept is "*/*", the returned content-type should be text/plain - If Accept cannot be understood, a 406 error should be returned. We are still missing the handling of the content-type header (that must indicate the type of the data being sent, if any) that should be checked against the available parsers (this is still missing).
This commit is contained in:
parent
0b92038f62
commit
59272482f6
|
@ -77,10 +77,18 @@ class Invalid(OCCIException):
|
|||
|
||||
|
||||
class InvalidContentType(Invalid):
|
||||
msg_fmt = "Invalid content type %(content_type)s."
|
||||
msg_fmt = "Invalid Content-type %(content_type)s."
|
||||
code = 406
|
||||
|
||||
|
||||
class NoContentType(InvalidContentType):
|
||||
msg_fmt = "No Content-type provided."
|
||||
|
||||
|
||||
class InvalidAccept(InvalidContentType):
|
||||
msg_fmt = "Invalid Accept %(content_type)s."
|
||||
|
||||
|
||||
class NotImplemented(OCCIException):
|
||||
msg_fmt = "Action not implemented."
|
||||
code = 501
|
||||
|
|
|
@ -36,7 +36,7 @@ class HeaderRenderer(object):
|
|||
|
||||
class ExceptionRenderer(HeaderRenderer):
|
||||
def render(self, env={}):
|
||||
return []
|
||||
return [("X-OCCI-Error", self.obj.explanation)]
|
||||
|
||||
|
||||
class CategoryRenderer(HeaderRenderer):
|
||||
|
|
|
@ -34,14 +34,17 @@ class TestMiddleware(base.TestCase):
|
|||
def setUp(self):
|
||||
super(TestMiddleware, self).setUp()
|
||||
|
||||
self.accept = None
|
||||
self.accept = self.content_type = None
|
||||
self.application_url = fakes.application_url
|
||||
|
||||
def get_app(self, resp=None):
|
||||
return wsgi.OCCIMiddleware(fakes.FakeApp())
|
||||
|
||||
def assertContentType(self, result):
|
||||
expected = self.accept or "text/plain"
|
||||
if self.accept in (None, "*/*"):
|
||||
expected = "text/plain"
|
||||
else:
|
||||
expected = self.accept
|
||||
self.assertEqual(expected, result.content_type)
|
||||
|
||||
def assertExpectedResult(self, expected, result):
|
||||
|
@ -54,6 +57,9 @@ class TestMiddleware(base.TestCase):
|
|||
if self.accept is not None:
|
||||
kwargs["accept"] = self.accept
|
||||
|
||||
if self.content_type is not None:
|
||||
kwargs["content_type"] = self.content_type
|
||||
|
||||
m = mock.MagicMock()
|
||||
m.user.project_id = tenant_id
|
||||
environ = {"keystone.token_auth": m}
|
||||
|
|
|
@ -24,13 +24,12 @@ from ooi import wsgi
|
|||
|
||||
@webob.dec.wsgify
|
||||
def fake_app(req):
|
||||
resp = webob.Response("Hi")
|
||||
resp = webob.Response("Foo")
|
||||
return resp
|
||||
|
||||
|
||||
class FakeController(object):
|
||||
def index(self, *args, **kwargs):
|
||||
# Return none so that the middleware passes to the app
|
||||
return None
|
||||
|
||||
def create(self, req, body):
|
||||
|
@ -63,7 +62,7 @@ class TestMiddleware(base.TestCase):
|
|||
result = webob.Request.blank("/foos",
|
||||
method="GET").get_response(self.app)
|
||||
self.assertEqual(200, result.status_code)
|
||||
self.assertEqual("Hi", result.text)
|
||||
self.assertEqual("", result.text)
|
||||
|
||||
def test_show(self):
|
||||
result = webob.Request.blank("/foos/stop",
|
||||
|
@ -91,10 +90,32 @@ class TestMiddleware(base.TestCase):
|
|||
result = webob.Request.blank("/bazonk").get_response(self.app)
|
||||
self.assertEqual(404, result.status_code)
|
||||
|
||||
def test_empty_accept(self):
|
||||
req = webob.Request.blank("/foos",
|
||||
method="GET",
|
||||
accept=None)
|
||||
result = req.get_response(self.app)
|
||||
self.assertEqual(200, result.status_code)
|
||||
self.assertEqual("text/plain", result.content_type)
|
||||
|
||||
def test_accept_all(self):
|
||||
req = webob.Request.blank("/foos",
|
||||
method="GET",
|
||||
accept="*/*")
|
||||
result = req.get_response(self.app)
|
||||
self.assertEqual(200, result.status_code)
|
||||
self.assertEqual("text/plain", result.content_type)
|
||||
|
||||
def test_bad_accept(self):
|
||||
req = webob.Request.blank("/foos",
|
||||
method="GET",
|
||||
accept="foo/bazonk",
|
||||
accept="foo/bazonk")
|
||||
result = req.get_response(self.app)
|
||||
self.assertEqual(406, result.status_code)
|
||||
|
||||
def test_bad_content_type(self):
|
||||
req = webob.Request.blank("/foos",
|
||||
method="GET",
|
||||
content_type="foo/bazonk")
|
||||
result = req.get_response(self.app)
|
||||
self.assertEqual(406, result.status_code)
|
||||
|
|
|
@ -22,7 +22,6 @@ import webob.dec
|
|||
import ooi.api.compute
|
||||
from ooi.api import query
|
||||
from ooi import exception
|
||||
from ooi.occi.core import collection
|
||||
from ooi import utils
|
||||
from ooi.wsgi import serializers
|
||||
|
||||
|
@ -31,30 +30,25 @@ LOG = logging.getLogger(__name__)
|
|||
|
||||
class Request(webob.Request):
|
||||
def get_content_type(self):
|
||||
"""Determine content type of the request body.
|
||||
|
||||
Does not do any body introspection, only checks header
|
||||
|
||||
"""
|
||||
if "Content-Type" not in self.headers:
|
||||
"""Determine content type of the request body."""
|
||||
if not self.content_type:
|
||||
return None
|
||||
|
||||
content_type = self.content_type
|
||||
# FIXME: we should change this, since the content type does not depend
|
||||
# on the serializers, but on the parsers
|
||||
if self.content_type not in serializers.get_supported_content_types():
|
||||
LOG.debug("Unrecognized Content-Type provided in request")
|
||||
raise exception.InvalidContentType(content_type=self.content_type)
|
||||
|
||||
if not content_type or content_type == 'text/plain':
|
||||
return None
|
||||
return self.content_type
|
||||
|
||||
if content_type not in serializers.get_supported_content_types():
|
||||
raise exception.InvalidContentType(content_type=content_type)
|
||||
|
||||
return content_type
|
||||
|
||||
def get_best_match_content_type(self):
|
||||
content_type = self.get_content_type()
|
||||
if content_type is None:
|
||||
content_types = serializers.get_supported_content_types()
|
||||
content_type = self.accept.best_match(content_types,
|
||||
default_match="text/plain")
|
||||
def get_best_match_content_type(self, default_match=None):
|
||||
content_types = serializers.get_supported_content_types()
|
||||
content_type = self.accept.best_match(content_types,
|
||||
default_match=default_match)
|
||||
if not content_type:
|
||||
LOG.debug("Unrecognized Accept Content-type provided in request")
|
||||
raise exception.InvalidAccept(content_type=content_type)
|
||||
return content_type
|
||||
|
||||
|
||||
|
@ -164,15 +158,6 @@ class Resource(object):
|
|||
|
||||
return args
|
||||
|
||||
def get_body(self, request):
|
||||
try:
|
||||
content_type = request.get_content_type()
|
||||
except exception.InvalidContentType:
|
||||
LOG.debug("Unrecognized Content-Type provided in request")
|
||||
return None, ''
|
||||
|
||||
return content_type, request.body
|
||||
|
||||
@staticmethod
|
||||
def _should_have_body(request):
|
||||
return request.method in ("POST", "PUT")
|
||||
|
@ -183,11 +168,13 @@ class Resource(object):
|
|||
action = action_args.pop('action', None)
|
||||
try:
|
||||
accept = request.get_best_match_content_type()
|
||||
except exception.InvalidContentType:
|
||||
msg = "Unsupported Content-Type"
|
||||
content_type = request.get_content_type()
|
||||
except exception.InvalidContentType as e:
|
||||
msg = e.format_message()
|
||||
return Fault(webob.exc.HTTPNotAcceptable(explanation=msg))
|
||||
|
||||
content_type, body = self.get_body(request)
|
||||
body = request.body
|
||||
|
||||
# Get the implementing method
|
||||
try:
|
||||
method = self.get_method(request, action,
|
||||
|
@ -216,18 +203,17 @@ class Resource(object):
|
|||
response = ex
|
||||
|
||||
# No exceptions, so create a response
|
||||
# NOTE(aloga): if the middleware returns None, the pipeline will
|
||||
# continue, but we do not want to do so, so we convert the action
|
||||
# result to a ResponseObject.
|
||||
if not response:
|
||||
resp_obj = None
|
||||
# We got something
|
||||
if isinstance(action_result, (list, collection.Collection)):
|
||||
resp_obj = ResponseObject(action_result)
|
||||
elif isinstance(action_result, ResponseObject):
|
||||
if isinstance(action_result, ResponseObject):
|
||||
resp_obj = action_result
|
||||
else:
|
||||
response = action_result
|
||||
if resp_obj and not response:
|
||||
response = resp_obj.serialize(request, accept,
|
||||
self.default_serializers)
|
||||
resp_obj = ResponseObject(action_result)
|
||||
|
||||
response = resp_obj.serialize(request, accept,
|
||||
self.default_serializers)
|
||||
return response
|
||||
|
||||
def get_method(self, request, action, content_type, body):
|
||||
|
@ -293,8 +279,8 @@ class ResponseObject(object):
|
|||
for hdr, value in self._headers.items():
|
||||
response.headers[hdr] = utils.utf8(value)
|
||||
response.headers['Content-Type'] = content_type
|
||||
response.charset = 'utf8'
|
||||
if self.obj is not None:
|
||||
response.charset = 'utf8'
|
||||
headers, body = serializer.serialize(self.obj)
|
||||
if headers is not None:
|
||||
for hdr in headers:
|
||||
|
@ -366,7 +352,7 @@ class Fault(webob.exc.HTTPException):
|
|||
self.wrapped_exc.headers[key] = str(value)
|
||||
self.status_int = exception.status_int
|
||||
|
||||
@webob.dec.wsgify()
|
||||
@webob.dec.wsgify(RequestClass=Request)
|
||||
def __call__(self, req):
|
||||
"""Generate a WSGI response based on the exception passed to ctor."""
|
||||
|
||||
|
@ -376,14 +362,24 @@ class Fault(webob.exc.HTTPException):
|
|||
LOG.debug("Returning %(code)s to user: %(explanation)s",
|
||||
{'code': code, 'explanation': explanation})
|
||||
|
||||
content_type = req.content_type or "text/plain"
|
||||
def_ct = "text/plain"
|
||||
content_type = req.get_best_match_content_type(default_match=def_ct)
|
||||
mtype = serializers.get_media_map().get(content_type,
|
||||
"text")
|
||||
serializer = serializers.get_default_serializers()[mtype]
|
||||
env = {}
|
||||
serialized_exc = serializer(env).serialize(self.wrapped_exc)
|
||||
self.wrapped_exc.body = serialized_exc[1]
|
||||
self.wrapped_exc.content_type = content_type
|
||||
self.wrapped_exc.body = serialized_exc[1]
|
||||
|
||||
# We need to specify the HEAD req.method here to be HEAD because of the
|
||||
# way that webob.exc.WSGIHTTPException.__call__ generates the response.
|
||||
# The text/occi will not have a body since it is based on headers. We
|
||||
# cannot set this earlier in the middleware, since we are calling
|
||||
# OpenStack and it will fail because the responses won't contain a
|
||||
# body.
|
||||
if content_type == "text/occi":
|
||||
req.method = "HEAD"
|
||||
|
||||
return self.wrapped_exc
|
||||
|
||||
|
|
|
@ -57,7 +57,7 @@ class HeaderSerializer(BaseSerializer):
|
|||
# Header renderers will return a list, so we must flatten the results
|
||||
# before returning them
|
||||
headers = [i for r in renderers for i in r.render(env=self.env)]
|
||||
return headers, ""
|
||||
return headers, utils.utf8("")
|
||||
|
||||
|
||||
_SERIALIZERS_MAP = {
|
||||
|
|
Loading…
Reference in New Issue