Skip to content

Commit

Permalink
Better escaping for GET /v1/a?format=xml.
Browse files Browse the repository at this point in the history
Commit 8f9b135 fixed a bug where an XML attribute could have arbitrary
characters jammed into it, resulting in a document with arbitrary
tags... and it did remove the ability to get an arbitrary XML document
out of the object server. However, it left a couple of ways to get a
malformed XML document, one example of which is an account named ".

This fixes up the remaining ways and ensures you always get a
well-formed XML document in the account-listing response. Also, it
adds a unit test for the escaping of the container name; this was
already working, just untested.

If you look in the discussion for bug 1183884, you'll see that the
review comments there are basically "seems good, but could use a unit
test". (The astute reader will note that I am one of the guilty
parties in that review.)

I found this bug while writing the missing unit test.

The moral of this story is left as an exercise for the reader.

Fixes bug 1183884 harder.

Change-Id: I84b24dd930ba1bb6c4f674f8d3996639dedbce15
  • Loading branch information
smerritt committed Jun 14, 2013
1 parent 7757144 commit 92d7ead
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
2 changes: 1 addition & 1 deletion swift/account/utils.py
Expand Up @@ -96,7 +96,7 @@ def account_listing_response(account, req, response_content_type, broker=None,
account_list = json.dumps(data)
elif response_content_type.endswith('/xml'):
output_list = ['<?xml version="1.0" encoding="UTF-8"?>',
'<account name="%s">' % saxutils.escape(account)]
'<account name=%s>' % saxutils.quoteattr(account)]
for (name, object_count, bytes_used, is_subdir) in account_list:
name = saxutils.escape(name)
if is_subdir:
Expand Down
37 changes: 37 additions & 0 deletions test/unit/account/test_server.py
Expand Up @@ -632,6 +632,43 @@ def test_GET_with_containers_xml(self):
self.assertEquals(node.firstChild.nodeValue, '4')
self.assertEquals(resp.charset, 'utf-8')

def test_GET_xml_escapes_account_name(self):
req = Request.blank(
'/sda1/p/%22%27', # "'
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'})
self.controller.PUT(req)

req = Request.blank(
'/sda1/p/%22%27?format=xml',
environ={'REQUEST_METHOD': 'GET', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.GET(req)

dom = xml.dom.minidom.parseString(resp.body)
self.assertEquals(dom.firstChild.attributes['name'].value, '"\'')

def test_GET_xml_escapes_container_name(self):
req = Request.blank(
'/sda1/p/a',
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '0'})
self.controller.PUT(req)

req = Request.blank(
'/sda1/p/a/%22%3Cword', # "<word
environ={'REQUEST_METHOD': 'PUT', 'HTTP_X_TIMESTAMP': '1',
'HTTP_X_PUT_TIMESTAMP': '1', 'HTTP_X_OBJECT_COUNT': '0',
'HTTP_X_DELETE_TIMESTAMP': '0', 'HTTP_X_BYTES_USED': '1'})
self.controller.PUT(req)

req = Request.blank(
'/sda1/p/a?format=xml',
environ={'REQUEST_METHOD': 'GET', 'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.GET(req)
dom = xml.dom.minidom.parseString(resp.body)

self.assertEquals(
dom.firstChild.firstChild.nextSibling.firstChild.firstChild.data,
'"<word')

def test_GET_limit_marker_plain(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
'HTTP_X_TIMESTAMP': '0'})
Expand Down

0 comments on commit 92d7ead

Please sign in to comment.