From bf7887a102ca33dd46ef24b67b07eb77154ce133 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 16 Aug 2017 10:29:46 -0400 Subject: [PATCH] Replace body_file with class to call uwsgi.chunked_read() Since the WebOb 1.7 release webob doesn't natively support receiving chunked transfer encoding bodies. [1] When glance is run under the eventlet wsgi server this was fine, because eventlet will dechunk the input on read() (or readline()) calls, so from the webob perspective it's just a file object. However, the effort to remove the dependence on using eventlet as the web server and deploy glance as a traditional wsgi script we lose this mechanism. The wsgi spec doesn't provide a consistent mechanism When we run glance under uwsgi the uwsgi server provides an api to read chunked data. [2] However, we need to explicitly call that api when to dechunk the data and pass it to glance code which expects a file object. This commit solves this issue by creating a fake file class that will call the chunked_read() api from uwsgi on read() calls. This object is then used if we're running the api code under uwsgi and the body has a chunked transfer-encoding. In conjuction with devstack change Iab2e2848877fa1497008d18c05b0154892941589 this closes glance bug 1703856 [1] https://docs.pylonsproject.org/projects/webob/en/stable/whatsnew-1.7.html#backwards-incompatibility [2] http://uwsgi-docs.readthedocs.io/en/latest/Chunked.html Partial-bug 1703856 Co-Authored-By: Chris Dent Change-Id: Idf6b4b891ba31cccbeb53d373b40fce5380cea64 --- glance/common/wsgi.py | 46 +++++++++++++++++++ glance/tests/unit/common/test_wsgi.py | 65 +++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index 26e5f6f24b..9ce7d9e079 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -320,6 +320,14 @@ profiler_opts.set_defaults(CONF) ASYNC_EVENTLET_THREAD_POOL_LIST = [] +# Detect if we're running under the uwsgi server +try: + import uwsgi + LOG.debug('Detected running under uwsgi') +except ImportError: + LOG.debug('Detected not running under uwsgi') + uwsgi = None + def get_num_workers(): """Return the configured number of workers.""" @@ -924,6 +932,31 @@ class Router(object): return app +class _UWSGIChunkFile(object): + + def read(self, length=None): + position = 0 + if length == 0: + return b"" + + if length and length < 0: + length = None + + response = [] + while True: + data = uwsgi.chunked_read() + # Return everything if we reached the end of the file + if not data: + break + response.append(data) + # Return the data if we've reached the length + if length is not None: + position += len(data) + if position >= length: + break + return b''.join(response) + + class Request(webob.Request): """Add some OpenStack API-specific logic to the base webob.Request.""" @@ -934,6 +967,19 @@ class Request(webob.Request): environ['wsgi.url_scheme'] = scheme super(Request, self).__init__(environ, *args, **kwargs) + @property + def body_file(self): + if uwsgi: + if self.headers.get('transfer-encoding', '').lower() == 'chunked': + return _UWSGIChunkFile() + return super(Request, self).body_file + + @body_file.setter + def body_file(self, value): + # NOTE(cdent): If you have a property setter in a superclass, it will + # not be inherited. + webob.Request.body_file.fset(self, value) + @property def params(self): """Override params property of webob.request.BaseRequest. diff --git a/glance/tests/unit/common/test_wsgi.py b/glance/tests/unit/common/test_wsgi.py index f82418775a..0f4c5ad566 100644 --- a/glance/tests/unit/common/test_wsgi.py +++ b/glance/tests/unit/common/test_wsgi.py @@ -738,3 +738,68 @@ class GetSocketTestCase(test_utils.BaseTestCase): 'glance.common.wsgi.ssl.wrap_socket', lambda *x, **y: None)) self.assertRaises(wsgi.socket.error, wsgi.get_socket, 1234) + + +def _cleanup_uwsgi(): + wsgi.uwsgi = None + + +class Test_UwsgiChunkedFile(test_utils.BaseTestCase): + + def test_read_no_data(self): + reader = wsgi._UWSGIChunkFile() + wsgi.uwsgi = mock.MagicMock() + self.addCleanup(_cleanup_uwsgi) + + def fake_read(): + return None + + wsgi.uwsgi.chunked_read = fake_read + out = reader.read() + self.assertEqual(out, b'') + + def test_read_data_no_length(self): + reader = wsgi._UWSGIChunkFile() + wsgi.uwsgi = mock.MagicMock() + self.addCleanup(_cleanup_uwsgi) + + values = iter([b'a', b'b', b'c', None]) + + def fake_read(): + return next(values) + + wsgi.uwsgi.chunked_read = fake_read + out = reader.read() + self.assertEqual(out, b'abc') + + def test_read_zero_length(self): + reader = wsgi._UWSGIChunkFile() + self.assertEqual(b'', reader.read(length=0)) + + def test_read_data_length(self): + reader = wsgi._UWSGIChunkFile() + wsgi.uwsgi = mock.MagicMock() + self.addCleanup(_cleanup_uwsgi) + + values = iter([b'a', b'b', b'c', None]) + + def fake_read(): + return next(values) + + wsgi.uwsgi.chunked_read = fake_read + out = reader.read(length=2) + self.assertEqual(out, b'ab') + + def test_read_data_negative_length(self): + reader = wsgi._UWSGIChunkFile() + wsgi.uwsgi = mock.MagicMock() + self.addCleanup(_cleanup_uwsgi) + + values = iter([b'a', b'b', b'c', None]) + + def fake_read(): + return next(values) + + wsgi.uwsgi.chunked_read = fake_read + out = reader.read(length=-2) + self.assertEqual(out, b'abc')