openstack/swift
Code Issues Proposed changes

Fix faked-out account GET for JSON and XML.

Browse Source 
If account autocreation is on and the proxy receives a GET request for
a nonexistent account, it'll fake up a response that makes it look as
if the account exists, but without reifying that account into sqlite
DB files.

That faked-out response was just fine as long as you wanted a
text/plain response, but it didn't handle the case of format=json or
format=xml; in those cases, the response would still be
text/plain. This can break clients, and certainly causes crashes in
swift3. Now, those responses match as closely as possible.

The code for generating an account-listing response has been pulled
into (the new) swift.account.utils module, and both the fake response
and the real response use it, thus ensuring that they can't
accidentally diverge. There's also a new probe test for that
non-divergence.

Also, cleaned up a redundant matching of the Accept header in the code
for generating the account listing.

Note that some of the added tests here pass with or without this code
change; they were added because the code I was changing (parts of the
real account GET) wasn't covered by tests.

Bug 1183169

Change-Id: I2a3b8e5d9053e4d0280a320f31efa7c90c94bb06
This commit is contained in:
Samuel Merritt 2013-05-23 18:53:51 -07:00
parent 5501a4031f
commit 15c2ca55f0
7 changed files with 325 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
swift/account/server.py
View File

@ -18,11 +18,12 @@ from __future__ import with_statement
import os
import time
import traceback
from xml.sax import saxutils
from eventlet import Timeout
import swift.common.db
from swift.account.utils import account_listing_response, \
account_listing_content_type
from swift.common.db import AccountBroker
from swift.common.utils import get_logger, get_param, hash_path, public, \
normalize_timestamp, storage_directory, config_true_value, \
@ -33,7 +34,7 @@ from swift.common.db_replicator import ReplicatorRpc
from swift.common.swob import HTTPAccepted, HTTPBadRequest, \
HTTPCreated, HTTPForbidden, HTTPInternalServerError, \
HTTPMethodNotAllowed, HTTPNoContent, HTTPNotFound, \
HTTPPreconditionFailed, HTTPConflict, Request, Response, \
HTTPPreconditionFailed, HTTPConflict, Request, \
HTTPInsufficientStorage, HTTPNotAcceptable
@ -219,17 +220,13 @@ class AccountController(object):
ACCOUNT_LISTING_LIMIT)
marker = get_param(req, 'marker', '')
end_marker = get_param(req, 'end_marker')
query_format = get_param(req, 'format')
except UnicodeDecodeError, err:
return HTTPBadRequest(body='parameters not utf8',
content_type='text/plain', request=req)
if query_format:
req.accept = FORMAT2CONTENT_TYPE.get(query_format.lower(),
FORMAT2CONTENT_TYPE['plain'])
out_content_type = req.accept.best_match(
['text/plain', 'application/json', 'application/xml', 'text/xml'])
if not out_content_type:
return HTTPNotAcceptable(request=req)
out_content_type, error = account_listing_content_type(req)
if error:
return error
if self.mount_check and not check_mount(self.root, drive):
return HTTPInsufficientStorage(drive=drive, request=req)
broker = self._get_account_broker(drive, part, account)
@ -237,53 +234,9 @@ class AccountController(object):
broker.stale_reads_ok = True
if broker.is_deleted():
return HTTPNotFound(request=req)
info = broker.get_info()
resp_headers = {
'X-Account-Container-Count': info['container_count'],
'X-Account-Object-Count': info['object_count'],
'X-Account-Bytes-Used': info['bytes_used'],
'X-Timestamp': info['created_at'],
'X-PUT-Timestamp': info['put_timestamp']}
resp_headers.update((key, value)
for key, (value, timestamp) in
broker.metadata.iteritems() if value != '')
account_list = broker.list_containers_iter(limit, marker, end_marker,
prefix, delimiter)
if out_content_type == 'application/json':
data = []
for (name, object_count, bytes_used, is_subdir) in account_list:
if is_subdir:
data.append({'subdir': name})
else:
data.append({'name': name, 'count': object_count,
'bytes': bytes_used})
account_list = json.dumps(data)
elif out_content_type.endswith('/xml'):
output_list = ['<?xml version="1.0" encoding="UTF-8"?>',
'<account name="%s">' % account]
for (name, object_count, bytes_used, is_subdir) in account_list:
name = saxutils.escape(name)
if is_subdir:
output_list.append('<subdir name="%s" />' % name)
else:
item = '<container><name>%s</name><count>%s</count>' \
'<bytes>%s</bytes></container>' % \
(name, object_count, bytes_used)
output_list.append(item)
output_list.append('</account>')
account_list = '\n'.join(output_list)
else:
if not account_list:
resp_headers['Content-Type'] = req.accept.best_match(
['text/plain', 'application/json',
'application/xml', 'text/xml'])
return HTTPNoContent(request=req, headers=resp_headers,
charset='utf-8')
account_list = '\n'.join(r[0] for r in account_list) + '\n'
ret = Response(body=account_list, request=req, headers=resp_headers)
ret.content_type = out_content_type
ret.charset = 'utf-8'
return ret
return account_listing_response(account, req, out_content_type, broker,
limit, marker, end_marker, prefix,
delimiter)
@public
@timing_stats()

121
swift/account/utils.py Normal file
View File

@ -0,0 +1,121 @@
# Copyright (c) 2010-2013 OpenStack, LLC.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import time
from xml.sax import saxutils
from swift.common.constraints import FORMAT2CONTENT_TYPE
from swift.common.swob import HTTPOk, HTTPNoContent, HTTPNotAcceptable, \
HTTPBadRequest
from swift.common.utils import get_param, json, normalize_timestamp
class FakeAccountBroker(object):
"""
Quacks like an account broker, but doesn't actually do anything. Responds
like an account broker would for a real, empty account with no metadata.
"""
def get_info(self):
now = normalize_timestamp(time.time())
return {'container_count': 0,
'object_count': 0,
'bytes_used': 0,
'created_at': now,
'put_timestamp': now}
def list_containers_iter(self, *_, **__):
return []
@property
def metadata(self):
return {}
def account_listing_content_type(req):
"""
Figure out the content type of an account-listing response.
Returns a 2-tuple: (content_type, error). Only one of them will be set;
the other will be None.
"""
try:
query_format = get_param(req, 'format')
except UnicodeDecodeError:
return (None, HTTPBadRequest(body='parameters not utf8',
content_type='text/plain', request=req))
if query_format:
req.accept = FORMAT2CONTENT_TYPE.get(query_format.lower(),
FORMAT2CONTENT_TYPE['plain'])
content_type = req.accept.best_match(
['text/plain', 'application/json', 'application/xml', 'text/xml'])
if not content_type:
return (None, HTTPNotAcceptable(request=req))
else:
return (content_type, None)
def account_listing_response(account, req, response_content_type, broker=None,
limit='', marker='', end_marker='', prefix='',
delimiter=''):
if broker is None:
broker = FakeAccountBroker()
info = broker.get_info()
resp_headers = {
'X-Account-Container-Count': info['container_count'],
'X-Account-Object-Count': info['object_count'],
'X-Account-Bytes-Used': info['bytes_used'],
'X-Timestamp': info['created_at'],
'X-PUT-Timestamp': info['put_timestamp']}
resp_headers.update((key, value)
for key, (value, timestamp) in
broker.metadata.iteritems() if value != '')
account_list = broker.list_containers_iter(limit, marker, end_marker,
prefix, delimiter)
if response_content_type == 'application/json':
data = []
for (name, object_count, bytes_used, is_subdir) in account_list:
if is_subdir:
data.append({'subdir': name})
else:
data.append({'name': name, 'count': object_count,
'bytes': bytes_used})
account_list = json.dumps(data)
elif response_content_type.endswith('/xml'):
output_list = ['<?xml version="1.0" encoding="UTF-8"?>',
'<account name="%s">' % account]
for (name, object_count, bytes_used, is_subdir) in account_list:
name = saxutils.escape(name)
if is_subdir:
output_list.append('<subdir name="%s" />' % name)
else:
item = '<container><name>%s</name><count>%s</count>' \
'<bytes>%s</bytes></container>' % \
(name, object_count, bytes_used)
output_list.append(item)
output_list.append('</account>')
account_list = '\n'.join(output_list)
else:
if not account_list:
resp = HTTPNoContent(request=req, headers=resp_headers)
resp.content_type = response_content_type
resp.charset = 'utf-8'
return resp
account_list = '\n'.join(r[0] for r in account_list) + '\n'
ret = HTTPOk(body=account_list, request=req, headers=resp_headers)
ret.content_type = response_content_type
ret.charset = 'utf-8'
return ret

2
swift/common/utils.py
View File

@ -1021,7 +1021,7 @@ def storage_directory(datadir, partition, hash):
def hash_path(account, container=None, object=None, raw_digest=False):
"""
Get the connonical hash for an account/container/object
Get the canonical hash for an account/container/object
:param account: Account
:param container: Container

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

@ -24,15 +24,15 @@
# These shenanigans are to ensure all related objects can be garbage
# collected. We've seen objects hang around forever otherwise.
import time
from urllib import unquote
from swift.common.utils import normalize_timestamp, public
from swift.account.utils import account_listing_response, \
account_listing_content_type
from swift.common.utils import public
from swift.common.constraints import check_metadata, MAX_ACCOUNT_NAME_LENGTH
from swift.common.http import HTTP_NOT_FOUND
from swift.proxy.controllers.base import Controller, get_account_memcache_key
from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed, \
HTTPNoContent
from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed
class AccountController(Controller):
@ -59,15 +59,11 @@ class AccountController(Controller):
req, _('Account'), self.app.account_ring, partition,
req.path_info.rstrip('/'))
if resp.status_int == HTTP_NOT_FOUND and self.app.account_autocreate:
# Fake a response
headers = {'Content-Length': '0',
'Accept-Ranges': 'bytes',
'Content-Type': 'text/plain; charset=utf-8',
'X-Timestamp': normalize_timestamp(time.time()),
'X-Account-Bytes-Used': '0',
'X-Account-Container-Count': '0',
'X-Account-Object-Count': '0'}
resp = HTTPNoContent(request=req, headers=headers)
content_type, error = account_listing_content_type(req)
if error:
return error
return account_listing_response(self.account_name, req,
content_type)
return resp
@public

115
test/probe/test_account_get_fake_responses_match.py Executable file
View File

@ -0,0 +1,115 @@
#!/usr/bin/python -u
# Copyright (c) 2010-2013 OpenStack, LLC.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import httplib
import re
import unittest
from swiftclient import client, get_auth
from test.probe.common import kill_servers, reset_environment
from urlparse import urlparse
class TestAccountGetFakeResponsesMatch(unittest.TestCase):
def setUp(self):
(self.pids, self.port2server, self.account_ring, self.container_ring,
self.object_ring, self.url, self.token,
self.account, self.configs) = reset_environment()
self.url, self.token = get_auth(
'http://127.0.0.1:8080/auth/v1.0', 'admin:admin', 'admin')
def tearDown(self):
kill_servers(self.port2server, self.pids)
def _account_path(self, account):
_, _, path, _, _, _ = urlparse(self.url)
basepath, _ = path.rsplit('/', 1)
return basepath + '/' + account
def _get(self, *a, **kw):
kw['method'] = 'GET'
return self._account_request(*a, **kw)
def _account_request(self, account, method, headers=None):
if headers is None:
headers = {}
headers['X-Auth-Token'] = self.token
scheme, netloc, path, _, _, _ = urlparse(self.url)
host, port = netloc.split(':')
port = int(port)
conn = httplib.HTTPConnection(host, port)
conn.request(method, self._account_path(account), headers=headers)
resp = conn.getresponse()
if resp.status // 100 != 2:
raise Exception("Unexpected status %s\n%s" %
(resp.status, resp.read()))
response_headers = dict(resp.getheaders())
response_body = resp.read()
resp.close()
return response_headers, response_body
def test_main(self):
# Two accounts: "real" and "fake". The fake one doesn't have any .db
# files on disk; the real one does. The real one is empty.
#
# Make sure the important response fields match.
real_acct = "AUTH_real"
fake_acct = "AUTH_fake"
self._account_request(real_acct, 'POST',
{'X-Account-Meta-Bert': 'Ernie'})
# text
real_headers, real_body = self._get(real_acct)
fake_headers, fake_body = self._get(fake_acct)
self.assertEqual(real_body, fake_body)
self.assertEqual(real_headers['content-type'],
fake_headers['content-type'])
# json
real_headers, real_body = self._get(
real_acct, headers={'Accept': 'application/json'})
fake_headers, fake_body = self._get(
fake_acct, headers={'Accept': 'application/json'})
self.assertEqual(real_body, fake_body)
self.assertEqual(real_headers['content-type'],
fake_headers['content-type'])
# xml
real_headers, real_body = self._get(
real_acct, headers={'Accept': 'application/xml'})
fake_headers, fake_body = self._get(
fake_acct, headers={'Accept': 'application/xml'})
# the account name is in the XML response
real_body = re.sub('AUTH_\w{4}', 'AUTH_someaccount', real_body)
fake_body = re.sub('AUTH_\w{4}', 'AUTH_someaccount', fake_body)
self.assertEqual(real_body, fake_body)
self.assertEqual(real_headers['content-type'],
fake_headers['content-type'])
if __name__ == '__main__':
unittest.main()

6
test/unit/account/test_server.py
View File

@ -406,6 +406,8 @@ class TestAccountController(unittest.TestCase):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.headers['Content-Type'],
'text/plain; charset=utf-8')
def test_GET_empty_account_json(self):
req = Request.blank('/sda1/p/a?format=json',
@ -415,6 +417,8 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
self.assertEquals(resp.headers['Content-Type'],
'application/json; charset=utf-8')
def test_GET_empty_account_xml(self):
req = Request.blank('/sda1/p/a?format=xml',
@ -424,6 +428,8 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
self.assertEquals(resp.headers['Content-Type'],
'application/xml; charset=utf-8')
def test_GET_over_limit(self):
req = Request.blank('/sda1/p/a?limit=%d' %

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

@ -5683,6 +5683,69 @@ class TestAccountController(unittest.TestCase):
test_status_map((204, 500, 404), 400)
class TestAccountControllerFakeGetResponse(unittest.TestCase):
"""
Test all the faked-out GET responses for accounts that don't exist. They
have to match the responses for empty accounts that really exist.
"""
def setUp(self):
self.app = proxy_server.Application(None, FakeMemcache(),
account_ring=FakeRing(),
container_ring=FakeRing(),
object_ring=FakeRing)
self.app.memcache = FakeMemcacheReturnsNone()
self.controller = proxy_server.AccountController(self.app, 'acc')
self.controller.app.account_autocreate = True
def test_GET_autocreate_accept_json(self):
with save_globals():
set_http_connect(404) # however many backends we ask, they all 404
req = Request.blank('/a', headers={'Accept': 'application/json'})
resp = self.controller.GET(req)
self.assertEqual(200, resp.status_int)
self.assertEqual('application/json; charset=utf-8',
resp.headers['Content-Type'])
self.assertEqual("[]", resp.body)
def test_GET_autocreate_format_json(self):
with save_globals():
set_http_connect(404) # however many backends we ask, they all 404
req = Request.blank('/a?format=json')
resp = self.controller.GET(req)
self.assertEqual(200, resp.status_int)
self.assertEqual('application/json; charset=utf-8',
resp.headers['Content-Type'])
self.assertEqual("[]", resp.body)
def test_GET_autocreate_accept_xml(self):
with save_globals():
set_http_connect(404) # however many backends we ask, they all 404
req = Request.blank('/a', headers={"Accept": "text/xml"})
resp = self.controller.GET(req)
self.assertEqual(200, resp.status_int)
self.assertEqual('text/xml; charset=utf-8',
resp.headers['Content-Type'])
empty_xml_listing = ('<?xml version="1.0" encoding="UTF-8"?>\n'
'<account name="acc">\n</account>')
self.assertEqual(empty_xml_listing, resp.body)
def test_GET_autocreate_format_xml(self):
with save_globals():
set_http_connect(404) # however many backends we ask, they all 404
req = Request.blank('/a?format=xml')
resp = self.controller.GET(req)
self.assertEqual(200, resp.status_int)
self.assertEqual('application/xml; charset=utf-8',
resp.headers['Content-Type'])
empty_xml_listing = ('<?xml version="1.0" encoding="UTF-8"?>\n'
'<account name="acc">\n</account>')
self.assertEqual(empty_xml_listing, resp.body)
class FakeObjectController(object):
def __init__(self):