openstack
/
swift
Code Issues Proposed changes

Raise error on long or short DLO 

The GET response for a DLO has a Content-Length that's computed from
the container listing, but the response body's length is determined by
the segments. If a segment grows or shrinks between when the headers
are sent and when the segment is requested, this confuses clients.

For example, if the DLO is longer than its Content-Length and the
client sends another request on the same TCP connection, then it can
get leftover object bytes instead of an HTTP status line.
Alternately, if the headers it sends fill up the TCP buffers since
Swift won't read them until the first response is done, then deadlock
hilarity ensues.

If the DLO is shorter than its Content-Length, you're pretty much
guaranteed a deadlock: the client is waiting for the rest of the
response, and the server is waiting for a new request.

Now SegmentedIterable detects both these conditions and raises an
exception so that the TCP connection gets torn down. It can't save
this request, but it can stop the next one from getting hosed too.

Change-Id: Icf79ba046ef7aaaab49ce6d0b33147332c967afc
This commit is contained in:
Samuel Merritt 2014-02-20 20:33:00 -08:00
parent 580b1a8ab3
commit 25ebf3aa9e
2 changed files with 103 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
swift/common/request_helpers.py
View File

@ -239,7 +239,7 @@ class SegmentedIterable(object):
:param ua_suffix: string to append to user-agent.
:param name: name of manifest (used in logging only)
:param response: optional response object for the response being sent
to the client. Only affects logs.
to the client.
"""
def __init__(self, req, app, listing_iter, max_get_time,
logger, ua_suffix, swift_source,
@ -267,6 +267,12 @@ class SegmentedIterable(object):
def __iter__(self):
start_time = time.time()
have_yielded_data = False
if self.response and self.response.content_length:
bytes_left = int(self.response.content_length)
else:
bytes_left = None
try:
for seg_path, seg_etag, seg_size, first_byte, last_byte \
in self.listing_iter:
@ -317,9 +323,28 @@ class SegmentedIterable(object):
's_size': seg_size})
for chunk in seg_resp.app_iter:
yield chunk
have_yielded_data = True
if bytes_left is None:
yield chunk
elif bytes_left >= len(chunk):
yield chunk
bytes_left -= len(chunk)
else:
yield chunk[:bytes_left]
bytes_left -= len(chunk)
close_if_possible(seg_resp.app_iter)
raise SegmentError(
'Too many bytes for %(name)s; truncating in '
'%(seg)s with %(left)d bytes left' %
{'name': self.name, 'seg': seg_req.path,
'left': bytes_left})
close_if_possible(seg_resp.app_iter)
if bytes_left:
raise SegmentError(
'Not enough bytes for %s; closing connection' %
self.name)
except ListingIterError as err:
# I have to save this error because yielding the ' ' below clears
# the exception from the current stack frame.

76
test/unit/common/middleware/test_dlo.py
View File

@ -682,6 +682,82 @@ class TestDloGetManifest(DloTestCase):
self.assertEqual(body, 'aaaaabbbbbccccc')
self.assertTrue(isinstance(exc, exceptions.SegmentError))
def test_get_oversize_segment(self):
# If we send a Content-Length header to the client, it's based on the
# container listing. If a segment gets bigger by the time we get to it
# (like if a client uploads a bigger segment w/the same name), we need
# to not send anything beyond the length we promised. Also, we should
# probably raise an exception.
# This is now longer than the original seg_03+seg_04+seg_05 combined
self.app.register(
'GET', '/v1/AUTH_test/c/seg_03',
swob.HTTPOk, {'Content-Length': '20', 'Etag': 'seg03-etag'},
'cccccccccccccccccccc')
req = swob.Request.blank(
'/v1/AUTH_test/mancon/manifest',
environ={'REQUEST_METHOD': 'GET'})
status, headers, body, exc = self.call_dlo(req, expect_exception=True)
headers = swob.HeaderKeyDict(headers)
self.assertEqual(status, '200 OK') # sanity check
self.assertEqual(headers.get('Content-Length'), '25') # sanity check
self.assertEqual(body, 'aaaaabbbbbccccccccccccccc')
self.assertTrue(isinstance(exc, exceptions.SegmentError))
self.assertEqual(
self.app.calls,
[('GET', '/v1/AUTH_test/mancon/manifest'),
('GET', '/v1/AUTH_test/c?format=json&prefix=seg'),
('GET', '/v1/AUTH_test/c/seg_01'),
('GET', '/v1/AUTH_test/c/seg_02'),
('GET', '/v1/AUTH_test/c/seg_03')])
def test_get_undersize_segment(self):
# If we send a Content-Length header to the client, it's based on the
# container listing. If a segment gets smaller by the time we get to
# it (like if a client uploads a smaller segment w/the same name), we
# need to raise an exception so that the connection will be closed by
# the WSGI server. Otherwise, the WSGI server will be waiting for the
# next request, the client will still be waiting for the rest of the
# response, and nobody will be happy.
# Shrink it by a single byte
self.app.register(
'GET', '/v1/AUTH_test/c/seg_03',
swob.HTTPOk, {'Content-Length': '4', 'Etag': 'seg03-etag'},
'cccc')
req = swob.Request.blank(
'/v1/AUTH_test/mancon/manifest',
environ={'REQUEST_METHOD': 'GET'})
status, headers, body, exc = self.call_dlo(req, expect_exception=True)
headers = swob.HeaderKeyDict(headers)
self.assertEqual(status, '200 OK') # sanity check
self.assertEqual(headers.get('Content-Length'), '25') # sanity check
self.assertEqual(body, 'aaaaabbbbbccccdddddeeeee')
self.assertTrue(isinstance(exc, exceptions.SegmentError))
def test_get_undersize_segment_range(self):
# Shrink it by a single byte
self.app.register(
'GET', '/v1/AUTH_test/c/seg_03',
swob.HTTPOk, {'Content-Length': '4', 'Etag': 'seg03-etag'},
'cccc')
req = swob.Request.blank(
'/v1/AUTH_test/mancon/manifest',
environ={'REQUEST_METHOD': 'GET'},
headers={'Range': 'bytes=0-14'})
status, headers, body, exc = self.call_dlo(req, expect_exception=True)
headers = swob.HeaderKeyDict(headers)
self.assertEqual(status, '206 Partial Content') # sanity check
self.assertEqual(headers.get('Content-Length'), '15') # sanity check
self.assertEqual(body, 'aaaaabbbbbcccc')
self.assertTrue(isinstance(exc, exceptions.SegmentError))
def fake_start_response(*args, **kwargs):
pass