[lxc-devel] [pylxd/master] Fix bug/281 unable to get file with json content

[ajkavanagh on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 620 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180320/730a2688/attachment.bin>
-------------- next part --------------
From 84f5c942d849131524467a17270580229b612430 Mon Sep 17 00:00:00 2001
From: Alex Kavanagh <alex at ajkavanagh.co.uk>
Date: Tue, 20 Mar 2018 12:28:55 +0000
Subject: [PATCH] Fix bug/281 unable to get file with json content

The issue was that the files handling uses the API get code path to
fetch files.  Thus json files were intepreted as an API reponse and
checked for content.  This change ensures that the contents of files are
returned without trying to interpret the contents.

Signed-off-by: Alex Kavanagh <alex at ajkavanagh.co.uk>
---
 integration/test_profiles.py         |  2 --
 pylxd/client.py                      | 20 +++++++++++++++-----
 pylxd/models/container.py            |  2 +-
 pylxd/tests/mock_lxd.py              |  5 +++++
 pylxd/tests/models/test_container.py |  6 ++++++
 5 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/integration/test_profiles.py b/integration/test_profiles.py
index e32afe8..3e6dc0d 100644
--- a/integration/test_profiles.py
+++ b/integration/test_profiles.py
@@ -11,8 +11,6 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-import unittest
-
 from pylxd import exceptions
 
 from integration.testing import IntegrationTestCase
diff --git a/pylxd/client.py b/pylxd/client.py
index d5bb8bf..e8622e3 100644
--- a/pylxd/client.py
+++ b/pylxd/client.py
@@ -75,8 +75,8 @@ def __getitem__(self, item):
             verify=self.session.verify,
             timeout=self._timeout)
 
-    def _assert_response(
-            self, response, allowed_status_codes=(200,), stream=False):
+    def _assert_response(self, response, allowed_status_codes=(200,),
+                         stream=False, is_api=True):
         """Assert properties of the response.
 
         LXD's API clearly defines specific responses. If the API
@@ -92,7 +92,9 @@ def _assert_response(
 
         # In the case of streaming, we can't validate the json the way we
         # would with normal HTTP responses, so just ignore that entirely.
-        if stream:
+        # Likewize, we can ignore NON api calls as the contents don't need to
+        # be validated.
+        if stream or not is_api:
             return
 
         try:
@@ -119,10 +121,18 @@ def netloc(self):
         return parse.urlparse(self.api._api_endpoint).netloc
 
     def get(self, *args, **kwargs):
-        """Perform an HTTP GET."""
+        """Perform an HTTP GET.
+
+        Note if 'is_api' is passed in the kwargs then it is popped and used to
+        determine whether the get is an API call or a raw call.
+        This is for py27 compatibility.
+        """
+        is_api = kwargs.pop('is_api', True)
         kwargs['timeout'] = kwargs.get('timeout', self._timeout)
         response = self.session.get(self._api_endpoint, *args, **kwargs)
-        self._assert_response(response, stream=kwargs.get('stream', False))
+        self._assert_response(response,
+                              stream=kwargs.get('stream', False),
+                              is_api=is_api)
         return response
 
     def post(self, *args, **kwargs):
diff --git a/pylxd/models/container.py b/pylxd/models/container.py
index bc7f984..b22485e 100644
--- a/pylxd/models/container.py
+++ b/pylxd/models/container.py
@@ -138,7 +138,7 @@ def delete(self, filepath):
 
         def get(self, filepath):
             response = (self._client.api.containers[self._container.name]
-                        .files.get(params={'path': filepath}))
+                        .files.get(params={'path': filepath}, is_api=False))
             return response.content
 
     @classmethod
diff --git a/pylxd/tests/mock_lxd.py b/pylxd/tests/mock_lxd.py
index b2febd3..d23004c 100644
--- a/pylxd/tests/mock_lxd.py
+++ b/pylxd/tests/mock_lxd.py
@@ -353,6 +353,11 @@ def profile_GET(request, context):
         'method': 'GET',
         'url': r'^http://pylxd.test/1.0/containers/an-container/files\?path=%2Ftmp%2Fgetted$',  # NOQA
     },
+    {
+        'text': '{"some": "value"}',
+        'method': 'GET',
+        'url': r'^http://pylxd.test/1.0/containers/an-container/files\?path=%2Ftmp%2Fjson-get$',  # NOQA
+    },
     {
         'method': 'POST',
         'url': r'^http://pylxd.test/1.0/containers/an-container/files\?path=%2Ftmp%2Fputted$',  # NOQA
diff --git a/pylxd/tests/models/test_container.py b/pylxd/tests/models/test_container.py
index b3db7ab..08e0adf 100644
--- a/pylxd/tests/models/test_container.py
+++ b/pylxd/tests/models/test_container.py
@@ -551,3 +551,9 @@ def not_found(request, context):
         self.assertRaises(
             exceptions.LXDAPIException,
             self.container.files.get, '/tmp/getted')
+
+    # for bug/281 -- getting an empty json file is interpreted as an API
+    # get rather than a raw get.
+    def test_get_json_file(self):
+        data = self.container.files.get('/tmp/json-get')
+        self.assertEqual(b'{"some": "value"}', data)