openstack/glance
Code Issues Proposed changes

Raise bad request early if image metadata is invalid

Browse Source 
Image data is currently uploaded into the backend store even if it is a bad
request. This patch add value validation in the deserializer so we don't set bad
data. It also does a full validation of the image metadata to make sure
everything that is needed for an image upload is set correctly before the upload
can occur.

There were currently some relevant tests that had incorrect names causing them
to not run. These have been fixed and modified.

Fixes bug 1035982

Change-Id: I6f3ef83ba7b3de3cb885fad9737fef1a03d9222c
This commit is contained in:
Alex Meade 2012-09-05 13:55:46 -04:00
parent 45198f4a0c
commit 3bfd7bd56c
5 changed files with 119 additions and 217 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
glance/api/v1/images.py
View File

@ -57,6 +57,9 @@ from glance.store import (create_stores,
LOG = logging.getLogger(__name__)
SUPPORTED_PARAMS = glance.api.v1.SUPPORTED_PARAMS
SUPPORTED_FILTERS = glance.api.v1.SUPPORTED_FILTERS
CONTAINER_FORMATS = ['ami', 'ari', 'aki', 'bare', 'ovf']
DISK_FORMATS = ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi',
'iso']
# Defined at module level due to _is_opt_registered
@ -67,6 +70,43 @@ CONF = cfg.CONF
CONF.register_opt(default_store_opt)
def validate_image_meta(req, values):
name = values.get('name')
disk_format = values.get('disk_format')
container_format = values.get('container_format')
if 'disk_format' in values:
if not disk_format in DISK_FORMATS:
msg = "Invalid disk format '%s' for image." % disk_format
raise HTTPBadRequest(explanation=msg, request=req)
if 'container_format' in values:
if not container_format in CONTAINER_FORMATS:
msg = "Invalid container format '%s' for image." % container_format
raise HTTPBadRequest(explanation=msg, request=req)
if name and len(name) > 255:
msg = _('Image name too long: %d') % len(name)
raise HTTPBadRequest(explanation=msg, request=req)
amazon_formats = ('aki', 'ari', 'ami')
if disk_format in amazon_formats or container_format in amazon_formats:
if disk_format is None:
values['disk_format'] = container_format
elif container_format is None:
values['container_format'] = disk_format
elif container_format != disk_format:
msg = ("Invalid mix of disk and container formats. "
"When setting a disk or container format to "
"one of 'aki', 'ari', or 'ami', the container "
"and disk formats must match.")
raise HTTPBadRequest(explanation=msg, request=req)
return values
class Controller(controller.BaseController):
"""
WSGI controller for images resource in Glance v1 API
@ -566,6 +606,8 @@ class Controller(controller.BaseController):
def _handle_source(self, req, image_id, image_meta, image_data):
if image_data:
image_meta = self._validate_image_for_activation(req, image_id,
image_meta)
image_meta = self._upload_and_activate(req, image_meta)
elif self._copy_from(req):
msg = _('Triggering asynchronous copy from external source')
@ -574,9 +616,23 @@ class Controller(controller.BaseController):
else:
location = image_meta.get('location')
if location:
self._validate_image_for_activation(req, image_id, image_meta)
image_meta = self._activate(req, image_id, location)
return image_meta
def _validate_image_for_activation(self, req, id, values):
"""Ensures that all required image metadata values are valid."""
image = self.get_image_meta_or_404(req, id)
if not 'disk_format' in values:
values['disk_format'] = image['disk_format']
if not 'container_format' in values:
values['container_format'] = image['container_format']
if not 'name' in values:
values['name'] = image['name']
values = validate_image_meta(req, values)
return values
@utils.mutating
def create(self, req, image_meta, image_data):
"""
@ -851,6 +907,7 @@ class ImageDeserializer(wsgi.JSONRequestDeserializer):
raise HTTPBadRequest(explanation=msg, request=request)
image_meta = result['image_meta']
image_meta = validate_image_meta(request, image_meta)
if request.content_length:
image_size = request.content_length
elif 'size' in image_meta:

52
glance/db/sqlalchemy/api.py
View File

@ -46,9 +46,6 @@ sa_logger = None
LOG = os_logging.getLogger(__name__)
CONTAINER_FORMATS = ['ami', 'ari', 'aki', 'bare', 'ovf']
DISK_FORMATS = ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi',
'iso']
STATUSES = ['active', 'saving', 'queued', 'killed', 'pending_delete',
'deleted']
@ -535,8 +532,6 @@ def validate_image(values):
:param values: Mapping of image metadata to check
"""
status = values.get('status')
disk_format = values.get('disk_format')
container_format = values.get('container_format')
status = values.get('status', None)
if not status:
@ -547,53 +542,6 @@ def validate_image(values):
msg = "Invalid image status '%s' for image." % status
raise exception.Invalid(msg)
def _amazon_format(disk, container):
amazon_formats = ('aki', 'ari', 'ami')
return ((disk in amazon_formats and
(container in CONTAINER_FORMATS or container is None)) or
(container in amazon_formats and
(disk in DISK_FORMATS or disk is None)))
def _only_one_of(a, b):
return (a and b is None) or (b and a is None)
if _amazon_format(disk_format, container_format):
if _only_one_of(container_format, disk_format):
container_format = (container_format if disk_format is None
else disk_format)
values['container_format'] = container_format
disk_format = container_format
values['disk_format'] = disk_format
elif container_format != disk_format:
msg = ("Invalid mix of disk and container formats. "
"When setting a disk or container format to "
"one of 'aki', 'ari', or 'ami', the container "
"and disk formats must match.")
raise exception.Invalid(msg)
def _required_format_absent(format, formats):
activating = status == 'active'
unrecognized = format not in formats
# We don't mind having format = None when we're just registering
# an image, but if the image is being activated, make sure that the
# format is valid. Conversely if the format happens to be set on
# registration, it must be one of the recognized formats.
return ((activating and (not format or unrecognized))
or (not activating and format and unrecognized))
if _required_format_absent(disk_format, DISK_FORMATS):
msg = "Invalid disk format '%s' for image." % disk_format
raise exception.Invalid(msg)
if _required_format_absent(container_format, CONTAINER_FORMATS):
msg = "Invalid container format '%s' for image." % container_format
raise exception.Invalid(msg)
name = values.get('name')
if name and len(name) > 255:
msg = _('Image name too long: %d') % len(name)
raise exception.Invalid(msg)
return values

43
glance/tests/functional/v1/test_api.py
View File

@ -530,8 +530,11 @@ class TestApi(functional.FunctionalTest):
test_data_file.flush()
path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
http = httplib2.Http()
headers = minimal_headers('Image1')
headers['Content-Type'] = 'not octet-stream'
response, content = http.request(path, 'POST',
body=test_data_file.name)
body=test_data_file.name,
headers=headers)
self.assertEqual(response.status, 400)
expected = "Content-Type must be application/octet-stream"
self.assertTrue(expected in content,
@ -1229,7 +1232,7 @@ class TestApi(functional.FunctionalTest):
expected_exitcode=255,
**self.__dict__.copy())
def _do_test_post_image_content_missing_format(self, format):
def _do_test_post_image_content_bad_format(self, format):
"""
We test that missing container/disk format fails with 400 "Bad Request"
@ -1238,11 +1241,19 @@ class TestApi(functional.FunctionalTest):
self.cleanup()
self.start_servers(**self.__dict__.copy())
# Verify no public images
path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
http = httplib2.Http()
response, content = http.request(path, 'GET')
self.assertEqual(response.status, 200)
images = json.loads(content)['images']
self.assertEqual(len(images), 0)
path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
# POST /images without given format being specified
headers = minimal_headers('Image1')
del headers['X-Image-Meta-' + format]
headers['X-Image-Meta-' + format] = 'bad_value'
with tempfile.NamedTemporaryFile() as test_data_file:
test_data_file.write("XXX")
test_data_file.flush()
@ -1252,19 +1263,28 @@ class TestApi(functional.FunctionalTest):
body=test_data_file.name)
self.assertEqual(response.status, 400)
type = format.replace('_format', '')
expected = "Details: Invalid %s format 'None' for image" % type
expected = "Invalid %s format 'bad_value' for image" % type
self.assertTrue(expected in content,
"Could not find '%s' in '%s'" % (expected, content))
# make sure the image was not created
# Verify no public images
path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
http = httplib2.Http()
response, content = http.request(path, 'GET')
self.assertEqual(response.status, 200)
images = json.loads(content)['images']
self.assertEqual(len(images), 0)
self.stop_servers()
@skip_if_disabled
def _do_test_post_image_content_missing_diskformat(self):
self._do_test_post_image_content_missing_format('container_format')
def test_post_image_content_bad_container_format(self):
self._do_test_post_image_content_bad_format('container_format')
@skip_if_disabled
def _do_test_post_image_content_missing_disk_format(self):
self._do_test_post_image_content_missing_format('disk_format')
def test_post_image_content_bad_disk_format(self):
self._do_test_post_image_content_bad_format('disk_format')
def _do_test_put_image_content_missing_format(self, format):
"""
@ -1288,6 +1308,7 @@ class TestApi(functional.FunctionalTest):
self.assertEqual(response.status, 201)
data = json.loads(content)
image_id = data['image']['id']
print data
# PUT image content images without given format being specified
path = ("http://%s:%d/v1/images/%s" %
@ -1303,18 +1324,18 @@ class TestApi(functional.FunctionalTest):
body=test_data_file.name)
self.assertEqual(response.status, 400)
type = format.replace('_format', '')
expected = "Details: Invalid %s format 'None' for image" % type
expected = "Invalid %s format 'None' for image" % type
self.assertTrue(expected in content,
"Could not find '%s' in '%s'" % (expected, content))
self.stop_servers()
@skip_if_disabled
def _do_test_put_image_content_missing_diskformat(self):
def test_put_image_content_bad_container_format(self):
self._do_test_put_image_content_missing_format('container_format')
@skip_if_disabled
def _do_test_put_image_content_missing_disk_format(self):
def test_put_image_content_bad_disk_format(self):
self._do_test_put_image_content_missing_format('disk_format')
@skip_if_disabled

51
glance/tests/unit/test_clients.py
View File

@ -1058,57 +1058,6 @@ class TestRegistryClient(base.IsolatedUnitTest):
self.client.add_image,
fixture)
def test_add_image_with_acceptably_long_name(self):
"""Tests adding image with acceptably long name"""
name = 'x' * 255
fixture = {'name': name,
'is_public': True,
'disk_format': 'vmdk',
'container_format': 'ovf',
'size': 19,
'location': "file:///tmp/glance-tests/2",
}
new_image = self.client.add_image(fixture)
data = self.client.get_image(new_image['id'])
self.assertEquals(name, data['name'])
def test_add_image_with_excessively_long_name(self):
"""Tests adding image with excessively long name"""
name = 'x' * 256
fixture = {'name': name,
'is_public': True,
'disk_format': 'vmdk',
'container_format': 'ovf',
'size': 19,
'location': "file:///tmp/glance-tests/2",
}
self.assertRaises(exception.Invalid,
self.client.add_image,
fixture)
def test_update_image_with_acceptably_long_name(self):
"""Tests updating image with acceptably long name"""
name = 'x' * 255
fixture = {'name': name}
self.assertTrue(self.client.update_image(UUID2, fixture))
data = self.client.get_image(UUID2)
self.assertEquals(name, data['name'])
def test_update_image_with_excessively_long_name(self):
"""Tests updating image with excessively long name"""
name = 'x' * 256
fixture = {'name': name}
self.assertRaises(exception.Invalid,
self.client.update_image,
UUID2,
fixture)
def test_update_image(self):
"""Tests that the /images PUT registry API updates the image"""
fixture = {'name': 'fake public image #2',

133
glance/tests/unit/v1/test_api.py
View File

@ -1645,62 +1645,6 @@ class TestRegistryAPI(base.IsolatedUnitTest):
self.assertEquals(0, res_dict['image']['min_disk'])
def test_create_image_with_bad_container_format(self):
"""Tests proper exception is raised if a bad disk_format is set"""
fixture = {'id': _gen_uuid(),
'name': 'fake public image',
'is_public': True,
'disk_format': 'vhd',
'container_format': 'invalid'}
req = webob.Request.blank('/images')
req.method = 'POST'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid container format' in res.body)
def test_create_image_with_bad_disk_format(self):
"""Tests proper exception is raised if a bad disk_format is set"""
fixture = {'id': _gen_uuid(),
'name': 'fake public image',
'is_public': True,
'disk_format': 'invalid',
'container_format': 'ovf'}
req = webob.Request.blank('/images')
req.method = 'POST'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid disk format' in res.body)
def test_create_image_with_mismatched_formats(self):
"""
Tests that exception raised for bad matching disk and
container formats
"""
fixture = {'name': 'fake public image #3',
'container_format': 'aki',
'disk_format': 'ari'}
req = webob.Request.blank('/images')
req.method = 'POST'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid mix of disk and container formats'
in res.body)
def test_create_image_with_bad_status(self):
"""Tests proper exception is raised if a bad status is set"""
fixture = {'id': _gen_uuid(),
@ -1793,53 +1737,6 @@ class TestRegistryAPI(base.IsolatedUnitTest):
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid image status' in res.body)
def test_update_image_with_bad_disk_format(self):
"""Tests that exception raised trying to set a bad disk_format"""
fixture = {'disk_format': 'invalid'}
req = webob.Request.blank('/images/%s' % UUID2)
req.method = 'PUT'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid disk format' in res.body)
def test_update_image_with_bad_container_format(self):
"""Tests that exception raised trying to set a bad container_format"""
fixture = {'container_format': 'invalid'}
req = webob.Request.blank('/images/%s' % UUID2)
req.method = 'PUT'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid container format' in res.body)
def test_update_image_with_mismatched_formats(self):
"""
Tests that exception raised for bad matching disk and
container formats
"""
fixture = {'container_format': 'ari'}
# Image 2 has disk format 'vhd'
req = webob.Request.blank('/images/%s' % UUID2)
req.method = 'PUT'
req.content_type = 'application/json'
req.body = json.dumps(dict(image=fixture))
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid mix of disk and container formats'
in res.body)
def test_delete_image(self):
"""Tests that the /images DELETE registry API deletes the image"""
@ -2072,6 +1969,21 @@ class TestGlanceAPI(base.IsolatedUnitTest):
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid disk format' in res.body, res.body)
def test_create_with_location_no_container_format(self):
fixture_headers = {'x-image-meta-store': 'bad',
'x-image-meta-name': 'bogus',
'x-image-meta-location': 'http://localhost:0/image.tar.gz',
'x-image-meta-disk-format': 'vhd'}
req = webob.Request.blank("/images")
req.method = 'POST'
for k, v in fixture_headers.iteritems():
req.headers[k] = v
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Invalid container format' in res.body)
def test_bad_container_format(self):
fixture_headers = {'x-image-meta-store': 'bad',
'x-image-meta-name': 'bogus',
@ -2105,6 +2017,21 @@ class TestGlanceAPI(base.IsolatedUnitTest):
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
self.assertTrue('Incoming image size' in res.body)
def test_bad_image_name(self):
fixture_headers = {'x-image-meta-store': 'bad',
'x-image-meta-name': 'X' * 256,
'x-image-meta-location': 'http://example.com/image.tar.gz',
'x-image-meta-disk-format': 'vhd',
'x-image-meta-container-format': 'bare'}
req = webob.Request.blank("/images")
req.method = 'POST'
for k, v in fixture_headers.iteritems():
req.headers[k] = v
res = req.get_response(self.api)
self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
def test_add_image_no_location_no_image_as_body(self):
"""Tests creates a queued image for no body and no loc header"""
fixture_headers = {'x-image-meta-store': 'file',