rolling back xml changes bzr merge -r319..318

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:
+        except UnicodeDecodeError, err:
-            return HTTPBadRequest(body='parameters not utf8 or contain NULLs',
+            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 = ['<?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, XML_EXTRA_ENTITIES)
+                name = saxutils.escape(name)
                 if is_subdir:
                     output_list.append('<subdir name="%s" />' % name)
                 else:

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,
+    :returns: True if the string is valid utf-8, False otherwise
-              False otherwise
     """
     try:
         string.decode('UTF-8')
+        return True
     except UnicodeDecodeError:
         return False
-    if '\x00' in string:
-        return False
-    return True

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):
+    if value:
-        raise ValueError('Not valid UTF-8 or contains NULL characters')
+        value.decode('utf8')    # Ensure UTF8ness
     return value
 
 

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('<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>' % 
+                '<container name=%s>' % saxutils.quoteattr(container),
-                    saxutils.quoteattr(container, XML_EXTRA_ENTITIES),
                 ''.join(xml_output), '</container>'])
         else:
             if not container_list:

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('<name>%s&#x1;test</name>' % (container,) in body)
-
 
 if __name__ == '__main__':
     unittest.main()

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('<name>object&#x1;test</name>' in body)
-
 
 if __name__ == '__main__':
     unittest.main()

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()

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',
+        req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'},
-                            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)
-            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'))
         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',
+        req = Request.blank('/sda1/p/a/c2', environ={'REQUEST_METHOD': 'PUT'},
-                            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)
-            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'))
         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()

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),
+            req = Request.blank('/sda1/p/a/xmlc/%s'%i, environ=
-                environ={'REQUEST_METHOD': 'PUT',
+                    {'REQUEST_METHOD': 'PUT',
-                         'HTTP_X_TIMESTAMP': '1',
+                    'HTTP_X_TIMESTAMP': '1',
-                         'HTTP_X_CONTENT_TYPE': 'text/plain',
+                    'HTTP_X_CONTENT_TYPE': 'text/plain',
-                         'HTTP_X_ETAG': 'x',
+                    'HTTP_X_ETAG': 'x',
-                         'HTTP_X_SIZE': 0})
+                    '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&#x1;</name><hash>x</hash><bytes>0</bytes>' \
+                '<object><name>0</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&#x2;</name><hash>x</hash><bytes>0</bytes>' \
+                '<object><name>1</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&#x3;</name><hash>x</hash><bytes>0</bytes>' \
+                '<object><name>2</name><hash>x</hash><bytes>0</bytes>' \
                     '<content_type>text/plain</content_type>' \
                     '<last_modified>1970-01-01T00:00:01' \
                     '</last_modified></object>' \
@@ -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()

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) = \