Fix container quota MW for handling a bad source path
The copy source must be container/object. This patch avoids the server to return an internal server error when user provides a path without a container. Fixes: bug #1255049 Change-Id: I1a85c98d9b3a78bad40b8ceba9088cf323042412
This commit is contained in:
parent
8b40f199eb
commit
8e1e67c02d
swift
test/unit
@ -15,11 +15,12 @@
|
||||
|
||||
import os
|
||||
import urllib
|
||||
from urllib import unquote
|
||||
from ConfigParser import ConfigParser, NoSectionError, NoOptionError
|
||||
|
||||
from swift.common.utils import ismount
|
||||
from swift.common.utils import ismount, split_path
|
||||
from swift.common.swob import HTTPBadRequest, HTTPLengthRequired, \
|
||||
HTTPRequestEntityTooLarge
|
||||
HTTPRequestEntityTooLarge, HTTPPreconditionFailed
|
||||
|
||||
constraints_conf = ConfigParser()
|
||||
constraints_conf.read('/etc/swift/swift.conf')
|
||||
@ -211,3 +212,26 @@ def check_utf8(string):
|
||||
# So, we should catch both UnicodeDecodeError & UnicodeEncodeError
|
||||
except UnicodeError:
|
||||
return False
|
||||
|
||||
|
||||
def check_copy_from_header(req):
|
||||
"""
|
||||
Validate that the value from x-copy-from header is
|
||||
well formatted. We assume the caller ensures that
|
||||
x-copy-from header is present in req.headers.
|
||||
|
||||
:param req: HTTP request object
|
||||
:returns: A tuple with container name and object name
|
||||
:raise: HTTPPreconditionFailed if x-copy-from value
|
||||
is not well formatted.
|
||||
"""
|
||||
src_header = unquote(req.headers.get('X-Copy-From'))
|
||||
if not src_header.startswith('/'):
|
||||
src_header = '/' + src_header
|
||||
try:
|
||||
return split_path(src_header, 2, 2, True)
|
||||
except ValueError:
|
||||
raise HTTPPreconditionFailed(
|
||||
request=req,
|
||||
body='X-Copy-From header must be of the form'
|
||||
'<container name>/<object name>')
|
||||
|
@ -41,7 +41,7 @@ set:
|
||||
| | container. |
|
||||
+---------------------------------------------+-------------------------------+
|
||||
"""
|
||||
|
||||
from swift.common.constraints import check_copy_from_header
|
||||
from swift.common.http import is_success
|
||||
from swift.common.swob import Response, HTTPBadRequest, wsgify
|
||||
from swift.common.utils import register_swift_info
|
||||
@ -90,10 +90,10 @@ class ContainerQuotaMiddleware(object):
|
||||
'bytes' in container_info and \
|
||||
container_info['meta']['quota-bytes'].isdigit():
|
||||
content_length = (req.content_length or 0)
|
||||
copy_from = req.headers.get('X-Copy-From')
|
||||
if copy_from:
|
||||
path = '/%s/%s/%s' % (version, account,
|
||||
copy_from.lstrip('/'))
|
||||
if 'x-copy-from' in req.headers:
|
||||
src_cont, src_obj = check_copy_from_header(req)
|
||||
path = '/%s/%s/%s/%s' % (version, account,
|
||||
src_cont, src_obj)
|
||||
object_info = get_object_info(req.environ, self.app, path)
|
||||
if not object_info or not object_info['length']:
|
||||
content_length = 0
|
||||
|
@ -45,7 +45,7 @@ from swift.common.utils import ContextPool, normalize_timestamp, \
|
||||
get_valid_utf8_str, GreenAsyncPile
|
||||
from swift.common.bufferedhttp import http_connect
|
||||
from swift.common.constraints import check_metadata, check_object_creation, \
|
||||
CONTAINER_LISTING_LIMIT, MAX_FILE_SIZE
|
||||
CONTAINER_LISTING_LIMIT, MAX_FILE_SIZE, check_copy_from_header
|
||||
from swift.common.exceptions import ChunkReadTimeout, \
|
||||
ChunkWriteTimeout, ConnectionTimeout, ListingIterNotFound, \
|
||||
ListingIterNotAuthorized, ListingIterError, SegmentError
|
||||
@ -992,21 +992,12 @@ class ObjectController(Controller):
|
||||
if req.environ.get('swift.orig_req_method', req.method) != 'POST':
|
||||
req.environ.setdefault('swift.log_info', []).append(
|
||||
'x-copy-from:%s' % source_header)
|
||||
source_header = unquote(source_header)
|
||||
acct = req.swift_entity_path.split('/', 2)[1]
|
||||
src_container_name, src_obj_name = check_copy_from_header(req)
|
||||
ver, acct, _rest = req.split_path(2, 3, True)
|
||||
if isinstance(acct, unicode):
|
||||
acct = acct.encode('utf-8')
|
||||
if not source_header.startswith('/'):
|
||||
source_header = '/' + source_header
|
||||
source_header = '/v1/' + acct + source_header
|
||||
try:
|
||||
src_container_name, src_obj_name = \
|
||||
source_header.split('/', 4)[3:]
|
||||
except ValueError:
|
||||
return HTTPPreconditionFailed(
|
||||
request=req,
|
||||
body='X-Copy-From header must be of the form'
|
||||
'<container name>/<object name>')
|
||||
source_header = '/%s/%s/%s/%s' % (ver, acct,
|
||||
src_container_name, src_obj_name)
|
||||
source_req = req.copy_get()
|
||||
source_req.path_info = source_header
|
||||
source_req.headers['X-Newest'] = 'true'
|
||||
|
@ -138,6 +138,16 @@ class TestContainerQuotas(unittest.TestCase):
|
||||
res = req.get_response(app)
|
||||
self.assertEquals(res.status_int, 200)
|
||||
|
||||
def test_bytes_quota_copy_from_bad_src(self):
|
||||
app = container_quotas.ContainerQuotaMiddleware(FakeApp(), {})
|
||||
cache = FakeCache({'bytes': 0, 'meta': {'quota-bytes': '100'}})
|
||||
req = Request.blank('/v1/a/c/o',
|
||||
environ={'REQUEST_METHOD': 'PUT',
|
||||
'swift.cache': cache},
|
||||
headers={'x-copy-from': 'bad_path'})
|
||||
res = req.get_response(app)
|
||||
self.assertEquals(res.status_int, 412)
|
||||
|
||||
def test_exceed_counts_quota(self):
|
||||
app = container_quotas.ContainerQuotaMiddleware(FakeApp(), {})
|
||||
cache = FakeCache({'object_count': 1, 'meta': {'quota-count': '1'}})
|
||||
|
@ -19,7 +19,7 @@ import mock
|
||||
from test import safe_repr
|
||||
from test.unit import MockTrue
|
||||
|
||||
from swift.common.swob import HTTPBadRequest, Request
|
||||
from swift.common.swob import HTTPBadRequest, Request, HTTPException
|
||||
from swift.common.http import HTTP_REQUEST_ENTITY_TOO_LARGE, \
|
||||
HTTP_BAD_REQUEST, HTTP_LENGTH_REQUIRED
|
||||
from swift.common import constraints
|
||||
@ -257,6 +257,33 @@ class TestConstraints(unittest.TestCase):
|
||||
self.assertTrue(c.MAX_HEADER_SIZE > c.MAX_META_NAME_LENGTH)
|
||||
self.assertTrue(c.MAX_HEADER_SIZE > c.MAX_META_VALUE_LENGTH)
|
||||
|
||||
def test_validate_copy_from(self):
|
||||
req = Request.blank(
|
||||
'/v/a/c/o',
|
||||
headers={'x-copy-from': 'c/o2'})
|
||||
src_cont, src_obj = constraints.check_copy_from_header(req)
|
||||
self.assertEqual(src_cont, 'c')
|
||||
self.assertEqual(src_obj, 'o2')
|
||||
req = Request.blank(
|
||||
'/v/a/c/o',
|
||||
headers={'x-copy-from': 'c/subdir/o2'})
|
||||
src_cont, src_obj = constraints.check_copy_from_header(req)
|
||||
self.assertEqual(src_cont, 'c')
|
||||
self.assertEqual(src_obj, 'subdir/o2')
|
||||
req = Request.blank(
|
||||
'/v/a/c/o',
|
||||
headers={'x-copy-from': '/c/o2'})
|
||||
src_cont, src_obj = constraints.check_copy_from_header(req)
|
||||
self.assertEqual(src_cont, 'c')
|
||||
self.assertEqual(src_obj, 'o2')
|
||||
|
||||
def test_validate_bad_copy_from(self):
|
||||
req = Request.blank(
|
||||
'/v/a/c/o',
|
||||
headers={'x-copy-from': 'bad_object'})
|
||||
self.assertRaises(HTTPException,
|
||||
constraints.check_copy_from_header, req)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
@ -21,6 +21,7 @@ import mock
|
||||
|
||||
import swift
|
||||
from swift.proxy import server as proxy_server
|
||||
from swift.common.swob import HTTPException
|
||||
from test.unit import FakeRing, FakeMemcache, fake_http_connect
|
||||
|
||||
|
||||
@ -107,14 +108,20 @@ class TestObjController(unittest.TestCase):
|
||||
# and now test that we add the header to log_info
|
||||
req = swift.common.swob.Request.blank('/v1/a/c/o')
|
||||
req.headers['x-copy-from'] = 'somewhere'
|
||||
controller.PUT(req)
|
||||
try:
|
||||
controller.PUT(req)
|
||||
except HTTPException:
|
||||
pass
|
||||
self.assertEquals(
|
||||
req.environ.get('swift.log_info'), ['x-copy-from:somewhere'])
|
||||
# and then check that we don't do that for originating POSTs
|
||||
req = swift.common.swob.Request.blank('/v1/a/c/o')
|
||||
req.method = 'POST'
|
||||
req.headers['x-copy-from'] = 'elsewhere'
|
||||
controller.PUT(req)
|
||||
try:
|
||||
controller.PUT(req)
|
||||
except HTTPException:
|
||||
pass
|
||||
self.assertEquals(req.environ.get('swift.log_info'), None)
|
||||
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user