From 503be24afa122eef08186001e54c1e1180114ccf Mon Sep 17 00:00:00 2001 From: Jon Bernard Date: Fri, 13 Dec 2013 11:12:34 -0500 Subject: [PATCH] Prevent creation of http images with invalid URIs Active images should always be ready to be downloaded, regardless they're locally or remotely stored. This patch prevents images with invalid location URIs from being created and entering the 'active' state with no data. This is only for the HTTP store. Closes-Bug: #1257273 Co-authored: Jon Bernard Change-Id: Iffce79b654cabe8397c85b2cc50c4b7f59733ea5 --- glance/api/v1/images.py | 8 ++- glance/store/http.py | 14 +++- glance/tests/functional/v1/test_ssl.py | 37 ----------- .../legacy_functional/test_v1_api.py | 33 ---------- glance/tests/unit/test_http_store.py | 2 +- glance/tests/unit/v1/test_api.py | 66 ++++++++++++------- 6 files changed, 61 insertions(+), 99 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 877e79ff95..f80a9a62ea 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -679,8 +679,12 @@ class Controller(controller.BaseController): def _get_size(self, context, image_meta, location): # retrieve the image size from remote store (if not provided) - return image_meta.get('size', 0) or get_size_from_backend(context, - location) + try: + return (image_meta.get('size', 0) or + get_size_from_backend(context, location)) + except (exception.NotFound, exception.BadStoreUri) as e: + LOG.debug(e) + raise HTTPBadRequest(explanation=e.msg, content_type="text/plain") def _handle_source(self, req, image_id, image_meta, image_data): copy_from = self._copy_from(req) diff --git a/glance/store/http.py b/glance/store/http.py index dc748bebb9..716fa18251 100644 --- a/glance/store/http.py +++ b/glance/store/http.py @@ -14,6 +14,7 @@ # under the License. import httplib +import socket import six.moves.urllib.parse as urlparse @@ -145,9 +146,16 @@ class Store(glance.store.base.Store): from glance.store.location.get_location_from_uri() """ try: - return self._query(location, 'HEAD')[2] + size = self._query(location, 'HEAD')[2] + except socket.error: + reason = _("The HTTP URL is invalid.") + LOG.debug(reason) + raise exception.BadStoreUri(reason) except Exception: + # NOTE(flaper87): Catch more granular exceptions, + # keeping this branch for backwards compatibility. return 0 + return size def _query(self, location, verb, depth=0): if depth > MAX_REDIRECTS: @@ -163,6 +171,10 @@ class Store(glance.store.base.Store): # Check for bad status codes if resp.status >= 400: + if resp.status == httplib.NOT_FOUND: + reason = _("HTTP datastore could not find image at URI.") + LOG.debug(reason) + raise exception.NotFound(reason) reason = _("HTTP URL returned a %s status code.") % resp.status LOG.debug(reason) raise exception.BadStoreUri(loc.path, reason) diff --git a/glance/tests/functional/v1/test_ssl.py b/glance/tests/functional/v1/test_ssl.py index a2fb94fff8..ad66b8eec0 100644 --- a/glance/tests/functional/v1/test_ssl.py +++ b/glance/tests/functional/v1/test_ssl.py @@ -643,43 +643,6 @@ class TestSSL(functional.FunctionalTest): self.stop_servers() - @skip_if_disabled - def test_size_greater_2G_mysql(self): - """ - A test against the actual datastore backend for the registry - to ensure that the image size property is not truncated. - - :see https://bugs.launchpad.net/glance/+bug/739433 - """ - - self.cleanup() - self.start_servers(**self.__dict__.copy()) - - # 1. POST /images with public image named Image1 - # attribute and a size of 5G. Use the HTTP engine with an - # X-Image-Meta-Location attribute to make Glance forego - # "adding" the image data. - # Verify a 201 OK is returned - headers = minimal_headers('Image1') - headers['X-Image-Meta-Location'] = 'https://example.com/fakeimage' - headers['X-Image-Meta-Size'] = str(FIVE_GB) - path = "https://%s:%d/v1/images" % ("127.0.0.1", self.api_port) - https = httplib2.Http(disable_ssl_certificate_validation=True) - response, content = https.request(path, 'POST', headers=headers) - self.assertEqual(response.status, 201) - - # 2. HEAD /images - # Verify image size is what was passed in, and not truncated - path = response.get('location') - https = httplib2.Http(disable_ssl_certificate_validation=True) - response, content = https.request(path, 'HEAD') - self.assertEqual(response.status, 200) - self.assertEqual(response['x-image-meta-size'], str(FIVE_GB)) - self.assertEqual(response['x-image-meta-name'], 'Image1') - self.assertEqual(response['x-image-meta-is_public'], 'True') - - self.stop_servers() - @skip_if_disabled def test_traceback_not_consumed(self): """ diff --git a/glance/tests/integration/legacy_functional/test_v1_api.py b/glance/tests/integration/legacy_functional/test_v1_api.py index 7fa0db17e9..ae17693280 100644 --- a/glance/tests/integration/legacy_functional/test_v1_api.py +++ b/glance/tests/integration/legacy_functional/test_v1_api.py @@ -328,39 +328,6 @@ class TestApi(base.ApiTest): response, content = self.http.request(path, 'DELETE') self.assertEqual(response.status, 200) - def test_size_greater_2G_mysql(self): - """ - A test against the actual datastore backend for the registry - to ensure that the image size property is not truncated. - - :see https://bugs.launchpad.net/glance/+bug/739433 - """ - - # 1. POST /images with public image named Image1 - # attribute and a size of 5G. Use the HTTP engine with an - # X-Image-Meta-Location attribute to make Glance forego - # "adding" the image data. - # Verify a 201 OK is returned - headers = {'Content-Type': 'application/octet-stream', - 'X-Image-Meta-Location': 'http://example.com/fakeimage', - 'X-Image-Meta-Size': str(FIVE_GB), - 'X-Image-Meta-Name': 'Image1', - 'X-Image-Meta-disk_format': 'raw', - 'X-image-Meta-container_format': 'ovf', - 'X-Image-Meta-Is-Public': 'True'} - path = "/v1/images" - response, content = self.http.request(path, 'POST', headers=headers) - self.assertEqual(response.status, 201) - - # 2. HEAD /images - # Verify image size is what was passed in, and not truncated - path = response.get('location') - response, content = self.http.request(path, 'HEAD') - self.assertEqual(response.status, 200) - self.assertEqual(response['x-image-meta-size'], str(FIVE_GB)) - self.assertEqual(response['x-image-meta-name'], 'Image1') - self.assertEqual(response['x-image-meta-is_public'], 'True') - def test_v1_not_enabled(self): self.config(enable_v1_api=False) path = "/v1/images" diff --git a/glance/tests/unit/test_http_store.py b/glance/tests/unit/test_http_store.py index 49ca719791..033532b62b 100644 --- a/glance/tests/unit/test_http_store.py +++ b/glance/tests/unit/test_http_store.py @@ -159,7 +159,7 @@ class TestHttpStore(base.StoreClearingUnitTest): uri = "http://netloc/path/to/file.tar.gz" loc = get_location_from_uri(uri) - self.assertRaises(exception.BadStoreUri, self.store.get, loc) + self.assertRaises(exception.NotFound, self.store.get, loc) def test_https_get(self): uri = "https://netloc/path/to/file.tar.gz" diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py index 56dac647af..5915219bf1 100644 --- a/glance/tests/unit/v1/test_api.py +++ b/glance/tests/unit/v1/test_api.py @@ -40,6 +40,7 @@ from glance.openstack.common import timeutils import glance.registry.client.v1.api as registry import glance.store.filesystem +from glance.store import http from glance.tests.unit import base import glance.tests.unit.utils as unit_test_utils from glance.tests import utils as test_utils @@ -122,11 +123,13 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in fixture_headers.iteritems(): req.headers[k] = v - res = req.get_response(self.api) - self.assertEqual(res.status_int, 201) - res_body = jsonutils.loads(res.body)['image'] - self.assertEqual(format_value, res_body['disk_format']) - self.assertEqual(format_value, res_body['container_format']) + with mock.patch.object(http.Store, 'get_size') as mocked_size: + mocked_size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 201) + res_body = jsonutils.loads(res.body)['image'] + self.assertEqual(format_value, res_body['disk_format']) + self.assertEqual(format_value, res_body['container_format']) def test_defaulted_amazon_format(self): for key in ('x-image-meta-disk-format', @@ -240,8 +243,10 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in fixture_headers.iteritems(): req.headers[k] = v - res = req.get_response(self.api) - self.assertEqual(res.status_int, 201) + with mock.patch.object(http.Store, 'get_size') as mocked_size: + mocked_size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 201) def test_configured_disk_format_bad(self): self.config(disk_formats=['foo'], group="image_format") @@ -276,8 +281,10 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in fixture_headers.iteritems(): req.headers[k] = v - res = req.get_response(self.api) - self.assertEqual(res.status_int, 201) + with mock.patch.object(http.Store, 'get_size') as mocked_size: + mocked_size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 201) def test_configured_container_format_bad(self): self.config(container_formats=['foo'], group="image_format") @@ -331,9 +338,11 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in fixture_headers.iteritems(): req.headers[k] = v - res = req.get_response(self.api) - self.assertEqual(res.status_int, 400) - self.assertIn('Invalid container format', res.body) + with mock.patch.object(http.Store, 'get_size') as mocked_size: + mocked_size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 400) + self.assertIn('Invalid container format', res.body) def test_create_with_bad_store_name(self): fixture_headers = { @@ -482,8 +491,11 @@ class TestGlanceAPI(base.IsolatedUnitTest): req = webob.Request.blank("/images/%s" % image_id) req.method = 'PUT' req.headers['x-image-meta-location'] = 'http://localhost:0/images/123' - res = req.get_response(self.api) - self.assertEqual(res.status_int, 200) + + with mock.patch.object(http.Store, 'get_size') as mocked_size: + mocked_size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 200) res_body = jsonutils.loads(res.body)['image'] # Once the location is set, the image should be activated @@ -928,9 +940,6 @@ class TestGlanceAPI(base.IsolatedUnitTest): def test_add_location_with_conflict_image_size(self): """Tests creates an image from location and conflict image size""" - self.stubs.Set(glance.api.v1.images, 'get_size_from_backend', - lambda *args, **kwargs: 2) - fixture_headers = {'x-image-meta-store': 'file', 'x-image-meta-disk-format': 'vhd', 'x-image-meta-location': 'http://a/b/c.tar.gz', @@ -941,10 +950,14 @@ class TestGlanceAPI(base.IsolatedUnitTest): req = webob.Request.blank("/images") req.headers['Content-Type'] = 'application/octet-stream' req.method = 'POST' - for k, v in fixture_headers.iteritems(): - req.headers[k] = v - res = req.get_response(self.api) - self.assertEqual(res.status_int, 409) + with mock.patch.object(http.Store, 'get_size') as size: + size.return_value = 2 + + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEqual(res.status_int, 409) def test_add_copy_from_with_location(self): """Tests creates an image from copy-from and location""" @@ -2109,10 +2122,13 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in headers.iteritems(): req.headers[k] = v req.method = 'POST' - res = req.get_response(self.api) - self.assertEqual(res.status_int, 201) - res_body = jsonutils.loads(res.body) - self.assertNotIn('location', res_body['image']) + + with mock.patch.object(http.Store, 'get_size') as size: + size.return_value = 0 + res = req.get_response(self.api) + self.assertEqual(res.status_int, 201) + res_body = jsonutils.loads(res.body) + self.assertNotIn('location', res_body['image']) def test_image_is_checksummed(self): """Test that the image contents are checksummed properly"""