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: Id8bc5eeb0fba9482809edd12543a75163e1227e9

2 files changed, 90 insertions, 10 deletions

diff --git a/nova/api/sizelimit.py b/nova/api/sizelimit.py
index 67d459583..1e88f183e 100644
--- a/nova/api/sizelimit.py
+++ b/nova/api/sizelimit.py
@@ -37,6 +37,35 @@ CONF.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):
     """Limit the size of incoming requests."""
 
@@ -45,9 +74,11 @@ class RequestBodySizeLimiter(wsgi.Middleware):
 
     @webob.dec.wsgify(RequestClass=wsgi.Request)
     def __call__(self, req):
-        if (req.content_length > CONF.osapi_max_request_body_size
-            or len(req.body) > CONF.osapi_max_request_body_size):
+        if req.content_length > CONF.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,
+                                     CONF.osapi_max_request_body_size)
+            req.body_file = limiter
+        return self.application
diff --git a/nova/tests/api/test_sizelimit.py b/nova/tests/api/test_sizelimit.py
index 862a0d65f..9e7a33d29 100644
--- a/nova/tests/api/test_sizelimit.py
+++ b/nova/tests/api/test_sizelimit.py
@@ -13,6 +13,7 @@
 #    under the License.
 
 from oslo.config import cfg
+import StringIO
 import webob
 
 import nova.api.sizelimit
@@ -22,6 +23,52 @@ CONF = cfg.CONF
 MAX_REQUEST_BODY_SIZE = CONF.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 nova.api.sizelimit.LimitingReader(data, BYTES):
+            bytes_read += len(chunk)
+
+        self.assertEquals(bytes_read, BYTES)
+
+        bytes_read = 0
+        data = StringIO.StringIO("*" * BYTES)
+        reader = nova.api.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 nova.api.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 = nova.api.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,7 +76,7 @@ class TestRequestBodySizeLimiter(test.TestCase):
 
         @webob.dec.wsgify()
         def fake_app(req):
-            return webob.Response()
+            return webob.Response(req.body)
 
         self.middleware = nova.api.sizelimit.RequestBodySizeLimiter(fake_app)
         self.request = webob.Request.blank('/', method='POST')
@@ -40,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)