fix: cli: url-encode path components of the URL

JohnVillalovos · JohnVillalovos · commit 3243735a8aba · 2022-01-06T17:10:21.000-08:00
In the CLI we need to make sure the components put into the path portion of the URL are url-encoded. Otherwise they will be interpreted as part of the path. For example can specify the project ID as a path, but in the URL it must be url-encoded or it doesn't work. Also stop adding the components of the path as query parameters in the URL. Closes: #783
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
@@ -39,6 +39,7 @@ def __init__(
         self.action = action.lower()
         self.gl = gl
         self.args = args
+        self.parent_args: Dict[str, Any] = {}
         self.mgr_cls: Union[
             Type[gitlab.mixins.CreateMixin],
             Type[gitlab.mixins.DeleteMixin],
@@ -53,7 +54,10 @@ def __init__(
         # the class _path attribute, and replace the value with the result.
         if TYPE_CHECKING:
             assert self.mgr_cls._path is not None
-        self.mgr_cls._path = self.mgr_cls._path.format(**self.args)
+
+        self._process_from_parent_attributes()
+
+        self.mgr_cls._path = self.mgr_cls._path.format(**self.parent_args)
         self.mgr = self.mgr_cls(gl)
 
         if self.mgr_cls._types:
@@ -63,6 +67,17 @@ def __init__(
                     obj.set_from_cli(self.args[attr_name])
                     self.args[attr_name] = obj.get()
 
+    def _process_from_parent_attributes(self) -> None:
+        """Items in the path need to be url-encoded. There is a 1:1 mapping from
+        mgr_cls._from_parent_attrs <--> mgr_cls._path. Those values must be url-encoded
+        as they may contain a slash '/'."""
+        for key in self.mgr_cls._from_parent_attrs:
+            if key in self.args:
+                self.parent_args[key] = gitlab.utils.clean_str_id(self.args[key])
+                # If we don't delete it then it will be added to the URL as a
+                # query-string
+                del self.args[key]
+
     def __call__(self) -> Any:
         # Check for a method that matches object + action
         method = f"do_{self.what}_{self.action}"
@@ -85,7 +100,7 @@ def do_custom(self) -> Any:
             data = {}
             if self.mgr._from_parent_attrs:
                 for k in self.mgr._from_parent_attrs:
-                    data[k] = self.args[k]
+                    data[k] = self.parent_args[k]
             if not issubclass(self.cls, gitlab.mixins.GetWithoutIdMixin):
                 if TYPE_CHECKING:
                     assert isinstance(self.cls._id_attr, str)
diff --git a/tests/functional/cli/test_cli_variables.py b/tests/functional/cli/test_cli_variables.py
@@ -1,3 +1,12 @@
+import copy
+
+import pytest
+import responses
+
+from gitlab import config
+from gitlab.const import DEFAULT_URL
+
+
 def test_list_instance_variables(gitlab_cli, gl):
     cmd = ["variable", "list"]
     ret = gitlab_cli(cmd)
@@ -17,3 +26,59 @@ def test_list_project_variables(gitlab_cli, project):
     ret = gitlab_cli(cmd)
 
     assert ret.success
+
+
+def test_list_project_variables_with_path(gitlab_cli, project):
+    cmd = ["project-variable", "list", "--project-id", project.path_with_namespace]
+    ret = gitlab_cli(cmd)
+
+    assert ret.success
+
+
+@pytest.fixture
+def resp_get_project():
+    return {
+        "method": responses.GET,
+        "url": f"{DEFAULT_URL}/api/v4/projects/1",
+        "json": {"name": "name", "path": "test-path", "id": 1},
+        "content_type": "application/json",
+        "status": 200,
+    }
+
+
+@pytest.mark.script_launch_mode("inprocess")
+@responses.activate
+def test_list_project_variables_with_path_url_check(
+    monkeypatch, script_runner, resp_get_project
+):
+    monkeypatch.setattr(config, "_DEFAULT_FILES", [])
+    resp_get_project_in_ci = copy.deepcopy(resp_get_project)
+    resp_get_project_in_ci.update(
+        url=f"{DEFAULT_URL}/api/v4/projects/project%2Fwith%2Fa%2Fnamespace/variables"
+    )
+    resp_get_project_in_ci.update(
+        json=[
+            {
+                "variable_type": "env_var",
+                "key": "TEST_VARIABLE_1",
+                "value": "TEST_1",
+                "protected": False,
+                "masked": True,
+                "environment_scope": "*",
+            },
+            {
+                "variable_type": "env_var",
+                "key": "TEST_VARIABLE_2",
+                "value": "TEST_2",
+                "protected": False,
+                "masked": False,
+                "environment_scope": "*",
+            },
+        ]
+    )
+
+    responses.add(**resp_get_project_in_ci)
+    ret = script_runner.run(
+        "gitlab", "project-variable", "list", "--project-id", "project/with/a/namespace"
+    )
+    assert ret.success
