Escape resource IDs when appending to URLs

gnocchiclient/tests/functional/test_resource.py

@@ -17,7 +17,8 @@
 
 class ResourceClientTest(base.ClientTestBase):
     RESOURCE_ID = str(uuid.uuid4())
-    RESOURCE_ID2 = "foo"
+    # NOTE(callumdickinson): Also tests URL escaping.
+    RESOURCE_ID2 = "foo%201"
     PROJECT_ID = str(uuid.uuid4())
 
     def test_help(self):

gnocchiclient/v1/metric.py

@@ -15,6 +15,7 @@
 import uuid
 
 from debtcollector import removals
+from urllib import parse as urlparse
 
 import iso8601
 
@@ -77,7 +78,8 @@ def get(self, metric, resource_id=None):
             self._ensure_metric_is_uuid(metric)
             url = self.metric_url + metric
         else:
-            url = (self.resource_url % resource_id) + metric
+            url = (self.resource_url
+                   % urlparse.quote(resource_id)) + metric
         return self._get(url).json()
 
     # FIXME(jd): This is what create will be after debtcollector warnings have
@@ -114,7 +116,7 @@ def _create_new(self, name=None, archive_policy_name=None,
                 "Metric name is required if resource_id is set")
 
         return self._post(
-            self.resource_url % resource_id,
+            self.resource_url % urlparse.quote(resource_id),
             headers={'Content-Type': "application/json"},
             data=ujson.dumps({name: metric})).json()[0]
 
@@ -168,7 +170,7 @@ def create(self, metric=None, refetch_metric=True,
         del metric['resource_id']
         metric = {metric_name: metric}
         metric = self._post(
-            self.resource_url % resource_id,
+            self.resource_url % urlparse.quote(resource_id),
             headers={'Content-Type': "application/json"},
             data=ujson.dumps(metric))
         return self.get(metric_name, resource_id)
@@ -186,7 +188,8 @@ def delete(self, metric, resource_id=None):
             self._ensure_metric_is_uuid(metric)
             url = self.metric_url + metric
         else:
-            url = self.resource_url % resource_id + metric
+            url = (self.resource_url
+                   % urlparse.quote(resource_id)) + metric
         self._delete(url)
 
     def add_measures(self, metric, measures, resource_id=None):
@@ -204,7 +207,8 @@ def add_measures(self, metric, measures, resource_id=None):
             self._ensure_metric_is_uuid(metric)
             url = self.metric_url + metric + "/measures"
         else:
-            url = self.resource_url % resource_id + metric + "/measures"
+            url = (self.resource_url
+                   % urlparse.quote(resource_id)) + metric + "/measures"
         return self._post(
             url, headers={'Content-Type': "application/json"},
             data=ujson.dumps(measures))
@@ -272,7 +276,8 @@ def get_measures(self, metric, start=None, stop=None, aggregation=None,
             self._ensure_metric_is_uuid(metric)
             url = self.metric_url + metric + "/measures"
         else:
-            url = self.resource_url % resource_id + metric + "/measures"
+            url = (self.resource_url
+                   % urlparse.quote(resource_id)) + metric + "/measures"
         measures = self._get(url, params=params).json()
         return [(iso8601.parse_date(ts), g, value)
                 for ts, g, value in measures]

gnocchiclient/v1/resource.py

@@ -13,6 +13,8 @@
 
 import ujson
 
+from urllib import parse as urlparse
+
 from gnocchiclient import utils
 from gnocchiclient.v1 import base
 
@@ -64,7 +66,8 @@ def get(self, resource_type, resource_id, history=False):
         :type history: bool
         """
         history = "/history" if history else ""
-        url = self.url + "%s/%s%s" % (resource_type, resource_id, history)
+        url = self.url + "%s/%s%s" % (
+            resource_type, urlparse.quote(resource_id), history)
         return self._get(url).json()
 
     def history(self, resource_type, resource_id, details=False,
@@ -88,7 +91,8 @@ def history(self, resource_type, resource_id, details=False,
         """
         params = utils.build_pagination_options(details, False, limit, marker,
                                                 sorts)
-        url = "%s%s/%s/history?%s" % (self.url, resource_type, resource_id,
+        url = "%s%s/%s/history?%s" % (self.url, resource_type,
+                                      urlparse.quote(resource_id),
                                       utils.dict_to_querystring(params))
         return self._get(url).json()
 
@@ -116,7 +120,10 @@ def update(self, resource_type, resource_id, resource):
         :type resource: dict
         """
         return self._patch(
-            self.url + resource_type + "/" + resource_id,
+            (self.url
+             + resource_type
+             + "/"
+             + urlparse.quote(resource_id)),
             headers={'Content-Type': "application/json"},
             data=ujson.dumps(resource)).json()
 
@@ -126,7 +133,7 @@ def delete(self, resource_id):
         :param resource_id: ID of the resource
         :type resource_id: str
         """
-        self._delete(self.url + "generic/" + resource_id)
+        self._delete(self.url + "generic/" + urlparse.quote(resource_id))
 
     def batch_delete(self, query, resource_type="generic"):
         """Delete a batch of resources based on attribute values.