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)

diff --git a/cinder/api/middleware/sizelimit.py b/cinder/api/middleware/sizelimit.py
index 7133e8f949f2fba1d276aae97fd322d6516713b8..868db0f39e2068c14b9fc04b637fb0308388d05e 100644 (file)
--- 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 0a151dabf4f558a0da40cb34e06b976f08a186ff..3b87a2cd963c8e35cf63cedefaefa1f92c290239 100644 (file)
--- 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)