openstack/swift
Code Issues Proposed changes

Adapt Swift for WebOb 1.2

Browse Source 
Based on PatchSet 3 of https://review.openstack.org/#/c/7569/ , make them to pass all funcional tests with both webob 1.x and 1.2.

The additional following compatibility issues were addressed:
 - Until patch for range header issue is merged into official webob release, testRangedGetsWithLWSinHeader() should skip test against webob 1.2
(49c175aec2)

 - common.constraints.check_utf8() can accept both utf8 str and unicode.

 - To convert unicode to utf-8 str if necessary.

 - Making proxy_logging can handle invalid utf-8 str

bug 888371
bug 959881

blueprint webob-support

Change-Id: I00e5fd04cd1653259606a4ffdd4926db3c84c496
This commit is contained in:
Iryoung Jeong 2012-06-06 03:39:53 +09:00
parent 0ab4f2ab4a
commit de4d23c2a5
20 changed files with 196 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
swift/account/server.py
View File

@ -252,7 +252,11 @@ class AccountController(object):
return HTTPBadRequest(body='parameters not utf8',
content_type='text/plain', request=req)
if query_format:
req.accept = 'application/%s' % query_format.lower()
qfmt_lower = query_format.lower()
if qfmt_lower not in ['xml', 'json', 'plain']:
return HTTPBadRequest(body='format not supported',
content_type='text/plain', request=req)
req.accept = 'application/%s' % qfmt_lower
try:
out_content_type = req.accept.best_match(
['text/plain', 'application/json',

5
swift/common/bufferedhttp.py
View File

@ -131,6 +131,11 @@ def http_connect(ipaddr, port, device, partition, method, path,
conn = HTTPSConnection('%s:%s' % (ipaddr, port))
else:
conn = BufferedHTTPConnection('%s:%s' % (ipaddr, port))
if isinstance(path, unicode):
try:
path = path.encode("utf-8")
except UnicodeError:
pass # what should I do?
path = quote('/' + device + '/' + str(partition) + path)
if query_string:
path += '?' + query_string

19
swift/common/constraints.py
View File

@ -159,13 +159,20 @@ def check_float(string):
def check_utf8(string):
"""
Validate if a string is valid UTF-8.
Validate if a string is valid UTF-8 str or unicode
:param string: string to be validated
:returns: True if the string is valid utf-8, False otherwise
:returns: True if the string is valid utf-8 str or unicode, False otherwise
"""
try:
string.decode('UTF-8')
return True
except UnicodeDecodeError:
if not string:
return False
try:
if isinstance(string, unicode):
string.encode('utf-8')
else:
string.decode('UTF-8')
return True
# If string is unicode, decode() will raise UnicodeEncodeError
# So, we should catch both UnicodeDecodeError & UnicodeEncodeError
except UnicodeError:
return False

2
swift/common/db.py
View File

@ -1102,6 +1102,8 @@ class ContainerBroker(DatabaseBroker):
if prefix is None:
return [r for r in curs]
if not delimiter:
if isinstance(prefix, unicode):
prefix = prefix.encode("utf-8")
return [r for r in curs if r[0].startswith(prefix)]
rowcount = 0
for row in curs:

11
swift/common/middleware/healthcheck.py
View File

@ -32,10 +32,13 @@ class HealthCheckMiddleware(object):
def __call__(self, env, start_response):
req = Request(env)
if req.path == '/healthcheck':
return self.GET(req)(env, start_response)
else:
return self.app(env, start_response)
try:
if req.path == '/healthcheck':
return self.GET(req)(env, start_response)
except UnicodeError:
# definitely, this is not /healthcheck
pass
return self.app(env, start_response)
def filter_factory(global_conf, **local_conf):

6
swift/common/middleware/proxy_logging.py
View File

@ -42,7 +42,8 @@ from urllib import quote, unquote
from webob import Request
from swift.common.utils import get_logger, get_remote_client, TRUE_VALUES
from swift.common.utils import (get_logger, get_remote_client,
get_valid_utf8_str, TRUE_VALUES)
class InputProxy(object):
@ -116,7 +117,8 @@ class ProxyLoggingMiddleware(object):
req = Request(env)
if client_disconnect: # log disconnected clients as '499' status code
status_int = 499
the_request = quote(unquote(req.path))
req_path = get_valid_utf8_str(env.get('PATH_INFO', ''))
the_request = quote(unquote(req_path))
if req.query_string:
the_request = the_request + '?' + req.query_string
logged_headers = None

19
swift/common/utils.py
View File

@ -45,6 +45,9 @@ import eventlet
from eventlet import GreenPool, sleep, Timeout
from eventlet.green import socket, threading
import netifaces
import codecs
utf8_decoder = codecs.getdecoder('utf-8')
utf8_encoder = codecs.getencoder('utf-8')
from swift.common.exceptions import LockTimeout, MessageTimeout
@ -114,8 +117,8 @@ def get_param(req, name, default=None):
:param default: result to return if the parameter is not found
:returns: HTTP request parameter value
"""
value = req.str_params.get(name, default)
if value:
value = req.params.get(name, default)
if value and not isinstance(value, unicode):
value.decode('utf8') # Ensure UTF8ness
return value
@ -1340,3 +1343,15 @@ def rsync_ip(ip):
return ip
else:
return '[%s]' % ip
def get_valid_utf8_str(str_or_unicode):
"""
Get valid parts of utf-8 str from str, unicode and even invalid utf-8 str
:param str_or_unicode: a string or an unicode which can be invalid utf-8
"""
if isinstance(str_or_unicode, unicode):
(str_or_unicode, _len) = utf8_encoder(str_or_unicode, 'replace')
(valid_utf8_str, _len) = utf8_decoder(str_or_unicode, 'replace')
return valid_utf8_str.encode('utf-8')

6
swift/container/server.py
View File

@ -348,7 +348,11 @@ class ContainerController(object):
return HTTPBadRequest(body='parameters not utf8',
content_type='text/plain', request=req)
if query_format:
req.accept = 'application/%s' % query_format.lower()
qfmt_lower = query_format.lower()
if qfmt_lower not in ['xml', 'json', 'plain']:
return HTTPBadRequest(body='format not supported',
content_type='text/plain', request=req)
req.accept = 'application/%s' % qfmt_lower
try:
out_content_type = req.accept.best_match(
['text/plain', 'application/json',

1
swift/obj/server.py
View File

@ -874,6 +874,7 @@ class ObjectController(object):
start_time = time.time()
req = Request(env)
self.logger.txn_id = req.headers.get('x-trans-id', None)
if not check_utf8(req.path_info):
res = HTTPPreconditionFailed(body='Invalid UTF8')
else:

31
swift/proxy/server.py
View File

@ -809,6 +809,8 @@ class Controller(object):
res.swift_conn = source.swift_conn
update_headers(res, source.getheaders())
# Used by container sync feature
if res.environ is None:
res.environ = dict()
res.environ['swift_x_timestamp'] = \
source.getheader('x-timestamp')
update_headers(res, {'accept-ranges': 'bytes'})
@ -822,6 +824,8 @@ class Controller(object):
res = status_map[source.status](request=req)
update_headers(res, source.getheaders())
# Used by container sync feature
if res.environ is None:
res.environ = dict()
res.environ['swift_x_timestamp'] = \
source.getheader('x-timestamp')
update_headers(res, {'accept-ranges': 'bytes'})
@ -913,7 +917,7 @@ class ObjectController(Controller):
'%s.timing' % (stats_type,), start_time)
return resp
resp = resp2
req.range = req_range
req.range = str(req_range)
if 'x-object-manifest' in resp.headers:
lcontainer, lprefix = \
@ -1176,7 +1180,7 @@ class ObjectController(Controller):
try:
req.headers['X-Timestamp'] = \
normalize_timestamp(float(req.headers['x-timestamp']))
if 'swift_x_timestamp' in hresp.environ and \
if hresp.environ and 'swift_x_timestamp' in hresp.environ and \
float(hresp.environ['swift_x_timestamp']) >= \
float(req.headers['x-timestamp']):
self.app.logger.timing_since(
@ -1240,6 +1244,8 @@ class ObjectController(Controller):
if source_header:
source_header = unquote(source_header)
acct = req.path_info.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
@ -1964,9 +1970,10 @@ class BaseApplication(object):
self.memcache = cache_from_env(env)
req = self.update_request(Request(env))
return self.handle_request(req)(env, start_response)
except UnicodeError:
err = HTTPPreconditionFailed(request=req, body='Invalid UTF8')
return err(env, start_response)
except (Exception, Timeout):
print "EXCEPTION IN __call__: %s: %s" % \
(traceback.format_exc(), env)
start_response('500 Server Error',
[('Content-Type', 'text/plain')])
return ['Internal server error.\n']
@ -1990,14 +1997,24 @@ class BaseApplication(object):
self.logger.increment('errors')
return HTTPBadRequest(request=req,
body='Invalid Content-Length')
try:
if not check_utf8(req.path_info):
self.logger.increment('errors')
return HTTPPreconditionFailed(request=req,
body='Invalid UTF8')
except UnicodeError:
self.logger.increment('errors')
return HTTPPreconditionFailed(request=req, body='Invalid UTF8')
try:
controller, path_parts = self.get_controller(req.path)
p = req.path_info
if isinstance(p, unicode):
p = p.encode('utf-8')
except ValueError:
self.logger.increment('errors')
return HTTPNotFound(request=req)
if not check_utf8(req.path_info):
self.logger.increment('errors')
return HTTPPreconditionFailed(request=req, body='Invalid UTF8')
if not controller:
self.logger.increment('errors')
return HTTPPreconditionFailed(request=req, body='Bad URL')

7
test/functional/swift.py
View File

@ -201,7 +201,6 @@ class Connection(object):
path = self.make_path(path, cfg=cfg)
headers = self.make_headers(hdrs, cfg=cfg)
if isinstance(parms, dict) and parms:
quote = urllib.quote
if cfg.get('no_quote') or cfg.get('no_parms_quote'):
@ -209,7 +208,6 @@ class Connection(object):
query_args = ['%s=%s' % (quote(x), quote(str(y))) for (x,y) in
parms.items()]
path = '%s?%s' % (path, '&'.join(query_args))
if not cfg.get('no_content_length'):
if cfg.get('set_content_length'):
headers['Content-Length'] = cfg.get('set_content_length')
@ -230,7 +228,7 @@ class Connection(object):
self.response = try_request()
except httplib.HTTPException:
continue
if self.response.status == 401:
self.authenticate()
continue
@ -244,7 +242,7 @@ class Connection(object):
if self.response:
return self.response.status
raise RequestError('Unable to compelte http request')
raise RequestError('Unable to complete http request')
def put_start(self, path, hdrs={}, parms={}, cfg={}, chunked=False):
self.http_connect()
@ -296,7 +294,6 @@ class Base:
def header_fields(self, fields):
headers = dict(self.conn.response.getheaders())
ret = {}
for field in fields:
if not headers.has_key(field[1]):

12
test/functional/tests.py
View File

@ -22,6 +22,7 @@ import time
import threading
import uuid
import unittest
from nose import SkipTest
from test import get_config
from test.functional.swift import Account, Connection, File, ResponseError
@ -1078,6 +1079,17 @@ class TestFile(Base):
hdrs = {'Range': '0-4'}
self.assert_(file.read(hdrs=hdrs) == data, range_string)
def testRangedGetsWithLWSinHeader(self):
#Skip this test until webob 1.2 can tolerate LWS in Range header.
from webob.byterange import Range
if not isinstance(Range.parse('bytes = 0-99 '), Range):
raise SkipTest
file_length = 10000
range_size = file_length/10
file = self.env.container.file(Utils.create_name())
data = file.write_random(file_length)
for r in ('BYTES=0-999', 'bytes = 0-999', 'BYTES = 0 - 999',
'bytes = 0 - 999', 'bytes=0 - 999', 'bytes=0-999 '):

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

@ -986,11 +986,25 @@ class TestAccountController(unittest.TestCase):
self.assertEquals(errbuf.getvalue(), '')
self.assertEquals(outbuf.getvalue()[:4], '405 ')
def test_params_format(self):
self.controller.PUT(Request.blank('/sda1/p/a',
headers={'X-Timestamp': normalize_timestamp(1)},
environ={'REQUEST_METHOD': 'PUT'}))
for format in ('xml', 'json'):
req = Request.blank('/sda1/p/a?format=%s' % format,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
req = Request.blank('/sda1/p/a?format=Foo!',
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 400)
def test_params_utf8(self):
self.controller.PUT(Request.blank('/sda1/p/a',
headers={'X-Timestamp': normalize_timestamp(1)},
environ={'REQUEST_METHOD': 'PUT'}))
for param in ('delimiter', 'format', 'limit', 'marker', 'prefix'):
for param in ('delimiter', 'limit', 'marker', 'prefix'):
req = Request.blank('/sda1/p/a?%s=\xce' % param,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)

50
test/unit/common/middleware/test_tempauth.py
View File

@ -140,16 +140,17 @@ class TestAuth(unittest.TestCase):
self.assertEquals(ath.auth_prefix, '/test/')
def test_top_level_deny(self):
resp = self._make_request('/').get_response(self.test_auth)
req = self._make_request('/')
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
self.test_auth.denied_response)
def test_anon(self):
resp = \
self._make_request('/v1/AUTH_account').get_response(self.test_auth)
req = self._make_request('/v1/AUTH_account')
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
self.test_auth.authorize)
def test_override_asked_for_but_not_allowed(self):
@ -159,7 +160,7 @@ class TestAuth(unittest.TestCase):
environ={'swift.authorize_override': True})
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
self.test_auth.authorize)
def test_override_asked_for_and_allowed(self):
@ -169,30 +170,32 @@ class TestAuth(unittest.TestCase):
environ={'swift.authorize_override': True})
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 404)
self.assertTrue('swift.authorize' not in resp.environ)
self.assertTrue('swift.authorize' not in req.environ)
def test_override_default_allowed(self):
req = self._make_request('/v1/AUTH_account',
environ={'swift.authorize_override': True})
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 404)
self.assertTrue('swift.authorize' not in resp.environ)
self.assertTrue('swift.authorize' not in req.environ)
def test_auth_deny_non_reseller_prefix(self):
resp = self._make_request('/v1/BLAH_account',
headers={'X-Auth-Token': 'BLAH_t'}).get_response(self.test_auth)
req = self._make_request('/v1/BLAH_account',
headers={'X-Auth-Token': 'BLAH_t'})
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
self.test_auth.denied_response)
def test_auth_deny_non_reseller_prefix_no_override(self):
fake_authorize = lambda x: Response(status='500 Fake')
resp = self._make_request('/v1/BLAH_account',
req = self._make_request('/v1/BLAH_account',
headers={'X-Auth-Token': 'BLAH_t'},
environ={'swift.authorize': fake_authorize}
).get_response(self.test_auth)
)
resp = req.get_response(self.test_auth)
self.assertEquals(resp.status_int, 500)
self.assertEquals(resp.environ['swift.authorize'], fake_authorize)
self.assertEquals(req.environ['swift.authorize'], fake_authorize)
def test_auth_no_reseller_prefix_deny(self):
# Ensures that when we have no reseller prefix, we don't deny a request
@ -200,30 +203,33 @@ class TestAuth(unittest.TestCase):
# down the chain.
local_app = FakeApp()
local_auth = auth.filter_factory({'reseller_prefix': ''})(local_app)
resp = self._make_request('/v1/account',
headers={'X-Auth-Token': 't'}).get_response(local_auth)
req = self._make_request('/v1/account',
headers={'X-Auth-Token': 't'})
resp = req.get_response(local_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(local_app.calls, 1)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
local_auth.denied_response)
def test_auth_no_reseller_prefix_no_token(self):
# Check that normally we set up a call back to our authorize.
local_auth = \
auth.filter_factory({'reseller_prefix': ''})(FakeApp(iter([])))
resp = self._make_request('/v1/account').get_response(local_auth)
req = self._make_request('/v1/account')
resp = req.get_response(local_auth)
self.assertEquals(resp.status_int, 401)
self.assertEquals(resp.environ['swift.authorize'],
self.assertEquals(req.environ['swift.authorize'],
local_auth.authorize)
# Now make sure we don't override an existing swift.authorize when we
# have no reseller prefix.
local_auth = \
auth.filter_factory({'reseller_prefix': ''})(FakeApp())
local_authorize = lambda req: Response('test')
resp = self._make_request('/v1/account', environ={'swift.authorize':
local_authorize}).get_response(local_auth)
req = self._make_request('/v1/account', environ={'swift.authorize':
local_authorize})
resp = req.get_response(local_auth)
self.assertEquals(resp.status_int, 200)
self.assertEquals(resp.environ['swift.authorize'], local_authorize)
self.assertEquals(req.environ['swift.authorize'], local_authorize)
def test_auth_fail(self):
resp = self._make_request('/v1/AUTH_cfa',

16
test/unit/common/middleware/test_tempurl.py
View File

@ -108,8 +108,8 @@ class TestTempURL(unittest.TestCase):
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.headers['content-disposition'],
'attachment; filename=o')
self.assertEquals(resp.environ['swift.authorize_override'], True)
self.assertEquals(resp.environ['REMOTE_USER'], '.wsgi.tempurl')
self.assertEquals(req.environ['swift.authorize_override'], True)
self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl')
def test_put_not_allowed_by_get(self):
method = 'GET'
@ -141,8 +141,8 @@ class TestTempURL(unittest.TestCase):
req.environ['swift.cache'].set('temp-url-key/a', key)
resp = req.get_response(self.tempurl)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.environ['swift.authorize_override'], True)
self.assertEquals(resp.environ['REMOTE_USER'], '.wsgi.tempurl')
self.assertEquals(req.environ['swift.authorize_override'], True)
self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl')
def test_get_not_allowed_by_put(self):
method = 'PUT'
@ -230,8 +230,8 @@ class TestTempURL(unittest.TestCase):
req.environ['swift.cache'].set('temp-url-key/a', key)
resp = req.get_response(self.tempurl)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.environ['swift.authorize_override'], True)
self.assertEquals(resp.environ['REMOTE_USER'], '.wsgi.tempurl')
self.assertEquals(req.environ['swift.authorize_override'], True)
self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl')
def test_head_allowed_by_put(self):
method = 'PUT'
@ -247,8 +247,8 @@ class TestTempURL(unittest.TestCase):
req.environ['swift.cache'].set('temp-url-key/a', key)
resp = req.get_response(self.tempurl)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.environ['swift.authorize_override'], True)
self.assertEquals(resp.environ['REMOTE_USER'], '.wsgi.tempurl')
self.assertEquals(req.environ['swift.authorize_override'], True)
self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl')
def test_head_otherwise_not_allowed(self):
method = 'PUT'

16
test/unit/common/test_constraints.py
View File

@ -174,5 +174,21 @@ class TestConstraints(unittest.TestCase):
self.assertFalse(constraints.check_float(''))
self.assertTrue(constraints.check_float('0'))
def test_check_utf8(self):
unicode_sample = u'\uc77c\uc601'
valid_utf8_str = unicode_sample.encode('utf-8')
invalid_utf8_str = unicode_sample.encode('utf-8')[::-1]
for false_argument in [None,
'',
invalid_utf8_str,
]:
self.assertFalse(constraints.check_utf8(false_argument))
for true_argument in ['this is ascii and utf-8, too',
unicode_sample,
valid_utf8_str]:
self.assertTrue(constraints.check_utf8(true_argument))
if __name__ == '__main__':
unittest.main()

11
test/unit/common/test_utils.py
View File

@ -1149,6 +1149,17 @@ class TestStatsdLoggingDelegation(unittest.TestCase):
self.logger.update_stats, 'another.counter', 3,
sample_rate=0.9912)
def test_get_valid_utf8_str(self):
unicode_sample = u'\uc77c\uc601'
valid_utf8_str = unicode_sample.encode('utf-8')
invalid_utf8_str = unicode_sample.encode('utf-8')[::-1]
self.assertEquals(valid_utf8_str,
utils.get_valid_utf8_str(valid_utf8_str))
self.assertEquals(valid_utf8_str,
utils.get_valid_utf8_str(unicode_sample))
self.assertEquals('\xef\xbf\xbd\xef\xbf\xbd\xec\xbc\x9d\xef\xbf\xbd',
utils.get_valid_utf8_str(invalid_utf8_str))
if __name__ == '__main__':
unittest.main()

17
test/unit/container/test_server.py
View File

@ -952,12 +952,25 @@ class TestContainerController(unittest.TestCase):
self.assertEquals(errbuf.getvalue(), '')
self.assertEquals(outbuf.getvalue()[:4], '405 ')
def test_params_format(self):
self.controller.PUT(Request.blank('/sda1/p/a/c',
headers={'X-Timestamp': normalize_timestamp(1)},
environ={'REQUEST_METHOD': 'PUT'}))
for format in ('xml', 'json'):
req = Request.blank('/sda1/p/a/c?format=%s' % format,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
req = Request.blank('/sda1/p/a/c?format=Foo!',
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 400)
def test_params_utf8(self):
self.controller.PUT(Request.blank('/sda1/p/a/c',
headers={'X-Timestamp': normalize_timestamp(1)},
environ={'REQUEST_METHOD': 'PUT'}))
for param in ('delimiter', 'format', 'limit', 'marker', 'path',
'prefix'):
for param in ('delimiter', 'limit', 'marker', 'path', 'prefix'):
req = Request.blank('/sda1/p/a/c?%s=\xce' % param,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)

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

@ -4075,10 +4075,11 @@ class FakeObjectController(object):
req = args[0]
path = args[4]
body = data = path[-1] * int(path[-1])
if req.range and req.range.ranges:
body = ''
for start, stop in req.range.ranges:
body += data[start:stop]
if req.range:
r = req.range.range_for_length(len(data))
if r:
(start, stop) = r
body = data[start:stop]
resp = Response(app_iter=iter(body))
return resp

2
tools/pip-requires
View File

@ -1,4 +1,4 @@
WebOb==1.0.8
WebOb>=1.0.8,<1.3
configobj==4.7.1
eventlet==0.9.15
greenlet==0.3.1