From d87259417c53d464dd3ec2b399faf22e65e1265c Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 19 Mar 2013 19:37:47 -0400 Subject: [PATCH] Updates to OSAPI sizelimit middleware. Updates the OSAPI sizelimit middleware so that we use avoid calling len on a request body which could cause a really large request to get buffered into memory. Also updates the middleware to return HTTP 413 which is a more correct error code in this case (previously it returned just 400). Fixes LP Bug #1131857. Change-Id: Iff6cb0c24bc21e5a1d5dd4cf29acb0b4ee221708 (cherry picked from commit 3fe81851bab39a1466d8744b372b5a287b4db93d) --- cinder/api/middleware/sizelimit.py | 41 ++++++++++-- cinder/tests/api/middleware/test_sizelimit.py | 64 ++++++++++++++++--- 2 files changed, 92 insertions(+), 13 deletions(-) diff --git a/cinder/api/middleware/sizelimit.py b/cinder/api/middleware/sizelimit.py index 7133e8f94..868db0f39 100644 --- a/cinder/api/middleware/sizelimit.py +++ b/cinder/api/middleware/sizelimit.py @@ -36,6 +36,35 @@ FLAGS.register_opt(max_request_body_size_opt) LOG = logging.getLogger(__name__) +class LimitingReader(object): + """Reader to limit the size of an incoming request.""" + def __init__(self, data, limit): + """ + :param data: Underlying data object + :param limit: maximum number of bytes the reader should allow + """ + self.data = data + self.limit = limit + self.bytes_read = 0 + + def __iter__(self): + for chunk in self.data: + self.bytes_read += len(chunk) + if self.bytes_read > self.limit: + msg = _("Request is too large.") + raise webob.exc.HTTPRequestEntityTooLarge(explanation=msg) + else: + yield chunk + + def read(self, i=None): + result = self.data.read(i) + self.bytes_read += len(result) + if self.bytes_read > self.limit: + msg = _("Request is too large.") + raise webob.exc.HTTPRequestEntityTooLarge(explanation=msg) + return result + + class RequestBodySizeLimiter(wsgi.Middleware): """Add a 'cinder.context' to WSGI environ.""" @@ -44,9 +73,11 @@ class RequestBodySizeLimiter(wsgi.Middleware): @webob.dec.wsgify(RequestClass=wsgi.Request) def __call__(self, req): - if (req.content_length > FLAGS.osapi_max_request_body_size - or len(req.body) > FLAGS.osapi_max_request_body_size): + if req.content_length > FLAGS.osapi_max_request_body_size: msg = _("Request is too large.") - raise webob.exc.HTTPBadRequest(explanation=msg) - else: - return self.application + raise webob.exc.HTTPRequestEntityTooLarge(explanation=msg) + if req.content_length is None and req.is_body_readable: + limiter = LimitingReader(req.body_file, + FLAGS.osapi_max_request_body_size) + req.body_file = limiter + return self.application diff --git a/cinder/tests/api/middleware/test_sizelimit.py b/cinder/tests/api/middleware/test_sizelimit.py index 0a151dabf..3b87a2cd9 100644 --- a/cinder/tests/api/middleware/test_sizelimit.py +++ b/cinder/tests/api/middleware/test_sizelimit.py @@ -12,9 +12,10 @@ # License for the specific language governing permissions and limitations # under the License. +import StringIO import webob -import cinder.api.middleware.sizelimit +from cinder.api.middleware import sizelimit from cinder import flags from cinder import test @@ -22,6 +23,52 @@ FLAGS = flags.FLAGS MAX_REQUEST_BODY_SIZE = FLAGS.osapi_max_request_body_size +class TestLimitingReader(test.TestCase): + + def test_limiting_reader(self): + BYTES = 1024 + bytes_read = 0 + data = StringIO.StringIO("*" * BYTES) + for chunk in sizelimit.LimitingReader(data, BYTES): + bytes_read += len(chunk) + + self.assertEquals(bytes_read, BYTES) + + bytes_read = 0 + data = StringIO.StringIO("*" * BYTES) + reader = sizelimit.LimitingReader(data, BYTES) + byte = reader.read(1) + while len(byte) != 0: + bytes_read += 1 + byte = reader.read(1) + + self.assertEquals(bytes_read, BYTES) + + def test_limiting_reader_fails(self): + BYTES = 1024 + + def _consume_all_iter(): + bytes_read = 0 + data = StringIO.StringIO("*" * BYTES) + for chunk in sizelimit.LimitingReader(data, BYTES - 1): + bytes_read += len(chunk) + + self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, + _consume_all_iter) + + def _consume_all_read(): + bytes_read = 0 + data = StringIO.StringIO("*" * BYTES) + reader = sizelimit.LimitingReader(data, BYTES - 1) + byte = reader.read(1) + while len(byte) != 0: + bytes_read += 1 + byte = reader.read(1) + + self.assertRaises(webob.exc.HTTPRequestEntityTooLarge, + _consume_all_read) + + class TestRequestBodySizeLimiter(test.TestCase): def setUp(self): @@ -29,10 +76,9 @@ class TestRequestBodySizeLimiter(test.TestCase): @webob.dec.wsgify() def fake_app(req): - return webob.Response() + return webob.Response(req.body) - self.middleware = (cinder.api.middleware.sizelimit - .RequestBodySizeLimiter(fake_app)) + self.middleware = sizelimit.RequestBodySizeLimiter(fake_app) self.request = webob.Request.blank('/', method='POST') def test_content_length_acceptable(self): @@ -41,12 +87,14 @@ class TestRequestBodySizeLimiter(test.TestCase): response = self.request.get_response(self.middleware) self.assertEqual(response.status_int, 200) - def test_content_length_to_large(self): + def test_content_length_too_large(self): self.request.headers['Content-Length'] = MAX_REQUEST_BODY_SIZE + 1 + self.request.body = "0" * (MAX_REQUEST_BODY_SIZE + 1) response = self.request.get_response(self.middleware) - self.assertEqual(response.status_int, 400) + self.assertEqual(response.status_int, 413) - def test_request_to_large(self): + def test_request_too_large_no_content_length(self): self.request.body = "0" * (MAX_REQUEST_BODY_SIZE + 1) + self.request.headers['Content-Length'] = None response = self.request.get_response(self.middleware) - self.assertEqual(response.status_int, 400) + self.assertEqual(response.status_int, 413) -- 2.45.2