openstack
/
swift
Code Issues Proposed changes

Merge "Stop mutating PATH_INFO in proxy server"

This commit is contained in:
Jenkins 2013-12-06 22:31:21 +00:00 committed by Gerrit Code Review
parent 46e7ba0e70 3530708619
commit 55b8ac4d53
11 changed files with 299 additions and 228 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
swift/common/swob.py
View File

@ -857,6 +857,18 @@ class Request(object):
return urllib2.quote(self.environ.get('SCRIPT_NAME', '') +
self.environ['PATH_INFO'])
@property
def swift_entity_path(self):
"""
Provides the account/container/object path, sans API version.
This can be useful when constructing a path to send to a backend
server, as that path will need everything after the "/v1".
"""
_ver, entity_path = self.split_path(1, 2, rest_with_last=True)
if entity_path is not None:
return '/' + entity_path
@property
def url(self):
"Provides the full url of the request"

10
swift/proxy/controllers/account.py
View File

@ -47,7 +47,7 @@ class AccountController(Controller):
partition, nodes = self.app.account_ring.get_nodes(self.account_name)
resp = self.GETorHEAD_base(
req, _('Account'), self.app.account_ring, partition,
req.path_info.rstrip('/'))
req.swift_entity_path.rstrip('/'))
if resp.status_int == HTTP_NOT_FOUND:
if resp.headers.get('X-Account-Status', '').lower() == 'deleted':
resp.status = HTTP_GONE
@ -81,7 +81,7 @@ class AccountController(Controller):
clear_info_cache(self.app, req.environ, self.account_name)
resp = self.make_requests(
req, self.app.account_ring, account_partition, 'PUT',
req.path_info, [headers] * len(accounts))
req.swift_entity_path, [headers] * len(accounts))
return resp
@public
@ -101,12 +101,12 @@ class AccountController(Controller):
clear_info_cache(self.app, req.environ, self.account_name)
resp = self.make_requests(
req, self.app.account_ring, account_partition, 'POST',
req.path_info, [headers] * len(accounts))
req.swift_entity_path, [headers] * len(accounts))
if resp.status_int == HTTP_NOT_FOUND and self.app.account_autocreate:
self.autocreate_account(req.environ, self.account_name)
resp = self.make_requests(
req, self.app.account_ring, account_partition, 'POST',
req.path_info, [headers] * len(accounts))
req.swift_entity_path, [headers] * len(accounts))
return resp
@public
@ -127,5 +127,5 @@ class AccountController(Controller):
clear_info_cache(self.app, req.environ, self.account_name)
resp = self.make_requests(
req, self.app.account_ring, account_partition, 'DELETE',
req.path_info, [headers] * len(accounts))
req.swift_entity_path, [headers] * len(accounts))
return resp

6
swift/proxy/controllers/base.py
View File

@ -947,6 +947,7 @@ class Controller(object):
:param part: the partition number
:param method: the method to send to the backend
:param path: the path to send to the backend
(full path ends up being /<$device>/<$part>/<$path>)
:param headers: a list of dicts, where each dict represents one
backend request that should be made.
:param query: query string to send to the backend.
@ -992,6 +993,7 @@ class Controller(object):
:param part: the partition number
:param method: the method to send to the backend
:param path: the path to send to the backend
(full path ends up being /<$device>/<$part>/<$path>)
:param headers: a list of dicts, where each dict represents one
backend request that should be made.
:param query_string: optional query string to send to the backend
@ -1138,12 +1140,12 @@ class Controller(object):
'%s %s' % (server_type, req.method),
headers=handler.source_headers)
try:
(account, container) = split_path(req.path_info, 1, 2)
(vrs, account, container) = req.split_path(2, 3)
_set_info_cache(self.app, req.environ, account, container, res)
except ValueError:
pass
try:
(account, container, obj) = split_path(req.path_info, 3, 3, True)
(vrs, account, container, obj) = req.split_path(4, 4, True)
_set_object_info_cache(self.app, req.environ, account,
container, obj, res)
except ValueError:

9
swift/proxy/controllers/container.py
View File

@ -64,7 +64,8 @@ class ContainerController(Controller):
part = self.app.container_ring.get_part(
self.account_name, self.container_name)
resp = self.GETorHEAD_base(
req, _('Container'), self.app.container_ring, part, req.path_info)
req, _('Container'), self.app.container_ring, part,
req.swift_entity_path)
if 'swift.authorize' in req.environ:
req.acl = resp.headers.get('x-container-read')
aresp = req.environ['swift.authorize'](req)
@ -126,7 +127,7 @@ class ContainerController(Controller):
self.account_name, self.container_name)
resp = self.make_requests(
req, self.app.container_ring,
container_partition, 'PUT', req.path_info, headers)
container_partition, 'PUT', req.swift_entity_path, headers)
return resp
@public
@ -148,7 +149,7 @@ class ContainerController(Controller):
self.account_name, self.container_name)
resp = self.make_requests(
req, self.app.container_ring, container_partition, 'POST',
req.path_info, [headers] * len(containers))
req.swift_entity_path, [headers] * len(containers))
return resp
@public
@ -167,7 +168,7 @@ class ContainerController(Controller):
self.account_name, self.container_name)
resp = self.make_requests(
req, self.app.container_ring, container_partition, 'DELETE',
req.path_info, headers)
req.swift_entity_path, headers)
# Indicates no server had the container
if resp.status_int == HTTP_ACCEPTED:
return HTTPNotFound(request=req)

40
swift/proxy/controllers/obj.py
View File

@ -158,8 +158,9 @@ class SegmentedIterable(object):
container, obj = self.container, self.segment_dict['name']
partition = self.controller.app.object_ring.get_part(
self.controller.account_name, container, obj)
path = '/%s/%s/%s' % (self.controller.account_name, container, obj)
req = Request.blank(path)
path = '/%s/%s/%s' % (self.controller.account_name,
container, obj)
req = Request.blank('/v1' + path)
if self.seek or (self.length and self.length > 0):
bytes_available = \
self.segment_dict['bytes'] - self.seek
@ -375,14 +376,14 @@ class ObjectController(Controller):
lreq = Request.blank('i will be overridden by env', environ=env)
# Don't quote PATH_INFO, by WSGI spec
lreq.environ['PATH_INFO'] = \
'/%s/%s' % (self.account_name, lcontainer)
'/v1/%s/%s' % (self.account_name, lcontainer)
lreq.environ['REQUEST_METHOD'] = 'GET'
lreq.environ['QUERY_STRING'] = \
'format=json&prefix=%s&marker=%s' % (quote(lprefix),
quote(marker))
lresp = self.GETorHEAD_base(
lreq, _('Container'), self.app.container_ring, lpartition,
lreq.path_info)
lreq.swift_entity_path)
if 'swift.authorize' in env:
lreq.acl = lresp.headers.get('x-container-read')
aresp = env['swift.authorize'](lreq)
@ -417,7 +418,7 @@ class ObjectController(Controller):
new_req = incoming_req.copy_get()
new_req.method = 'GET'
new_req.range = None
new_req.path_info = '/'.join(['', account, container, obj])
new_req.path_info = '/'.join(['/v1', account, container, obj])
if partition is None:
try:
partition = self.app.object_ring.get_part(
@ -428,7 +429,7 @@ class ObjectController(Controller):
new_req.path)
valid_resp = self.GETorHEAD_base(
new_req, _('Object'), self.app.object_ring, partition,
new_req.path_info)
new_req.swift_entity_path)
if 'swift.authorize' in incoming_req.environ:
incoming_req.acl = valid_resp.headers.get('x-container-read')
@ -535,7 +536,8 @@ class ObjectController(Controller):
partition = self.app.object_ring.get_part(
self.account_name, self.container_name, self.object_name)
resp = self.GETorHEAD_base(
req, _('Object'), self.app.object_ring, partition, req.path_info)
req, _('Object'), self.app.object_ring, partition,
req.swift_entity_path)
if ';' in resp.headers.get('content-type', ''):
# strip off swift_bytes from content-type
@ -677,7 +679,7 @@ class ObjectController(Controller):
req.headers['x-delete-at'] = '%d' % (time.time() + x_delete_after)
if self.app.object_post_as_copy:
req.method = 'PUT'
req.path_info = '/%s/%s/%s' % (
req.path_info = '/v1/%s/%s/%s' % (
self.account_name, self.container_name, self.object_name)
req.headers['Content-Length'] = 0
req.headers['X-Copy-From'] = quote('/%s/%s' % (self.container_name,
@ -741,7 +743,7 @@ class ObjectController(Controller):
delete_at_container, delete_at_part, delete_at_nodes)
resp = self.make_requests(req, self.app.object_ring, partition,
'POST', req.path_info, headers)
'POST', req.swift_entity_path, headers)
return resp
def _backend_requests(self, req, n_outgoing,
@ -913,7 +915,7 @@ class ObjectController(Controller):
environ={'REQUEST_METHOD': 'HEAD'})
hresp = self.GETorHEAD_base(
hreq, _('Object'), self.app.object_ring, partition,
hreq.path_info)
hreq.swift_entity_path)
# Used by container sync feature
if 'x-timestamp' in req.headers:
try:
@ -989,15 +991,15 @@ class ObjectController(Controller):
req.environ.setdefault('swift.log_info', []).append(
'x-copy-from:%s' % source_header)
source_header = unquote(source_header)
acct = req.path_info.split('/', 2)[1]
acct = req.swift_entity_path.split('/', 2)[1]
if isinstance(acct, unicode):
acct = acct.encode('utf-8')
if not source_header.startswith('/'):
source_header = '/' + source_header
source_header = '/' + acct + source_header
source_header = '/v1/' + acct + source_header
try:
src_container_name, src_obj_name = \
source_header.split('/', 3)[2:]
source_header.split('/', 4)[3:]
except ValueError:
return HTTPPreconditionFailed(
request=req,
@ -1082,7 +1084,8 @@ class ObjectController(Controller):
if (req.content_length > 0) or chunked:
nheaders['Expect'] = '100-continue'
pile.spawn(self._connect_put_node, node_iter, partition,
req.path_info, nheaders, self.app.logger.thread_locals)
req.swift_entity_path, nheaders,
self.app.logger.thread_locals)
conns = [conn for conn in pile if conn]
min_conns = quorum_size(len(nodes))
@ -1156,7 +1159,7 @@ class ObjectController(Controller):
_('Object PUT'), etag=etag)
if source_header:
resp.headers['X-Copied-From'] = quote(
source_header.split('/', 2)[2])
source_header.split('/', 3)[3])
if 'last-modified' in source_resp.headers:
resp.headers['X-Copied-From-Last-Modified'] = \
source_resp.headers['last-modified']
@ -1201,7 +1204,7 @@ class ObjectController(Controller):
orig_obj = self.object_name
self.container_name = lcontainer
self.object_name = last_item['name'].encode('utf-8')
copy_path = '/' + self.account_name + '/' + \
copy_path = '/v1/' + self.account_name + '/' + \
self.container_name + '/' + self.object_name
copy_headers = {'X-Newest': 'True',
'Destination': orig_container + '/' + orig_obj
@ -1256,7 +1259,8 @@ class ObjectController(Controller):
headers = self._backend_requests(
req, len(nodes), container_partition, containers)
resp = self.make_requests(req, self.app.object_ring,
partition, 'DELETE', req.path_info, headers)
partition, 'DELETE', req.swift_entity_path,
headers)
return resp
@public
@ -1284,7 +1288,7 @@ class ObjectController(Controller):
# re-write the existing request as a PUT instead of creating a new one
# since this one is already attached to the posthooklogger
req.method = 'PUT'
req.path_info = '/' + self.account_name + dest
req.path_info = '/v1/' + self.account_name + dest
req.headers['Content-Length'] = 0
req.headers['X-Copy-From'] = quote(source)
del req.headers['Destination']

2
swift/proxy/server.py
View File

@ -292,8 +292,6 @@ class Application(object):
allowed_methods = getattr(controller, 'allowed_methods', set())
return HTTPMethodNotAllowed(
request=req, headers={'Allow': ', '.join(allowed_methods)})
if path_parts['version']:
req.path_info_pop()
if 'swift.authorize' in req.environ:
# We call authorize before the handler, always. If authorized,
# we remove the swift.authorize hook so isn't ever called

13
test/unit/common/test_swob.py
View File

@ -669,6 +669,19 @@ class TestRequest(unittest.TestCase):
req.accept.best_match(['text/plain', 'application/json']),
'application/json')
def test_swift_entity_path(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
self.assertEqual(req.swift_entity_path, '/a/c/o')
req = swift.common.swob.Request.blank('/v1/a/c')
self.assertEqual(req.swift_entity_path, '/a/c')
req = swift.common.swob.Request.blank('/v1/a')
self.assertEqual(req.swift_entity_path, '/a')
req = swift.common.swob.Request.blank('/v1')
self.assertEqual(req.swift_entity_path, None)
def test_path_qs(self):
req = swift.common.swob.Request.blank('/hi/there?hello=equal&acl')
self.assertEqual(req.path_qs, '/hi/there?hello=equal&acl')

10
test/unit/proxy/controllers/test_account.py
View File

@ -34,7 +34,7 @@ class TestAccountController(unittest.TestCase):
controller = proxy_server.AccountController(self.app, 'AUTH_bob')
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, body='')):
req = Request.blank('/AUTH_bob', {'PATH_INFO': '/AUTH_bob'})
req = Request.blank('/v1/AUTH_bob', {'PATH_INFO': '/v1/AUTH_bob'})
resp = controller.HEAD(req)
self.assertEqual(2, resp.status_int // 100)
self.assertTrue('swift.account/AUTH_bob' in resp.environ)
@ -47,7 +47,7 @@ class TestAccountController(unittest.TestCase):
'x-account-meta-temp-url-key-2': 'value'}
controller = proxy_server.AccountController(self.app, 'a')
req = Request.blank('/a')
req = Request.blank('/v1/a')
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, headers=owner_headers)):
resp = controller.HEAD(req)
@ -55,7 +55,7 @@ class TestAccountController(unittest.TestCase):
for key in owner_headers:
self.assertTrue(key not in resp.headers)
req = Request.blank('/a', environ={'swift_owner': True})
req = Request.blank('/v1/a', environ={'swift_owner': True})
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, headers=owner_headers)):
resp = controller.HEAD(req)
@ -69,7 +69,7 @@ class TestAccountController(unittest.TestCase):
}
controller = proxy_server.AccountController(self.app, 'a')
req = Request.blank('/a')
req = Request.blank('/v1/a')
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(404, headers=resp_headers)):
resp = controller.HEAD(req)
@ -79,7 +79,7 @@ class TestAccountController(unittest.TestCase):
long_acct_name = '%sLongAccountName' % ('Very' * (MAX_ANAME_LEN // 4))
controller = proxy_server.AccountController(self.app, long_acct_name)
req = Request.blank('/%s' % long_acct_name)
req = Request.blank('/v1/%s' % long_acct_name)
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200)):
resp = controller.HEAD(req)

8
test/unit/proxy/controllers/test_base.py
View File

@ -89,7 +89,7 @@ class TestFuncs(unittest.TestCase):
def test_GETorHEAD_base(self):
base = Controller(self.app)
req = Request.blank('/a/c/o/with/slashes')
req = Request.blank('/v1/a/c/o/with/slashes')
with patch('swift.proxy.controllers.base.'
'http_connect', fake_http_connect(200)):
resp = base.GETorHEAD_base(req, 'object', FakeRing(), 'part',
@ -97,14 +97,14 @@ class TestFuncs(unittest.TestCase):
self.assertTrue('swift.object/a/c/o/with/slashes' in resp.environ)
self.assertEqual(
resp.environ['swift.object/a/c/o/with/slashes']['status'], 200)
req = Request.blank('/a/c/o')
req = Request.blank('/v1/a/c/o')
with patch('swift.proxy.controllers.base.'
'http_connect', fake_http_connect(200)):
resp = base.GETorHEAD_base(req, 'object', FakeRing(), 'part',
'/a/c/o')
self.assertTrue('swift.object/a/c/o' in resp.environ)
self.assertEqual(resp.environ['swift.object/a/c/o']['status'], 200)
req = Request.blank('/a/c')
req = Request.blank('/v1/a/c')
with patch('swift.proxy.controllers.base.'
'http_connect', fake_http_connect(200)):
resp = base.GETorHEAD_base(req, 'container', FakeRing(), 'part',
@ -112,7 +112,7 @@ class TestFuncs(unittest.TestCase):
self.assertTrue('swift.container/a/c' in resp.environ)
self.assertEqual(resp.environ['swift.container/a/c']['status'], 200)
req = Request.blank('/a')
req = Request.blank('/v1/a')
with patch('swift.proxy.controllers.base.'
'http_connect', fake_http_connect(200)):
resp = base.GETorHEAD_base(req, 'account', FakeRing(), 'part',

6
test/unit/proxy/controllers/test_container.py
View File

@ -33,7 +33,7 @@ class TestContainerController(unittest.TestCase):
controller = proxy_server.ContainerController(self.app, 'a', 'c')
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, 200, body='')):
req = Request.blank('/a/c', {'PATH_INFO': '/a/c'})
req = Request.blank('/v1/a/c', {'PATH_INFO': '/v1/a/c'})
resp = controller.HEAD(req)
self.assertEqual(2, resp.status_int // 100)
self.assertTrue("swift.container/a/c" in resp.environ)
@ -46,7 +46,7 @@ class TestContainerController(unittest.TestCase):
'x-container-sync-key': 'value', 'x-container-sync-to': 'value'}
controller = proxy_server.ContainerController(self.app, 'a', 'c')
req = Request.blank('/a/c')
req = Request.blank('/v1/a/c')
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, 200, headers=owner_headers)):
resp = controller.HEAD(req)
@ -54,7 +54,7 @@ class TestContainerController(unittest.TestCase):
for key in owner_headers:
self.assertTrue(key not in resp.headers)
req = Request.blank('/a/c', environ={'swift_owner': True})
req = Request.blank('/v1/a/c', environ={'swift_owner': True})
with mock.patch('swift.proxy.controllers.base.http_connect',
fake_http_connect(200, 200, headers=owner_headers)):
resp = controller.HEAD(req)

411
test/unit/proxy/test_server.py
View File

File diff suppressed because it is too large Load Diff