Skip to content

fix: use url-encoded ID in all paths #1819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 13, 2022
Merged

fix: use url-encoded ID in all paths #1819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12435d7
fix: use url-encoded ID in all paths
JohnVillalovos Jan 10, 2022
a2e7c38
chore: add EncodedId string class to use to hold URL-encoded paths
JohnVillalovos Jan 11, 2022
b07eece
chore: replace usage of utils._url_encode() with utils.EncodedId()
JohnVillalovos Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions gitlab/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,15 @@ def get_id(self) -> Any:
return None
return getattr(self, self._id_attr)

@property
def encoded_id(self) -> Any:
"""Ensure that the ID is url-encoded so that it can be safely used in a URL
path"""
obj_id = self.get_id()
if isinstance(obj_id, str):
obj_id = gitlab.utils.EncodedId(obj_id)
return obj_id

@property
def attributes(self) -> Dict[str, Any]:
d = self.__dict__["_updated_attrs"].copy()
Expand Down
42 changes: 20 additions & 22 deletions gitlab/mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ def get(
GitlabAuthenticationError: If authentication is not correct
GitlabGetError: If the server cannot perform the request
"""
if not isinstance(id, int):
id = utils._url_encode(id)
if isinstance(id, str):
id = utils.EncodedId(id)
path = f"{self.path}/{id}"
if TYPE_CHECKING:
assert self._obj_cls is not None
Expand Down Expand Up @@ -173,7 +173,7 @@ def refresh(self, **kwargs: Any) -> None:
GitlabGetError: If the server cannot perform the request
"""
if self._id_attr:
path = f"{self.manager.path}/{self.id}"
path = f"{self.manager.path}/{self.encoded_id}"
else:
if TYPE_CHECKING:
assert self.manager.path is not None
Expand Down Expand Up @@ -391,7 +391,7 @@ def update(
if id is None:
path = self.path
else:
path = f"{self.path}/{id}"
path = f"{self.path}/{utils.EncodedId(id)}"

self._check_missing_update_attrs(new_data)
files = {}
Expand Down Expand Up @@ -444,7 +444,7 @@ def set(self, key: str, value: str, **kwargs: Any) -> base.RESTObject:
Returns:
The created/updated attribute
"""
path = f"{self.path}/{utils._url_encode(key)}"
path = f"{self.path}/{utils.EncodedId(key)}"
data = {"value": value}
server_data = self.gitlab.http_put(path, post_data=data, **kwargs)
if TYPE_CHECKING:
Expand Down Expand Up @@ -477,9 +477,7 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
if id is None:
path = self.path
else:
if not isinstance(id, int):
id = utils._url_encode(id)
path = f"{self.path}/{id}"
path = f"{self.path}/{utils.EncodedId(id)}"
self.gitlab.http_delete(path, **kwargs)


Expand Down Expand Up @@ -545,7 +543,7 @@ def save(self, **kwargs: Any) -> None:
return

# call the manager
obj_id = self.get_id()
obj_id = self.encoded_id
if TYPE_CHECKING:
assert isinstance(self.manager, UpdateMixin)
server_data = self.manager.update(obj_id, updated_data, **kwargs)
Expand Down Expand Up @@ -575,7 +573,7 @@ def delete(self, **kwargs: Any) -> None:
"""
if TYPE_CHECKING:
assert isinstance(self.manager, DeleteMixin)
self.manager.delete(self.get_id(), **kwargs)
self.manager.delete(self.encoded_id, **kwargs)


class UserAgentDetailMixin(_RestObjectBase):
Expand All @@ -598,7 +596,7 @@ def user_agent_detail(self, **kwargs: Any) -> Dict[str, Any]:
GitlabAuthenticationError: If authentication is not correct
GitlabGetError: If the server cannot perform the request
"""
path = f"{self.manager.path}/{self.get_id()}/user_agent_detail"
path = f"{self.manager.path}/{self.encoded_id}/user_agent_detail"
result = self.manager.gitlab.http_get(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(result, requests.Response)
Expand Down Expand Up @@ -631,7 +629,7 @@ def approve(
GitlabUpdateError: If the server fails to perform the request
"""

path = f"{self.manager.path}/{self.id}/approve"
path = f"{self.manager.path}/{self.encoded_id}/approve"
data = {"access_level": access_level}
server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs)
if TYPE_CHECKING:
Expand Down Expand Up @@ -705,7 +703,7 @@ def subscribe(self, **kwargs: Any) -> None:
GitlabAuthenticationError: If authentication is not correct
GitlabSubscribeError: If the subscription cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/subscribe"
path = f"{self.manager.path}/{self.encoded_id}/subscribe"
server_data = self.manager.gitlab.http_post(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(server_data, requests.Response)
Expand All @@ -725,7 +723,7 @@ def unsubscribe(self, **kwargs: Any) -> None:
GitlabAuthenticationError: If authentication is not correct
GitlabUnsubscribeError: If the unsubscription cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/unsubscribe"
path = f"{self.manager.path}/{self.encoded_id}/unsubscribe"
server_data = self.manager.gitlab.http_post(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(server_data, requests.Response)
Expand All @@ -752,7 +750,7 @@ def todo(self, **kwargs: Any) -> None:
GitlabAuthenticationError: If authentication is not correct
GitlabTodoError: If the todo cannot be set
"""
path = f"{self.manager.path}/{self.get_id()}/todo"
path = f"{self.manager.path}/{self.encoded_id}/todo"
self.manager.gitlab.http_post(path, **kwargs)


Expand Down Expand Up @@ -781,7 +779,7 @@ def time_stats(self, **kwargs: Any) -> Dict[str, Any]:
if "time_stats" in self.attributes:
return self.attributes["time_stats"]

path = f"{self.manager.path}/{self.get_id()}/time_stats"
path = f"{self.manager.path}/{self.encoded_id}/time_stats"
result = self.manager.gitlab.http_get(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(result, requests.Response)
Expand All @@ -800,7 +798,7 @@ def time_estimate(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
GitlabAuthenticationError: If authentication is not correct
GitlabTimeTrackingError: If the time tracking update cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/time_estimate"
path = f"{self.manager.path}/{self.encoded_id}/time_estimate"
data = {"duration": duration}
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
if TYPE_CHECKING:
Expand All @@ -819,7 +817,7 @@ def reset_time_estimate(self, **kwargs: Any) -> Dict[str, Any]:
GitlabAuthenticationError: If authentication is not correct
GitlabTimeTrackingError: If the time tracking update cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/reset_time_estimate"
path = f"{self.manager.path}/{self.encoded_id}/reset_time_estimate"
result = self.manager.gitlab.http_post(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(result, requests.Response)
Expand All @@ -838,7 +836,7 @@ def add_spent_time(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
GitlabAuthenticationError: If authentication is not correct
GitlabTimeTrackingError: If the time tracking update cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/add_spent_time"
path = f"{self.manager.path}/{self.encoded_id}/add_spent_time"
data = {"duration": duration}
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
if TYPE_CHECKING:
Expand All @@ -857,7 +855,7 @@ def reset_spent_time(self, **kwargs: Any) -> Dict[str, Any]:
GitlabAuthenticationError: If authentication is not correct
GitlabTimeTrackingError: If the time tracking update cannot be done
"""
path = f"{self.manager.path}/{self.get_id()}/reset_spent_time"
path = f"{self.manager.path}/{self.encoded_id}/reset_spent_time"
result = self.manager.gitlab.http_post(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(result, requests.Response)
Expand Down Expand Up @@ -893,7 +891,7 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
The list of participants
"""

path = f"{self.manager.path}/{self.get_id()}/participants"
path = f"{self.manager.path}/{self.encoded_id}/participants"
result = self.manager.gitlab.http_get(path, **kwargs)
if TYPE_CHECKING:
assert not isinstance(result, requests.Response)
Expand Down Expand Up @@ -967,7 +965,7 @@ def promote(self, **kwargs: Any) -> Dict[str, Any]:
The updated object data (*not* a RESTObject)
"""

path = f"{self.manager.path}/{self.id}/promote"
path = f"{self.manager.path}/{self.encoded_id}/promote"
http_method = self._get_update_method()
result = http_method(path, **kwargs)
if TYPE_CHECKING:
Expand Down
37 changes: 22 additions & 15 deletions gitlab/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import urllib.parse
from typing import Any, Callable, Dict, Optional
from typing import Any, Callable, Dict, Optional, Union

import requests

Expand Down Expand Up @@ -56,25 +56,32 @@ def copy_dict(dest: Dict[str, Any], src: Dict[str, Any]) -> None:
dest[k] = v


def _url_encode(id: str) -> str:
"""Encode/quote the characters in the string so that they can be used in a path.
class EncodedId(str):
"""A custom `str` class that will return the URL-encoded value of the string.

Reference to documentation on why this is necessary.

https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding

If using namespaced API requests, make sure that the NAMESPACE/PROJECT_PATH is
URL-encoded. For example, / is represented by %2F
* Using it recursively will only url-encode the value once.
* Can accept either `str` or `int` as input value.
* Can be used in an f-string and output the URL-encoded string.

https://docs.gitlab.com/ee/api/index.html#path-parameters
Reference to documentation on why this is necessary.

Path parameters that are required to be URL-encoded must be followed. If not, it
doesn’t match an API endpoint and responds with a 404. If there’s something in front
of the API (for example, Apache), ensure that it doesn’t decode the URL-encoded path
parameters.
See::

https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
https://docs.gitlab.com/ee/api/index.html#path-parameters
"""
return urllib.parse.quote(id, safe="")

# mypy complains if return type other than the class type. So we ignore issue.
def __new__( # type: ignore
cls, value: Union[str, int, "EncodedId"]
) -> Union[int, "EncodedId"]:
if isinstance(value, (int, EncodedId)):
return value

if not isinstance(value, str):
raise TypeError(f"Unsupported type received: {type(value)}")
value = urllib.parse.quote(value, safe="")
return super().__new__(cls, value)


def remove_none_from_dict(data: Dict[str, Any]) -> Dict[str, Any]:
Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def _process_from_parent_attrs(self) -> None:
if key not in self.args:
continue

self.parent_args[key] = gitlab.utils._url_encode(self.args[key])
self.parent_args[key] = gitlab.utils.EncodedId(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]

Expand Down
12 changes: 6 additions & 6 deletions gitlab/v4/objects/commits.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def diff(self, **kwargs: Any) -> Union[gitlab.GitlabList, List[Dict[str, Any]]]:
Returns:
The changes done in this commit
"""
path = f"{self.manager.path}/{self.get_id()}/diff"
path = f"{self.manager.path}/{self.encoded_id}/diff"
return self.manager.gitlab.http_list(path, **kwargs)

@cli.register_custom_action("ProjectCommit", ("branch",))
Expand All @@ -58,7 +58,7 @@ def cherry_pick(self, branch: str, **kwargs: Any) -> None:
GitlabAuthenticationError: If authentication is not correct
GitlabCherryPickError: If the cherry-pick could not be performed
"""
path = f"{self.manager.path}/{self.get_id()}/cherry_pick"
path = f"{self.manager.path}/{self.encoded_id}/cherry_pick"
post_data = {"branch": branch}
self.manager.gitlab.http_post(path, post_data=post_data, **kwargs)

Expand All @@ -80,7 +80,7 @@ def refs(
Returns:
The references the commit is pushed to.
"""
path = f"{self.manager.path}/{self.get_id()}/refs"
path = f"{self.manager.path}/{self.encoded_id}/refs"
query_data = {"type": type}
return self.manager.gitlab.http_list(path, query_data=query_data, **kwargs)

Expand All @@ -101,7 +101,7 @@ def merge_requests(
Returns:
The merge requests related to the commit.
"""
path = f"{self.manager.path}/{self.get_id()}/merge_requests"
path = f"{self.manager.path}/{self.encoded_id}/merge_requests"
return self.manager.gitlab.http_list(path, **kwargs)

@cli.register_custom_action("ProjectCommit", ("branch",))
Expand All @@ -122,7 +122,7 @@ def revert(
Returns:
The new commit data (*not* a RESTObject)
"""
path = f"{self.manager.path}/{self.get_id()}/revert"
path = f"{self.manager.path}/{self.encoded_id}/revert"
post_data = {"branch": branch}
return self.manager.gitlab.http_post(path, post_data=post_data, **kwargs)

Expand All @@ -141,7 +141,7 @@ def signature(self, **kwargs: Any) -> Union[Dict[str, Any], requests.Response]:
Returns:
The commit's signature data
"""
path = f"{self.manager.path}/{self.get_id()}/signature"
path = f"{self.manager.path}/{self.encoded_id}/signature"
return self.manager.gitlab.http_get(path, **kwargs)


Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/objects/environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def stop(self, **kwargs: Any) -> Union[Dict[str, Any], requests.Response]:
Returns:
A dict of the result.
"""
path = f"{self.manager.path}/{self.get_id()}/stop"
path = f"{self.manager.path}/{self.encoded_id}/stop"
return self.manager.gitlab.http_post(path, **kwargs)


Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/objects/epics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def save(self, **kwargs: Any) -> None:
return

# call the manager
obj_id = self.get_id()
obj_id = self.encoded_id
self.manager.update(obj_id, updated_data, **kwargs)


Expand Down
2 changes: 1 addition & 1 deletion gitlab/v4/objects/features.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def set(
Returns:
The created/updated attribute
"""
name = utils._url_encode(name)
name = utils.EncodedId(name)
path = f"{self.path}/{name}"
data = {
"value": value,
Expand Down
14 changes: 7 additions & 7 deletions gitlab/v4/objects/files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def save( # type: ignore
"""
self.branch = branch
self.commit_message = commit_message
self.file_path = utils._url_encode(self.file_path)
self.file_path = utils.EncodedId(self.file_path)
super(ProjectFile, self).save(**kwargs)

@exc.on_http_error(exc.GitlabDeleteError)
Expand All @@ -76,7 +76,7 @@ def delete( # type: ignore
GitlabAuthenticationError: If authentication is not correct
GitlabDeleteError: If the server cannot perform the request
"""
file_path = utils._url_encode(self.get_id())
file_path = self.encoded_id
self.manager.delete(file_path, branch, commit_message, **kwargs)


Expand Down Expand Up @@ -144,7 +144,7 @@ def create(
assert data is not None
self._check_missing_create_attrs(data)
new_data = data.copy()
file_path = utils._url_encode(new_data.pop("file_path"))
file_path = utils.EncodedId(new_data.pop("file_path"))
path = f"{self.path}/{file_path}"
server_data = self.gitlab.http_post(path, post_data=new_data, **kwargs)
if TYPE_CHECKING:
Expand Down Expand Up @@ -173,7 +173,7 @@ def update( # type: ignore
"""
new_data = new_data or {}
data = new_data.copy()
file_path = utils._url_encode(file_path)
file_path = utils.EncodedId(file_path)
data["file_path"] = file_path
path = f"{self.path}/{file_path}"
self._check_missing_update_attrs(data)
Expand Down Expand Up @@ -203,7 +203,7 @@ def delete( # type: ignore
GitlabAuthenticationError: If authentication is not correct
GitlabDeleteError: If the server cannot perform the request
"""
file_path = utils._url_encode(file_path)
file_path = utils.EncodedId(file_path)
path = f"{self.path}/{file_path}"
data = {"branch": branch, "commit_message": commit_message}
self.gitlab.http_delete(path, query_data=data, **kwargs)
Expand Down Expand Up @@ -239,7 +239,7 @@ def raw(
Returns:
The file content
"""
file_path = utils._url_encode(file_path)
file_path = utils.EncodedId(file_path)
path = f"{self.path}/{file_path}/raw"
query_data = {"ref": ref}
result = self.gitlab.http_get(
Expand All @@ -266,7 +266,7 @@ def blame(self, file_path: str, ref: str, **kwargs: Any) -> List[Dict[str, Any]]
Returns:
A list of commits/lines matching the file
"""
file_path = utils._url_encode(file_path)
file_path = utils.EncodedId(file_path)
path = f"{self.path}/{file_path}/blame"
query_data = {"ref": ref}
result = self.gitlab.http_list(path, query_data, **kwargs)
Expand Down
Loading