openstack/swift
Code Issues Proposed changes

Extra safety on account-level DELETE

Browse Source 
I just noticed tonight when adding a bunch of stuff to Swiftly that
the Bulk Delete middleware uses an account-level DELETE request,
albeit with a query parameter of bulk-delete. But, one typo and,
assuming the cluster supports it and you have access, whoops, you
just marked the account for deletion!

I put a bit of extra safety on the account deletion by requiring it
to have an empty query string.

Change-Id: Ib5df11193b04eff69d14185bd9d0607169131e7f
This commit is contained in:
gholt 2013-04-19 09:43:31 +00:00
parent dec517e349
commit f63dc07b9d
2 changed files with 28 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
swift/proxy/controllers/account.py
View File

@ -139,6 +139,11 @@ class AccountController(Controller):
@public
def DELETE(self, req):
"""HTTP DELETE request handler."""
# Extra safety in case someone typos a query string for an
# account-level DELETE request that was really meant to be caught by
# some middleware.
if req.query_string:
return HTTPBadRequest(request=req)
if not self.app.allow_account_management:
return HTTPMethodNotAllowed(
request=req,

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

@ -5564,6 +5564,29 @@ class TestAccountController(unittest.TestCase):
test_status_map((201, 500, 500), 503)
test_status_map((204, 500, 404), 503)
def test_DELETE_with_query_string(self):
# Extra safety in case someone typos a query string for an
# account-level DELETE request that was really meant to be caught by
# some middleware.
with save_globals():
controller = proxy_server.AccountController(self.app, 'account')
def test_status_map(statuses, expected, **kwargs):
set_http_connect(*statuses, **kwargs)
self.app.memcache.store = {}
req = Request.blank('/a?whoops', {'REQUEST_METHOD': 'DELETE'})
req.content_length = 0
self.app.update_request(req)
res = controller.DELETE(req)
expected = str(expected)
self.assertEquals(res.status[:len(expected)], expected)
test_status_map((201, 201, 201), 400)
self.app.allow_account_management = True
test_status_map((201, 201, 201), 400)
test_status_map((201, 201, 500), 400)
test_status_map((201, 500, 500), 400)
test_status_map((204, 500, 404), 400)
class FakeObjectController(object):