Skip to content

Commit

Permalink
fix(cli): url-encode path components of the URL
Browse files Browse the repository at this point in the history
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
Closes: #1498
  • Loading branch information
JohnVillalovos committed Jan 8, 2022
1 parent c9ed3dd commit ac1c619
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 13 deletions.
20 changes: 18 additions & 2 deletions gitlab/v4/cli.py
Expand Up @@ -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],
Expand All @@ -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_attrs()

self.mgr_cls._path = self.mgr_cls._path.format(**self.parent_args)
self.mgr = self.mgr_cls(gl)

if self.mgr_cls._types:
Expand All @@ -63,6 +67,18 @@ def __init__(
obj.set_from_cli(self.args[attr_name])
self.args[attr_name] = obj.get()

def _process_from_parent_attrs(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 not in self.args:
continue

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}"
Expand All @@ -85,7 +101,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)
Expand Down
14 changes: 14 additions & 0 deletions tests/functional/cli/conftest.py
@@ -1,4 +1,7 @@
import pytest
import responses

from gitlab.const import DEFAULT_URL


@pytest.fixture
Expand All @@ -19,3 +22,14 @@ def _gitlab_cli(subcommands):
return script_runner.run(*command)

return _gitlab_cli


@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,
}
11 changes: 0 additions & 11 deletions tests/functional/cli/test_cli.py
Expand Up @@ -17,17 +17,6 @@
CI_SERVER_URL = "https://gitlab.example.com"


@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,
}


def test_main_entrypoint(script_runner, gitlab_config):
ret = script_runner.run("python", "-m", "gitlab", "--config-file", gitlab_config)
assert ret.returncode == 2
Expand Down
35 changes: 35 additions & 0 deletions 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)
Expand All @@ -17,3 +26,29 @@ 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.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_variables = copy.deepcopy(resp_get_project)
resp_get_project_variables.update(
url=f"{DEFAULT_URL}/api/v4/projects/project%2Fwith%2Fa%2Fnamespace/variables"
)
resp_get_project_variables.update(json=[])

responses.add(**resp_get_project_variables)
ret = script_runner.run(
"gitlab", "project-variable", "list", "--project-id", "project/with/a/namespace"
)
assert ret.success

0 comments on commit ac1c619

Please sign in to comment.