From 118a76fd651c19ad098cdee9d0a122a00ddc4e3b Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Thu, 1 Oct 2015 17:13:25 +0200 Subject: [PATCH] metadata: don't crash proxy on non-unicode user data We attempt to log every successful metadata response with LOG.debug. But as per oslo.log docs [1], we should make sure that what we pass into the library is unicode. Http.request returns a tuple of Response object and a string, which is bytes in Python 2.x [2]. That's why we need to convert the response content to unicode before passing it into oslo.log. To achieve it, we utilize encodeutils.safe_decode with 'replace' errors handling strategy, so that we don't get exceptions on input that does not conform unicode. For the unit test case, we pass a string that is not expected to convert to unicode with errors='strict' strategy or similar, and check that we still don't crash. While at it, we remove a check for the number of log calls being triggered, because it's something that we should avoid validating in test cases, and it cannot trigger a real bug. The mock that was used to count the number would also hide the bug that we try to reproduce. Note that the bug does not require debug to be set because the crash occurs before oslo.log machinery decides it should not log the message. [1]: http://docs.openstack.org/developer/oslo.log/usage.html#no-more-implicit-conversion-to-unicode-str [2]: http://bitworking.org/projects/httplib2/doc/html/libhttplib2.html#httplib2.Http.request Conflicts: neutron/agent/metadata/namespace_proxy.py neutron/tests/unit/agent/metadata/test_namespace_proxy.py Closes-Bug: #1501772 Change-Id: I6a32c40ff117fae43913386134c8981539697ce8 (cherry picked from commit 80e3d9be4923ecad17377b1d15c8392b8a43dac6) --- neutron/agent/metadata/namespace_proxy.py | 3 ++- .../unit/agent/metadata/test_namespace_proxy.py | 16 +++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/neutron/agent/metadata/namespace_proxy.py b/neutron/agent/metadata/namespace_proxy.py index e84a256de69..273c14c9b01 100644 --- a/neutron/agent/metadata/namespace_proxy.py +++ b/neutron/agent/metadata/namespace_proxy.py @@ -15,6 +15,7 @@ import httplib2 from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import encodeutils import six.moves.urllib.parse as urlparse import webob @@ -86,7 +87,7 @@ class NetworkMetadataProxyHandler(object): if resp.status == 200: LOG.debug(resp) - LOG.debug(content) + LOG.debug(encodeutils.safe_decode(content, errors='replace')) response = webob.Response() response.status = resp.status response.headers['Content-Type'] = resp['content-type'] diff --git a/neutron/tests/unit/agent/metadata/test_namespace_proxy.py b/neutron/tests/unit/agent/metadata/test_namespace_proxy.py index 8cf8d1415ff..e7f70224e10 100644 --- a/neutron/tests/unit/agent/metadata/test_namespace_proxy.py +++ b/neutron/tests/unit/agent/metadata/test_namespace_proxy.py @@ -38,9 +38,6 @@ class FakeConf(object): class TestNetworkMetadataProxyHandler(base.BaseTestCase): def setUp(self): super(TestNetworkMetadataProxyHandler, self).setUp() - self.log_p = mock.patch.object(ns_proxy, 'LOG') - self.log = self.log_p.start() - self.handler = ns_proxy.NetworkMetadataProxyHandler('router_id') def test_call(self): @@ -67,7 +64,6 @@ class TestNetworkMetadataProxyHandler(base.BaseTestCase): proxy_req.side_effect = Exception retval = self.handler(req) self.assertIsInstance(retval, webob.exc.HTTPInternalServerError) - self.assertEqual(len(self.log.mock_calls), 2) self.assertTrue(proxy_req.called) def test_proxy_request_router_200(self): @@ -100,13 +96,13 @@ class TestNetworkMetadataProxyHandler(base.BaseTestCase): self.assertEqual(retval.headers['Content-Type'], 'text/plain') self.assertEqual(retval.body, 'content') - def test_proxy_request_network_200(self): + def _test_proxy_request_network_200(self, content): self.handler.network_id = 'network_id' resp = mock.MagicMock(status=200) with mock.patch('httplib2.Http') as mock_http: resp.__getitem__.return_value = "application/json" - mock_http.return_value.request.return_value = (resp, '{}') + mock_http.return_value.request.return_value = (resp, content) retval = self.handler._proxy_request('192.168.1.1', 'GET', @@ -129,7 +125,13 @@ class TestNetworkMetadataProxyHandler(base.BaseTestCase): self.assertEqual(retval.headers['Content-Type'], 'application/json') - self.assertEqual(retval.body, '{}') + self.assertEqual(content, retval.body) + + def test_proxy_request_network_200(self): + self._test_proxy_request_network_200('{}') + + def test_proxy_request_network_200_unicode_in_content(self): + self._test_proxy_request_network_200('Gl\xfcck') def _test_proxy_request_network_4xx(self, status, method, expected): self.handler.network_id = 'network_id'