openstack
/
swift
Code Issues Proposed changes

Update to comply with XML 1.1: No NULLs in names allowed; control chars are converted to entities (ex: )

This commit is contained in:
gholt 2011-06-17 00:57:00 +00:00
parent fc68f824f1
commit 082b324bc3
10 changed files with 230 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
swift/account/server.py
View File

@ -29,7 +29,7 @@ import simplejson
from swift.common.db import AccountBroker
from swift.common.utils import get_logger, get_param, hash_path, \
normalize_timestamp, split_path, storage_directory
normalize_timestamp, split_path, storage_directory, XML_EXTRA_ENTITIES
from swift.common.constraints import ACCOUNT_LISTING_LIMIT, \
check_mount, check_float, check_utf8
from swift.common.db_replicator import ReplicatorRpc
@ -79,6 +79,9 @@ class AccountController(object):
try:
drive, part, account, container = split_path(unquote(req.path),
3, 4)
if (account and not check_utf8(account)) or \
(container and not check_utf8(container)):
raise ValueError('NULL characters not allowed in names')
except ValueError, err:
return HTTPBadRequest(body=str(err), content_type='text/plain',
request=req)
@ -201,8 +204,8 @@ class AccountController(object):
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',
except (UnicodeDecodeError, ValueError), err:
return HTTPBadRequest(body='parameters not utf8 or contain NULLs',
content_type='text/plain', request=req)
if query_format:
req.accept = 'application/%s' % query_format.lower()
@ -228,7 +231,7 @@ class AccountController(object):
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)
name = saxutils.escape(name, XML_EXTRA_ENTITIES)
if is_subdir:
output_list.append('<subdir name="%s" />' % name)
else:

9
swift/common/constraints.py
View File

@ -159,13 +159,16 @@ def check_float(string):
def check_utf8(string):
"""
Validate if a string is valid UTF-8.
Validate if a string is valid UTF-8 and has no NULL characters.
: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 and has no NULL characters,
False otherwise
"""
try:
string.decode('UTF-8')
return True
except UnicodeDecodeError:
return False
if '\x00' in string:
return False
return True

7
swift/common/utils.py
View File

@ -42,6 +42,7 @@ from eventlet import greenio, GreenPool, sleep, Timeout, listen
from eventlet.green import socket, subprocess, ssl, thread, threading
import netifaces
from swift.common.constraints import check_utf8
from swift.common.exceptions import LockTimeout, MessageTimeout
# logging doesn't import patched as cleanly as one would like
@ -74,6 +75,8 @@ if hash_conf.read('/etc/swift/swift.conf'):
# Used when reading config values
TRUE_VALUES = set(('true', '1', 'yes', 'on', 't', 'y'))
# Used with xml.sax.saxutils.escape
XML_EXTRA_ENTITIES = dict((chr(x), '&#x%x;' % x) for x in xrange(1, 20))
def validate_configuration():
if HASH_PATH_SUFFIX == '':
@ -110,8 +113,8 @@ def get_param(req, name, default=None):
:returns: HTTP request parameter value
"""
value = req.str_params.get(name, default)
if value:
value.decode('utf8') # Ensure UTF8ness
if value and not check_utf8(value):
raise ValueError('Not valid UTF-8 or contains NULL characters')
return value

16
swift/container/server.py
View File

@ -32,7 +32,7 @@ from webob.exc import HTTPAccepted, HTTPBadRequest, HTTPConflict, \
from swift.common.db import ContainerBroker
from swift.common.utils import get_logger, get_param, hash_path, \
normalize_timestamp, storage_directory, split_path
normalize_timestamp, storage_directory, split_path, XML_EXTRA_ENTITIES
from swift.common.constraints import CONTAINER_LISTING_LIMIT, \
check_mount, check_float, check_utf8
from swift.common.bufferedhttp import http_connect
@ -167,6 +167,10 @@ class ContainerController(object):
try:
drive, part, account, container, obj = split_path(
unquote(req.path), 4, 5, True)
if (account and not check_utf8(account)) or \
(container and not check_utf8(container)) or \
(obj and not check_utf8(obj)):
raise ValueError('NULL characters not allowed in names')
except ValueError, err:
return HTTPBadRequest(body=str(err), content_type='text/plain',
request=req)
@ -277,7 +281,7 @@ class ContainerController(object):
return HTTPPreconditionFailed(request=req,
body='Maximum limit is %d' % CONTAINER_LISTING_LIMIT)
query_format = get_param(req, 'format')
except UnicodeDecodeError, err:
except (UnicodeDecodeError, ValueError), err:
return HTTPBadRequest(body='parameters not utf8',
content_type='text/plain', request=req)
if query_format:
@ -312,21 +316,23 @@ class ContainerController(object):
xml_output = []
for (name, created_at, size, content_type, etag) in container_list:
# escape name and format date here
name = saxutils.escape(name)
name = saxutils.escape(name, XML_EXTRA_ENTITIES)
created_at = datetime.utcfromtimestamp(
float(created_at)).isoformat()
if content_type is None:
xml_output.append('<subdir name="%s"><name>%s</name>'
'</subdir>' % (name, name))
else:
content_type = saxutils.escape(content_type)
content_type = saxutils.escape(content_type,
XML_EXTRA_ENTITIES)
xml_output.append('<object><name>%s</name><hash>%s</hash>'\
'<bytes>%d</bytes><content_type>%s</content_type>'\
'<last_modified>%s</last_modified></object>' % \
(name, etag, size, content_type, created_at))
container_list = ''.join([
'<?xml version="1.0" encoding="UTF-8"?>\n',
'<container name=%s>' % saxutils.quoteattr(container),
'<container name=%s>' %
saxutils.quoteattr(container, XML_EXTRA_ENTITIES),
''.join(xml_output), '</container>'])
else:
if not container_list:

27
test/functionalnosetests/test_account.py
View File

@ -2,6 +2,7 @@
import unittest
from nose import SkipTest
from uuid import uuid4
from swift.common.constraints import MAX_META_COUNT, MAX_META_NAME_LENGTH, \
MAX_META_OVERALL_SIZE, MAX_META_VALUE_LENGTH
@ -132,6 +133,32 @@ class TestAccount(unittest.TestCase):
resp.read()
self.assertEquals(resp.status, 400)
def test_name_control_chars(self):
if skip:
raise SkipTest
container = uuid4().hex
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s%%01test' %
(parsed.path, container), '',
{'X-Auth-Token': token, 'Content-Length': '0'})
return check_response(conn)
resp = retry(put)
resp.read()
self.assertTrue(resp.status in (201, 202))
def get(url, token, parsed, conn):
conn.request('GET', '%s?format=xml' % (parsed.path,), '',
{'X-Auth-Token': token})
return check_response(conn)
resp = retry(get)
body = resp.read()
self.assertEquals(resp.status, 200)
self.assertTrue('<name>%s&#x1;test</name>' % (container,) in body)
if __name__ == '__main__':
unittest.main()

44
test/functionalnosetests/test_container.py
View File

@ -522,6 +522,50 @@ class TestContainer(unittest.TestCase):
resp.read()
self.assertEquals(resp.status, 201)
def test_name_control_chars(self):
if skip:
raise SkipTest
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s%%00test' % (parsed.path, self.name), '',
{'X-Auth-Token': token})
return check_response(conn)
resp = retry(put)
resp.read()
# NULLs not allowed
self.assertEquals(resp.status, 412)
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s%%01test' % (parsed.path, self.name), '',
{'X-Auth-Token': token})
return check_response(conn)
resp = retry(put)
resp.read()
# 0x01 allowed
self.assertTrue(resp.status in (201, 202))
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s/object%%01test' %
(parsed.path, self.name), '',
{'X-Auth-Token': token, 'Content-Length': '0'})
return check_response(conn)
resp = retry(put)
resp.read()
self.assertTrue(resp.status in (201, 202))
def get(url, token, parsed, conn):
conn.request('GET', '%s/%s?format=xml' % (parsed.path, self.name),
'', {'X-Auth-Token': token})
return check_response(conn)
resp = retry(get)
body = resp.read()
self.assertEquals(resp.status, 200)
self.assertTrue('<name>object&#x1;test</name>' in body)
if __name__ == '__main__':
unittest.main()

24
test/functionalnosetests/test_object.py
View File

@ -541,6 +541,30 @@ class TestObject(unittest.TestCase):
resp.read()
self.assertEquals(resp.status, 204)
def test_name_control_chars(self):
if skip:
raise SkipTest
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s/obj%%00test' % (parsed.path,
self.container), 'test', {'X-Auth-Token': token})
return check_response(conn)
resp = retry(put)
resp.read()
# NULLs not allowed
self.assertEquals(resp.status, 412)
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s/obj%%01test' % (parsed.path,
self.container), 'test', {'X-Auth-Token': token})
return check_response(conn)
resp = retry(put)
resp.read()
# 0x01 allowed
self.assertEquals(resp.status, 201)
if __name__ == '__main__':
unittest.main()

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

@ -22,6 +22,7 @@ from StringIO import StringIO
import simplejson
import xml.dom.minidom
from webob import Request
from xml.parsers.expat import ExpatError
from swift.account.server import AccountController, ACCOUNT_LISTING_LIMIT
from swift.common.utils import normalize_timestamp
@ -450,7 +451,8 @@ class TestAccountController(unittest.TestCase):
'X-Bytes-Used': '0',
'X-Timestamp': normalize_timestamp(0)})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'},
req = Request.blank('/sda1/p/a/c2%04',
environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Put-Timestamp': '2',
'X-Delete-Timestamp': '0',
'X-Object-Count': '0',
@ -462,7 +464,15 @@ class TestAccountController(unittest.TestCase):
resp = self.controller.GET(req)
self.assertEquals(resp.content_type, 'application/xml')
self.assertEquals(resp.status_int, 200)
dom = xml.dom.minidom.parseString(resp.body)
try:
dom = xml.dom.minidom.parseString(resp.body)
except ExpatError, err:
# Expat doesn't like control characters, which are XML 1.1
# compatible. Soooo, we have to replace them. We'll do a specific
# replace in this case, but real code that uses Expat will need
# something more resilient.
dom = xml.dom.minidom.parseString(
resp.body.replace('&#x4;', '\\x04'))
self.assertEquals(dom.firstChild.nodeName, 'account')
listing = \
[n for n in dom.firstChild.childNodes if n.nodeName != '#text']
@ -483,7 +493,7 @@ class TestAccountController(unittest.TestCase):
self.assertEquals(sorted([n.nodeName for n in container]),
['bytes', 'count', 'name'])
node = [n for n in container if n.nodeName == 'name'][0]
self.assertEquals(node.firstChild.nodeValue, 'c2')
self.assertEquals(node.firstChild.nodeValue, 'c2\\x04')
node = [n for n in container if n.nodeName == 'count'][0]
self.assertEquals(node.firstChild.nodeValue, '0')
node = [n for n in container if n.nodeName == 'bytes'][0]
@ -495,7 +505,8 @@ class TestAccountController(unittest.TestCase):
'X-Bytes-Used': '2',
'X-Timestamp': normalize_timestamp(0)})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'},
req = Request.blank('/sda1/p/a/c2%04',
environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Put-Timestamp': '2',
'X-Delete-Timestamp': '0',
'X-Object-Count': '3',
@ -506,7 +517,15 @@ class TestAccountController(unittest.TestCase):
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
dom = xml.dom.minidom.parseString(resp.body)
try:
dom = xml.dom.minidom.parseString(resp.body)
except ExpatError, err:
# Expat doesn't like control characters, which are XML 1.1
# compatible. Soooo, we have to replace them. We'll do a specific
# replace in this case, but real code that uses Expat will need
# something more resilient.
dom = xml.dom.minidom.parseString(
resp.body.replace('&#x4;', '\\x04'))
self.assertEquals(dom.firstChild.nodeName, 'account')
listing = \
[n for n in dom.firstChild.childNodes if n.nodeName != '#text']
@ -526,7 +545,7 @@ class TestAccountController(unittest.TestCase):
self.assertEquals(sorted([n.nodeName for n in container]),
['bytes', 'count', 'name'])
node = [n for n in container if n.nodeName == 'name'][0]
self.assertEquals(node.firstChild.nodeValue, 'c2')
self.assertEquals(node.firstChild.nodeValue, 'c2\\x04')
node = [n for n in container if n.nodeName == 'count'][0]
self.assertEquals(node.firstChild.nodeValue, '3')
node = [n for n in container if n.nodeName == 'bytes'][0]
@ -959,6 +978,35 @@ class TestAccountController(unittest.TestCase):
resp = self.controller.GET(req)
self.assert_(resp.status_int in (204, 412), resp.status_int)
def test_params_no_null(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'):
req = Request.blank('/sda1/p/a?%s=\x00' % param,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 400)
def test_PUT_account_no_null(self):
req = Request.blank('/sda1/p/test\x00test',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 400)
def test_PUT_container_no_null(self):
req = Request.blank('/sda1/p/a',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
req = Request.blank('/sda1/p/a/test\x00test',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_PUT_TIMESTAMP': '1',
'HTTP_X_DELETE_TIMESTAMP': '0',
'HTTP_X_OBJECT_COUNT': '0', 'HTTP_X_BYTES_USED': '0'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 400)
if __name__ == '__main__':
unittest.main()

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

@ -246,6 +246,24 @@ class TestContainerController(unittest.TestCase):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
def test_PUT_container_no_null(self):
req = Request.blank('/sda1/p/a/test\x00test',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 400)
def test_PUT_object_no_null(self):
req = Request.blank('/sda1/p/a/test',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
req = Request.blank('/sda1/p/a/test/test\x00test',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1',
'HTTP_X_SIZE': '0', 'HTTP_X_CONTENT_TYPE': 'text/plain',
'HTTP_X_ETAG': 'd41d8cd98f00b204e9800998ecf8427e'})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 400)
def test_PUT_account_update(self):
bindsock = listen(('127.0.0.1', 0))
def accept(return_code, expected_timestamp):
@ -582,25 +600,25 @@ class TestContainerController(unittest.TestCase):
resp = self.controller.PUT(req)
# fill the container
for i in range(3):
req = Request.blank('/sda1/p/a/xmlc/%s'%i, environ=
{'REQUEST_METHOD': 'PUT',
'HTTP_X_TIMESTAMP': '1',
'HTTP_X_CONTENT_TYPE': 'text/plain',
'HTTP_X_ETAG': 'x',
'HTTP_X_SIZE': 0})
req = Request.blank('/sda1/p/a/xmlc/%s%%%02x' % (i, i + 1),
environ={'REQUEST_METHOD': 'PUT',
'HTTP_X_TIMESTAMP': '1',
'HTTP_X_CONTENT_TYPE': 'text/plain',
'HTTP_X_ETAG': 'x',
'HTTP_X_SIZE': 0})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
xml_body = '<?xml version="1.0" encoding="UTF-8"?>\n' \
'<container name="xmlc">' \
'<object><name>0</name><hash>x</hash><bytes>0</bytes>' \
'<object><name>0&#x1;</name><hash>x</hash><bytes>0</bytes>' \
'<content_type>text/plain</content_type>' \
'<last_modified>1970-01-01T00:00:01' \
'</last_modified></object>' \
'<object><name>1</name><hash>x</hash><bytes>0</bytes>' \
'<object><name>1&#x2;</name><hash>x</hash><bytes>0</bytes>' \
'<content_type>text/plain</content_type>' \
'<last_modified>1970-01-01T00:00:01' \
'</last_modified></object>' \
'<object><name>2</name><hash>x</hash><bytes>0</bytes>' \
'<object><name>2&#x3;</name><hash>x</hash><bytes>0</bytes>' \
'<content_type>text/plain</content_type>' \
'<last_modified>1970-01-01T00:00:01' \
'</last_modified></object>' \
@ -822,6 +840,17 @@ class TestContainerController(unittest.TestCase):
resp = self.controller.GET(req)
self.assert_(resp.status_int in (204, 412), resp.status_int)
def test_params_no_null(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'):
req = Request.blank('/sda1/p/a/c?%s=\x00' % param,
environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 400)
if __name__ == '__main__':
unittest.main()

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

@ -2111,6 +2111,20 @@ class TestObjectController(unittest.TestCase):
exp = 'HTTP/1.1 412'
self.assertEquals(headers[:len(exp)], exp)
def test_chunked_put_bad_utf8_null(self):
# Check invalid utf-8
(prolis, acc1lis, acc2lis, con2lis, con2lis, obj1lis, obj2lis) = \
_test_sockets
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
fd = sock.makefile()
fd.write('GET /v1/a%00 HTTP/1.1\r\nHost: localhost\r\n'
'Connection: close\r\nX-Auth-Token: t\r\n'
'Content-Length: 0\r\n\r\n')
fd.flush()
headers = readuntil2crlfs(fd)
exp = 'HTTP/1.1 412'
self.assertEquals(headers[:len(exp)], exp)
def test_chunked_put_bad_path_no_controller(self):
# Check bad path, no controller
(prolis, acc1lis, acc2lis, con2lis, con2lis, obj1lis, obj2lis) = \