Skip to content

Commit

Permalink
Fix 500 in versioned writes with bad Destination
Browse files Browse the repository at this point in the history
When this code lived in the proxy, it was protected by an "except
HTTPException" clause in proxy.Application.handle_request(). When it
moved to middleware, it lost that, and then things like
constraints.check_name_format that raised HTTPException would cause
500s. The HTTPException would make it all the way out to catch_errors
and get translated to a 500.

This commit just wraps a couple try/excepts around the bits in
versioned writes that can raise HTTPException. I tried to make it use
wsgify so I could get that for free, but that wound up being a real
pain because env/start_response are plumbed through pretty much the
whole versioned-writes middleware.

Closes-Bug: 1483705

Change-Id: Ife165bf709e64f313ed07c779b21914045e51f25
  • Loading branch information
smerritt authored and Alistair Coles committed Aug 12, 2015
1 parent eb62f1b commit 7064706
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 11 deletions.
18 changes: 12 additions & 6 deletions swift/common/middleware/versioned_writes.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
register_swift_info, config_true_value
from swift.common.request_helpers import get_sys_meta_prefix
from swift.common.wsgi import WSGIContext, make_pre_authed_request
from swift.common.swob import Request
from swift.common.swob import Request, HTTPException
from swift.common.constraints import (
check_account_format, check_container_format, check_destination_header)
from swift.proxy.controllers.base import get_container_info
Expand Down Expand Up @@ -468,12 +468,18 @@ def __call__(self, env, start_response):
# container_info
allow_versioned_writes = self.conf.get('allow_versioned_writes')
if allow_versioned_writes and container and not obj:
return self.container_request(req, start_response,
allow_versioned_writes)
try:
return self.container_request(req, start_response,
allow_versioned_writes)
except HTTPException as error_response:
return error_response(env, start_response)
elif obj and req.method in ('PUT', 'COPY', 'DELETE'):
return self.object_request(
req, version, account, container, obj,
allow_versioned_writes)(env, start_response)
try:
return self.object_request(
req, version, account, container, obj,
allow_versioned_writes)(env, start_response)
except HTTPException as error_response:
return error_response(env, start_response)
else:
return self.app(env, start_response)

Expand Down
18 changes: 13 additions & 5 deletions test/unit/common/middleware/test_versioned_writes.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,8 @@ def test_container_allow_versioned_writes_false(self):
req = Request.blank('/v1/a/c',
headers={'X-Versions-Location': 'ver_cont'},
environ={'REQUEST_METHOD': method})
try:
status, headers, body = self.call_vw(req)
except swob.HTTPException as e:
pass
self.assertEquals(e.status_int, 412)
status, headers, body = self.call_vw(req)
self.assertEquals(status, "412 Precondition Failed")

# GET/HEAD performs as normal
self.app.register('GET', '/v1/a/c', swob.HTTPOk, {}, 'passed')
Expand Down Expand Up @@ -414,6 +411,17 @@ def test_copy_new_version_different_account(self):
self.assertEqual(len(self.authorized), 1)
self.assertRequestEqual(req, self.authorized[0])

def test_copy_new_version_bogus_account(self):
cache = FakeCache({'sysmeta': {'versions-location': 'ver_cont'}})
req = Request.blank(
'/v1/src_a/src_cont/src_obj',
environ={'REQUEST_METHOD': 'COPY', 'swift.cache': cache,
'CONTENT_LENGTH': '100'},
headers={'Destination': 'tgt_cont/tgt_obj',
'Destination-Account': '/im/on/a/boat'})
status, headers, body = self.call_vw(req)
self.assertEquals(status, '412 Precondition Failed')

def test_delete_first_object_success(self):
self.app.register(
'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
Expand Down

0 comments on commit 7064706

Please sign in to comment.