diff --git a/swift/account/server.py b/swift/account/server.py index ee55c0556b..fd839a4909 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -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, XML_EXTRA_ENTITIES + normalize_timestamp, split_path, storage_directory from swift.common.constraints import ACCOUNT_LISTING_LIMIT, \ check_mount, check_float, check_utf8 from swift.common.db_replicator import ReplicatorRpc @@ -79,9 +79,6 @@ 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) @@ -204,8 +201,8 @@ class AccountController(object): marker = get_param(req, 'marker', '') end_marker = get_param(req, 'end_marker') query_format = get_param(req, 'format') - except (UnicodeDecodeError, ValueError), err: - return HTTPBadRequest(body='parameters not utf8 or contain NULLs', + except UnicodeDecodeError, err: + return HTTPBadRequest(body='parameters not utf8', content_type='text/plain', request=req) if query_format: req.accept = 'application/%s' % query_format.lower() @@ -231,7 +228,7 @@ class AccountController(object): output_list = ['', '' % account] for (name, object_count, bytes_used, is_subdir) in account_list: - name = saxutils.escape(name, XML_EXTRA_ENTITIES) + name = saxutils.escape(name) if is_subdir: output_list.append('' % name) else: diff --git a/swift/common/constraints.py b/swift/common/constraints.py index 1f314ef0bb..ad5a37b9a8 100644 --- a/swift/common/constraints.py +++ b/swift/common/constraints.py @@ -159,16 +159,13 @@ def check_float(string): def check_utf8(string): """ - Validate if a string is valid UTF-8 and has no NULL characters. + Validate if a string is valid UTF-8. :param string: string to be validated - :returns: True if the string is valid utf-8 and has no NULL characters, - False otherwise + :returns: True if the string is valid utf-8, False otherwise """ try: string.decode('UTF-8') + return True except UnicodeDecodeError: return False - if '\x00' in string: - return False - return True diff --git a/swift/common/utils.py b/swift/common/utils.py index 2629849775..f95fa4aa96 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -42,7 +42,6 @@ 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 @@ -75,8 +74,6 @@ 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, 0x20)) def validate_configuration(): if HASH_PATH_SUFFIX == '': @@ -113,8 +110,8 @@ def get_param(req, name, default=None): :returns: HTTP request parameter value """ value = req.str_params.get(name, default) - if value and not check_utf8(value): - raise ValueError('Not valid UTF-8 or contains NULL characters') + if value: + value.decode('utf8') # Ensure UTF8ness return value diff --git a/swift/container/server.py b/swift/container/server.py index 9a7d32399d..bc3856d18e 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -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, XML_EXTRA_ENTITIES + normalize_timestamp, storage_directory, split_path from swift.common.constraints import CONTAINER_LISTING_LIMIT, \ check_mount, check_float, check_utf8 from swift.common.bufferedhttp import http_connect @@ -167,10 +167,6 @@ 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) @@ -281,7 +277,7 @@ class ContainerController(object): return HTTPPreconditionFailed(request=req, body='Maximum limit is %d' % CONTAINER_LISTING_LIMIT) query_format = get_param(req, 'format') - except (UnicodeDecodeError, ValueError), err: + except UnicodeDecodeError, err: return HTTPBadRequest(body='parameters not utf8', content_type='text/plain', request=req) if query_format: @@ -316,23 +312,21 @@ 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, XML_EXTRA_ENTITIES) + name = saxutils.escape(name) created_at = datetime.utcfromtimestamp( float(created_at)).isoformat() if content_type is None: xml_output.append('%s' '' % (name, name)) else: - content_type = saxutils.escape(content_type, - XML_EXTRA_ENTITIES) + content_type = saxutils.escape(content_type) xml_output.append('%s%s'\ '%d%s'\ '%s' % \ (name, etag, size, content_type, created_at)) container_list = ''.join([ '\n', - '' % - saxutils.quoteattr(container, XML_EXTRA_ENTITIES), + '' % saxutils.quoteattr(container), ''.join(xml_output), '']) else: if not container_list: diff --git a/test/functionalnosetests/test_account.py b/test/functionalnosetests/test_account.py index 02f48e62ba..4b5da32da1 100755 --- a/test/functionalnosetests/test_account.py +++ b/test/functionalnosetests/test_account.py @@ -2,7 +2,6 @@ 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 @@ -133,32 +132,6 @@ 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('%stest' % (container,) in body) - if __name__ == '__main__': unittest.main() diff --git a/test/functionalnosetests/test_container.py b/test/functionalnosetests/test_container.py index a89b224899..738231e02b 100755 --- a/test/functionalnosetests/test_container.py +++ b/test/functionalnosetests/test_container.py @@ -522,50 +522,6 @@ 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('objecttest' in body) - if __name__ == '__main__': unittest.main() diff --git a/test/functionalnosetests/test_object.py b/test/functionalnosetests/test_object.py index 73bfae2f5e..5975cf16a2 100644 --- a/test/functionalnosetests/test_object.py +++ b/test/functionalnosetests/test_object.py @@ -541,30 +541,6 @@ 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() diff --git a/test/unit/account/test_server.py b/test/unit/account/test_server.py index cbc89395f1..238b7f3d18 100644 --- a/test/unit/account/test_server.py +++ b/test/unit/account/test_server.py @@ -22,7 +22,6 @@ 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 @@ -451,8 +450,7 @@ class TestAccountController(unittest.TestCase): 'X-Bytes-Used': '0', 'X-Timestamp': normalize_timestamp(0)}) self.controller.PUT(req) - req = Request.blank('/sda1/p/a/c2%04', - environ={'REQUEST_METHOD': 'PUT'}, + req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'}, headers={'X-Put-Timestamp': '2', 'X-Delete-Timestamp': '0', 'X-Object-Count': '0', @@ -464,15 +462,7 @@ class TestAccountController(unittest.TestCase): resp = self.controller.GET(req) self.assertEquals(resp.content_type, 'application/xml') self.assertEquals(resp.status_int, 200) - 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('', '\\x04')) + dom = xml.dom.minidom.parseString(resp.body) self.assertEquals(dom.firstChild.nodeName, 'account') listing = \ [n for n in dom.firstChild.childNodes if n.nodeName != '#text'] @@ -493,7 +483,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\\x04') + self.assertEquals(node.firstChild.nodeValue, 'c2') 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] @@ -505,8 +495,7 @@ class TestAccountController(unittest.TestCase): 'X-Bytes-Used': '2', 'X-Timestamp': normalize_timestamp(0)}) self.controller.PUT(req) - req = Request.blank('/sda1/p/a/c2%04', - environ={'REQUEST_METHOD': 'PUT'}, + req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'}, headers={'X-Put-Timestamp': '2', 'X-Delete-Timestamp': '0', 'X-Object-Count': '3', @@ -517,15 +506,7 @@ class TestAccountController(unittest.TestCase): environ={'REQUEST_METHOD': 'GET'}) resp = self.controller.GET(req) self.assertEquals(resp.status_int, 200) - 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('', '\\x04')) + dom = xml.dom.minidom.parseString(resp.body) self.assertEquals(dom.firstChild.nodeName, 'account') listing = \ [n for n in dom.firstChild.childNodes if n.nodeName != '#text'] @@ -545,7 +526,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\\x04') + self.assertEquals(node.firstChild.nodeValue, 'c2') 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] @@ -978,35 +959,6 @@ 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() diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index 7ef0cee5c3..117a0dccd5 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -246,24 +246,6 @@ 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): @@ -600,25 +582,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%%%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}) + 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}) resp = self.controller.PUT(req) self.assertEquals(resp.status_int, 201) xml_body = '\n' \ '' \ - '0x0' \ + '0x0' \ 'text/plain' \ '1970-01-01T00:00:01' \ '' \ - '1x0' \ + '1x0' \ 'text/plain' \ '1970-01-01T00:00:01' \ '' \ - '2x0' \ + '2x0' \ 'text/plain' \ '1970-01-01T00:00:01' \ '' \ @@ -840,17 +822,6 @@ 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() diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index f7b1edd328..b0a2def4b0 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -2111,20 +2111,6 @@ 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) = \