Enforce Content-Length in catch_errors

If a WSGI application produces the header "Content-Length: <N>" but
does not produce exactly N bytes of response, then that is an error
and an exception should be thrown so that the WSGI server can take the
correct action (close the TCP connection for HTTP <= 1.1, something
else for HTTP 2.0).

As part of this, I also fixed a bug in DLOs where a HEAD response
might have a body. The way it works is this:

 * user makes HEAD request for DLO manifest

 * DLO middleware makes GET request for container

 * authorize callback (e.g. from tempurl) replies 401 for container
   GET; response has a nonempty body (it's a GET response; that's
   fine)

 * DLO notes that response is non-2xx, returns it as-is

 * client gets response with nonempty body to a HEAD request

The fix there was simple; if the original request method was HEAD,
clear out the response body.

Change-Id: I74d8c13eba2a4917b5a116875b51a781b33a7abf
Related-Bug: 1568650
This commit is contained in:
Samuel Merritt 2018-06-15 16:31:25 -07:00
parent b08355ea3f
commit 4a0afa9fea
4 changed files with 142 additions and 1 deletions

View File

@ -16,10 +16,44 @@
from swift import gettext_ as _
from swift.common.swob import Request, HTTPServerError
from swift.common.utils import get_logger, generate_trans_id
from swift.common.utils import get_logger, generate_trans_id, close_if_possible
from swift.common.wsgi import WSGIContext
class BadResponseLength(Exception):
pass
def enforce_byte_count(inner_iter, nbytes):
"""
Enforces that inner_iter yields exactly <nbytes> bytes before
exhaustion.
If inner_iter fails to do so, BadResponseLength is raised.
:param inner_iter: iterable of bytestrings
:param nbytes: number of bytes expected
"""
try:
bytes_left = nbytes
for chunk in inner_iter:
if bytes_left >= len(chunk):
yield chunk
bytes_left -= len(chunk)
else:
yield chunk[:bytes_left]
raise BadResponseLength(
"Too many bytes; truncating after %d bytes "
"with at least %d surplus bytes remaining" % (
nbytes, len(chunk) - bytes_left))
if bytes_left:
raise BadResponseLength('Expected another %d bytes' % (
bytes_left,))
finally:
close_if_possible(inner_iter)
class CatchErrorsContext(WSGIContext):
def __init__(self, app, logger, trans_id_suffix=''):
@ -35,6 +69,7 @@ class CatchErrorsContext(WSGIContext):
trans_id = generate_trans_id(trans_id_suffix)
env['swift.trans_id'] = trans_id
method = env['REQUEST_METHOD']
self.logger.txn_id = trans_id
try:
# catch any errors in the pipeline
@ -48,6 +83,37 @@ class CatchErrorsContext(WSGIContext):
resp.headers['X-Openstack-Request-Id'] = trans_id
return resp(env, start_response)
# If the app specified a Content-Length, enforce that it sends that
# many bytes.
#
# If an app gives too few bytes, then the client will wait for the
# remainder before sending another HTTP request on the same socket;
# since no more bytes are coming, this will result in either an
# infinite wait or a timeout. In this case, we want to raise an
# exception to signal to the WSGI server that it should close the
# TCP connection.
#
# If an app gives too many bytes, then we can deadlock with the
# client; if the client reads its N bytes and then sends a large-ish
# request (enough to fill TCP buffers), it'll block until we read
# some of the request. However, we won't read the request since
# we'll be trying to shove the rest of our oversized response out
# the socket. In that case, we truncate the response body at N bytes
# and raise an exception to stop any more bytes from being
# generated and also to kill the TCP connection.
if self._response_headers:
content_lengths = [val for header, val in self._response_headers
if header.lower() == "content-length"]
if len(content_lengths) == 1:
try:
content_length = int(content_lengths[0])
except ValueError:
pass
else:
resp = enforce_byte_count(
resp,
0 if method == 'HEAD' else content_length)
# make sure the response has the trans_id
if self._response_headers is None:
self._response_headers = []

View File

@ -154,6 +154,9 @@ class GetContext(WSGIContext):
con_resp = con_req.get_response(self.dlo.app)
if not is_success(con_resp.status_int):
if req.method == 'HEAD':
close_if_possible(con_resp.app_iter)
con_resp.body = ''
return con_resp, None
with closing_if_possible(con_resp.app_iter):
return None, json.loads(''.join(con_resp.app_iter))

View File

@ -137,6 +137,63 @@ class TestCatchErrors(unittest.TestCase):
resp = app(req.environ, self.start_response)
self.assertEqual(list(resp), ['An error occurred'])
def test_HEAD_with_content_length(self):
def cannot_count_app(env, sr):
sr("200 OK", [("Content-Length", "10")])
return [b""]
app = catch_errors.CatchErrorMiddleware(cannot_count_app, {})
list(app({'REQUEST_METHOD': 'HEAD'}, self.start_response))
def test_short_response_body(self):
def cannot_count_app(env, sr):
sr("200 OK", [("Content-Length", "2000")])
return [b"our staff tailor is Euripedes Imenedes"]
app = catch_errors.CatchErrorMiddleware(cannot_count_app, {})
with self.assertRaises(catch_errors.BadResponseLength):
list(app({'REQUEST_METHOD': 'GET'}, self.start_response))
def test_long_response_body(self):
def cannot_count_app(env, sr):
sr("200 OK", [("Content-Length", "10")])
return [b"our optometric firm is C.F. Eye Care"]
app = catch_errors.CatchErrorMiddleware(cannot_count_app, {})
with self.assertRaises(catch_errors.BadResponseLength):
list(app({'REQUEST_METHOD': 'GET'}, self.start_response))
def test_bogus_content_length(self):
def bogus_cl_app(env, sr):
sr("200 OK", [("Content-Length", "25 cm")])
return [b"our British cutlery specialist is Sir Irving Spoon"]
app = catch_errors.CatchErrorMiddleware(bogus_cl_app, {})
list(app({'REQUEST_METHOD': 'GET'}, self.start_response))
def test_no_content_length(self):
def no_cl_app(env, sr):
sr("200 OK", [("Content-Type", "application/names")])
return [b"our staff statistician is Marge Inovera"]
app = catch_errors.CatchErrorMiddleware(no_cl_app, {})
list(app({'REQUEST_METHOD': 'GET'}, self.start_response))
def test_multiple_content_lengths(self):
def poly_cl_app(env, sr):
sr("200 OK", [("Content-Length", "30"),
("Content-Length", "40")])
return [b"The head of our personal trainers is Jim Shortz"]
app = catch_errors.CatchErrorMiddleware(poly_cl_app, {})
list(app({'REQUEST_METHOD': 'GET'}, self.start_response))
if __name__ == '__main__':
unittest.main()

View File

@ -612,6 +612,21 @@ class TestDloGetManifest(DloTestCase):
self.assertEqual(status, "200 OK")
self.assertEqual(body, "aaaaabbbbbccccc")
def test_error_listing_container_HEAD(self):
self.app.register(
'GET', '/v1/AUTH_test/c?prefix=seg_',
# for example, if a manifest refers to segments in another
# container, but the user is accessing the manifest via a
# container-level tempurl key
swob.HTTPUnauthorized, {}, None)
req = swob.Request.blank('/v1/AUTH_test/mancon/manifest-many-segments',
environ={'REQUEST_METHOD': 'HEAD'})
with mock.patch(LIMIT, 3):
status, headers, body = self.call_dlo(req)
self.assertEqual(status, "401 Unauthorized")
self.assertEqual(body, b"")
def test_mismatched_etag_fetching_second_segment(self):
self.app.register(
'GET', '/v1/AUTH_test/c/seg_02',