openstack/glance
Code Issues Proposed changes

Adds proper response checking to HTTP Store.

Browse Source 
The HTTP Store now handles the following cases:

  * Redirects are resolved (infinite redirect chains are prevented).
  * 4xx and 5xx status codes result in a proper exception instead
    of trying to continue with the image.

Fixes bug 1009248.

Change-Id: Ibc44413d630bf35e0c396adc430c39f4295030fd
This commit is contained in:
Gabriel Hurley
2012-08-01 17:40:30 -07:00
parent bab5aa1d6a
commit 616d2f9a65
2 changed files with 102 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
glance/store/http.py
View File

@@ -26,6 +26,9 @@ import glance.store.location
LOG = logging.getLogger(__name__)
MAX_REDIRECTS = 5
class StoreLocation(glance.store.location.StoreLocation):
"""Class describing an HTTP(S) URI"""
@@ -147,12 +150,33 @@ class Store(glance.store.base.Store):
except Exception:
return 0
def _query(self, location, verb):
def _query(self, location, verb, depth=0):
if depth > MAX_REDIRECTS:
raise exception.MaxRedirectsExceeded(redirects=MAX_REDIRECTS)
loc = location.store_location
conn_class = self._get_conn_class(loc)
conn = conn_class(loc.netloc)
conn.request(verb, loc.path, "", {})
resp = conn.getresponse()
# Check for bad status codes
if resp.status >= 400:
reason = _("HTTP URL returned a %s status code.") % resp.status
raise exception.BadStoreUri(loc.path, reason)
location_header = resp.getheader("location")
if location_header:
if resp.status not in (301, 302):
reason = _("The HTTP URL attempted to redirect with an "
"invalid status code.")
raise exception.BadStoreUri(loc.path, reason)
location_class = glance.store.location.Location
new_loc = location_class(location.store_name,
location.store_location.__class__,
uri=location_header,
image_id=location.image_id,
store_specs=location.store_specs)
return self._query(new_loc, verb, depth + 1)
content_length = resp.getheader('content-length', 0)
return (conn, resp, content_length)

90
glance/tests/unit/test_http_store.py
View File

@@ -26,12 +26,30 @@ from glance.db.sqlalchemy import api as db_api
from glance.registry import configure_registry_client
from glance.store import (delete_from_backend,
schedule_delete_from_backend)
from glance.store.http import Store
from glance.store.http import Store, MAX_REDIRECTS
from glance.store.location import get_location_from_uri
from glance.tests.unit import base
from glance.tests import utils, stubs as test_stubs
# The response stack is used to return designated responses in order;
# however when it's empty a default 200 OK response is returned from
# FakeHTTPConnection below.
FAKE_RESPONSE_STACK = []
class FakeHTTPResponse(object):
def __init__(self, status=200, headers=None, data=None, *args, **kwargs):
data = data or 'I am a teapot, short and stout\n'
self.data = StringIO.StringIO(data)
self.read = self.data.read
self.status = status
self.headers = headers or {'content-length': len(data)}
def getheader(self, name, default=None):
return self.headers.get(name.lower(), default)
def stub_out_http_backend(stubs):
"""
Stubs out the httplib.HTTPRequest.getresponse to return
@@ -43,24 +61,14 @@ def stub_out_http_backend(stubs):
:param stubs: Set of stubout stubs
"""
class FakeHTTPResponse(object):
DATA = 'I am a teapot, short and stout\n'
HEADERS = {'content-length': 31}
def __init__(self, *args, **kwargs):
self.data = StringIO.StringIO(self.DATA)
self.read = self.data.read
def getheader(self, name, default=None):
return self.HEADERS.get(name.lower(), default)
class FakeHTTPConnection(object):
def __init__(self, *args, **kwargs):
pass
def getresponse(self):
if len(FAKE_RESPONSE_STACK):
return FAKE_RESPONSE_STACK.pop()
return FakeHTTPResponse()
def request(self, *_args, **_kwargs):
@@ -92,6 +100,8 @@ def stub_out_registry_image_update(stubs):
class TestHttpStore(base.StoreClearingUnitTest):
def setUp(self):
global FAKE_RESPONSE_STACK
FAKE_RESPONSE_STACK = []
self.config(default_store='http',
known_stores=['glance.store.http.Store'])
super(TestHttpStore, self).setUp()
@@ -111,6 +121,60 @@ class TestHttpStore(base.StoreClearingUnitTest):
chunks = [c for c in image_file]
self.assertEqual(chunks, expected_returns)
def test_http_get_redirect(self):
# Add two layers of redirects to the response stack, which will
# return the default 200 OK with the expected data after resolving
# both redirects.
redirect_headers_1 = {"location": "http://example.com/teapot.img"}
redirect_resp_1 = FakeHTTPResponse(status=302,
headers=redirect_headers_1)
redirect_headers_2 = {"location": "http://example.com/teapot_real.img"}
redirect_resp_2 = FakeHTTPResponse(status=301,
headers=redirect_headers_2)
FAKE_RESPONSE_STACK.append(redirect_resp_1)
FAKE_RESPONSE_STACK.append(redirect_resp_2)
uri = "http://netloc/path/to/file.tar.gz"
expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s',
'ho', 'rt', ' a', 'nd', ' s', 'to', 'ut', '\n']
loc = get_location_from_uri(uri)
(image_file, image_size) = self.store.get(loc)
self.assertEqual(image_size, 31)
chunks = [c for c in image_file]
self.assertEqual(chunks, expected_returns)
def test_http_get_max_redirects(self):
# Add more than MAX_REDIRECTS redirects to the response stack
redirect_headers = {"location": "http://example.com/teapot.img"}
redirect_resp = FakeHTTPResponse(status=302,
headers=redirect_headers)
for i in xrange(MAX_REDIRECTS + 2):
FAKE_RESPONSE_STACK.append(redirect_resp)
uri = "http://netloc/path/to/file.tar.gz"
loc = get_location_from_uri(uri)
self.assertRaises(exception.MaxRedirectsExceeded, self.store.get, loc)
def test_http_get_redirect_invalid(self):
redirect_headers = {"location": "http://example.com/teapot.img"}
redirect_resp = FakeHTTPResponse(status=307,
headers=redirect_headers)
FAKE_RESPONSE_STACK.append(redirect_resp)
uri = "http://netloc/path/to/file.tar.gz"
loc = get_location_from_uri(uri)
self.assertRaises(exception.BadStoreUri, self.store.get, loc)
def test_http_get_not_found(self):
not_found_resp = FakeHTTPResponse(status=404,
data="404 Not Found")
FAKE_RESPONSE_STACK.append(not_found_resp)
uri = "http://netloc/path/to/file.tar.gz"
loc = get_location_from_uri(uri)
self.assertRaises(exception.BadStoreUri, self.store.get, loc)
def test_https_get(self):
uri = "https://netloc/path/to/file.tar.gz"
expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s',