Skip to content

Commit

Permalink
Fix faked-out account GET for JSON and XML.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
smerritt committed May 31, 2013
1 parent 5501a40 commit 15c2ca5
Show file tree
Hide file tree
Showing 7 changed files with 325 additions and 71 deletions.
67 changes: 10 additions & 57 deletions swift/account/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
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, \
Expand All @@ -33,7 +34,7 @@
from swift.common.swob import HTTPAccepted, HTTPBadRequest, \
HTTPCreated, HTTPForbidden, HTTPInternalServerError, \
HTTPMethodNotAllowed, HTTPNoContent, HTTPNotFound, \
HTTPPreconditionFailed, HTTPConflict, Request, Response, \
HTTPPreconditionFailed, HTTPConflict, Request, \
HTTPInsufficientStorage, HTTPNotAcceptable


Expand Down Expand Up @@ -219,71 +220,23 @@ def GET(self, req):
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)
broker.pending_timeout = 0.1
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()
Expand Down
121 changes: 121 additions & 0 deletions swift/account/utils.py
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion swift/common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 9 additions & 13 deletions swift/proxy/controllers/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -59,15 +59,11 @@ def GETorHEAD(self, req):
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
Expand Down
115 changes: 115 additions & 0 deletions test/probe/test_account_get_fake_responses_match.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit 15c2ca5

Please sign in to comment.