fix: convert # to %23 in URLsfix/779

Refactor a bit to handle this change, and add unit tests.

Closes #779

3 files changed, 51 insertions, 3 deletions

diff --git a/gitlab/mixins.py b/gitlab/mixins.py
index 70de992..b1309f6 100644
--- a/gitlab/mixins.py
+++ b/gitlab/mixins.py
@@ -20,6 +20,7 @@ from gitlab import base
 from gitlab import cli
 from gitlab import exceptions as exc
 from gitlab import types as g_types
+from gitlab import utils
 
 
 class GetMixin(object):
@@ -42,7 +43,7 @@ class GetMixin(object):
             GitlabGetError: If the server cannot perform the request
         """
         if not isinstance(id, int):
-            id = id.replace("/", "%2F")
+            id = utils.clean_str_id(id)
         path = "%s/%s" % (self.path, id)
         if lazy is True:
             return self._obj_cls(self, {self._obj_cls._id_attr: id})
@@ -299,7 +300,7 @@ class SetMixin(object):
         Returns:
             obj: The created/updated attribute
         """
-        path = "%s/%s" % (self.path, key.replace("/", "%2F"))
+        path = "%s/%s" % (self.path, utils.clean_str_id(key))
         data = {"value": value}
         server_data = self.gitlab.http_put(path, post_data=data, **kwargs)
         return self._obj_cls(self, server_data)
@@ -322,7 +323,7 @@ class DeleteMixin(object):
             path = self.path
         else:
             if not isinstance(id, int):
-                id = id.replace("/", "%2F")
+                id = utils.clean_str_id(id)
             path = "%s/%s" % (self.path, id)
         self.gitlab.http_delete(path, **kwargs)
 
diff --git a/gitlab/tests/test_utils.py b/gitlab/tests/test_utils.py
new file mode 100644
index 0000000..f84f954
--- /dev/null
+++ b/gitlab/tests/test_utils.py
@@ -0,0 +1,43 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2019 Gauvain Pocentek <gauvain@pocentek.net>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+try:
+    import unittest
+except ImportError:
+    import unittest2 as unittest
+
+from gitlab import utils
+
+
+class TestUtils(unittest.TestCase):
+    def test_clean_str_id(self):
+        src = "nothing_special"
+        dest = "nothing_special"
+        self.assertEqual(dest, utils.clean_str_id(src))
+
+        src = "foo#bar/baz/"
+        dest = "foo%23bar%2Fbaz%2F"
+        self.assertEqual(dest, utils.clean_str_id(src))
+
+    def test_sanitized_url(self):
+        src = "http://localhost/foo/bar"
+        dest = "http://localhost/foo/bar"
+        self.assertEqual(dest, utils.sanitized_url(src))
+
+        src = "http://localhost/foo.bar.baz"
+        dest = "http://localhost/foo%2Ebar%2Ebaz"
+        self.assertEqual(dest, utils.sanitized_url(src))
diff --git a/gitlab/utils.py b/gitlab/utils.py
index 6b43800..94528e1 100644
--- a/gitlab/utils.py
+++ b/gitlab/utils.py
@@ -47,6 +47,10 @@ def copy_dict(dest, src):
             dest[k] = v
 
 
+def clean_str_id(id):
+    return id.replace("/", "%2F").replace("#", "%23")
+
+
 def sanitized_url(url):
     parsed = six.moves.urllib.parse.urlparse(url)
     new_path = parsed.path.replace(".", "%2E")