Skip to content

Commit 598aa08

Browse files
JohnVillalovosJohnVillalovos
committed
fix: use url-encoded ID in all paths
Make sure all usage of the ID in the URL path is encoded. Normally it isn't an issue as most IDs are integers or strings which don't contain a slash ('/'). But when the ID is a string with a slash character it will break things. Add a test case that shows this fixes wikis issue with subpages which use the slash character. Closes: #1079
1 parent ac81272 commit 598aa08

17 files changed

+109
-83
lines changed

‎gitlab/base.py

+9
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,15 @@ def get_id(self) -> Any:
208208
return None
209209
return getattr(self, self._id_attr)
210210

211+
@property
212+
def encoded_id(self) -> Any:
213+
"""Ensure that the ID is url-encoded so that it can be safely used in a URL
214+
path"""
215+
obj_id = self.get_id()
216+
if isinstance(obj_id, str):
217+
obj_id = gitlab.utils._url_encode(obj_id)
218+
return obj_id
219+
211220
@property
212221
def attributes(self) -> Dict[str, Any]:
213222
d = self.__dict__["_updated_attrs"].copy()

‎gitlab/mixins.py

+13-11
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ def save(self, **kwargs: Any) -> None:
545545
return
546546

547547
# call the manager
548-
obj_id = self.get_id()
548+
obj_id = self.encoded_id
549549
if TYPE_CHECKING:
550550
assert isinstance(self.manager, UpdateMixin)
551551
server_data = self.manager.update(obj_id, updated_data, **kwargs)
@@ -575,6 +575,8 @@ def delete(self, **kwargs: Any) -> None:
575575
"""
576576
if TYPE_CHECKING:
577577
assert isinstance(self.manager, DeleteMixin)
578+
# NOTE: Don't use `self.encoded_id` here as `self.manager.delete()` will encode
579+
# it.
578580
self.manager.delete(self.get_id(), **kwargs)
579581

580582

@@ -598,7 +600,7 @@ def user_agent_detail(self, **kwargs: Any) -> Dict[str, Any]:
598600
GitlabAuthenticationError: If authentication is not correct
599601
GitlabGetError: If the server cannot perform the request
600602
"""
601-
path = f"{self.manager.path}/{self.get_id()}/user_agent_detail"
603+
path = f"{self.manager.path}/{self.encoded_id}/user_agent_detail"
602604
result = self.manager.gitlab.http_get(path, **kwargs)
603605
if TYPE_CHECKING:
604606
assert not isinstance(result, requests.Response)
@@ -705,7 +707,7 @@ def subscribe(self, **kwargs: Any) -> None:
705707
GitlabAuthenticationError: If authentication is not correct
706708
GitlabSubscribeError: If the subscription cannot be done
707709
"""
708-
path = f"{self.manager.path}/{self.get_id()}/subscribe"
710+
path = f"{self.manager.path}/{self.encoded_id}/subscribe"
709711
server_data = self.manager.gitlab.http_post(path, **kwargs)
710712
if TYPE_CHECKING:
711713
assert not isinstance(server_data, requests.Response)
@@ -725,7 +727,7 @@ def unsubscribe(self, **kwargs: Any) -> None:
725727
GitlabAuthenticationError: If authentication is not correct
726728
GitlabUnsubscribeError: If the unsubscription cannot be done
727729
"""
728-
path = f"{self.manager.path}/{self.get_id()}/unsubscribe"
730+
path = f"{self.manager.path}/{self.encoded_id}/unsubscribe"
729731
server_data = self.manager.gitlab.http_post(path, **kwargs)
730732
if TYPE_CHECKING:
731733
assert not isinstance(server_data, requests.Response)
@@ -752,7 +754,7 @@ def todo(self, **kwargs: Any) -> None:
752754
GitlabAuthenticationError: If authentication is not correct
753755
GitlabTodoError: If the todo cannot be set
754756
"""
755-
path = f"{self.manager.path}/{self.get_id()}/todo"
757+
path = f"{self.manager.path}/{self.encoded_id}/todo"
756758
self.manager.gitlab.http_post(path, **kwargs)
757759

758760

@@ -781,7 +783,7 @@ def time_stats(self, **kwargs: Any) -> Dict[str, Any]:
781783
if "time_stats" in self.attributes:
782784
return self.attributes["time_stats"]
783785

784-
path = f"{self.manager.path}/{self.get_id()}/time_stats"
786+
path = f"{self.manager.path}/{self.encoded_id}/time_stats"
785787
result = self.manager.gitlab.http_get(path, **kwargs)
786788
if TYPE_CHECKING:
787789
assert not isinstance(result, requests.Response)
@@ -800,7 +802,7 @@ def time_estimate(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
800802
GitlabAuthenticationError: If authentication is not correct
801803
GitlabTimeTrackingError: If the time tracking update cannot be done
802804
"""
803-
path = f"{self.manager.path}/{self.get_id()}/time_estimate"
805+
path = f"{self.manager.path}/{self.encoded_id}/time_estimate"
804806
data = {"duration": duration}
805807
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
806808
if TYPE_CHECKING:
@@ -819,7 +821,7 @@ def reset_time_estimate(self, **kwargs: Any) -> Dict[str, Any]:
819821
GitlabAuthenticationError: If authentication is not correct
820822
GitlabTimeTrackingError: If the time tracking update cannot be done
821823
"""
822-
path = f"{self.manager.path}/{self.get_id()}/reset_time_estimate"
824+
path = f"{self.manager.path}/{self.encoded_id}/reset_time_estimate"
823825
result = self.manager.gitlab.http_post(path, **kwargs)
824826
if TYPE_CHECKING:
825827
assert not isinstance(result, requests.Response)
@@ -838,7 +840,7 @@ def add_spent_time(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
838840
GitlabAuthenticationError: If authentication is not correct
839841
GitlabTimeTrackingError: If the time tracking update cannot be done
840842
"""
841-
path = f"{self.manager.path}/{self.get_id()}/add_spent_time"
843+
path = f"{self.manager.path}/{self.encoded_id}/add_spent_time"
842844
data = {"duration": duration}
843845
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
844846
if TYPE_CHECKING:
@@ -857,7 +859,7 @@ def reset_spent_time(self, **kwargs: Any) -> Dict[str, Any]:
857859
GitlabAuthenticationError: If authentication is not correct
858860
GitlabTimeTrackingError: If the time tracking update cannot be done
859861
"""
860-
path = f"{self.manager.path}/{self.get_id()}/reset_spent_time"
862+
path = f"{self.manager.path}/{self.encoded_id}/reset_spent_time"
861863
result = self.manager.gitlab.http_post(path, **kwargs)
862864
if TYPE_CHECKING:
863865
assert not isinstance(result, requests.Response)
@@ -893,7 +895,7 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
893895
The list of participants
894896
"""
895897

896-
path = f"{self.manager.path}/{self.get_id()}/participants"
898+
path = f"{self.manager.path}/{self.encoded_id}/participants"
897899
result = self.manager.gitlab.http_get(path, **kwargs)
898900
if TYPE_CHECKING:
899901
assert not isinstance(result, requests.Response)

‎gitlab/v4/objects/commits.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def diff(self, **kwargs: Any) -> Union[gitlab.GitlabList, List[Dict[str, Any]]]:
4242
Returns:
4343
The changes done in this commit
4444
"""
45-
path = f"{self.manager.path}/{self.get_id()}/diff"
45+
path = f"{self.manager.path}/{self.encoded_id}/diff"
4646
return self.manager.gitlab.http_list(path, **kwargs)
4747

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

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

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

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

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

147147

‎gitlab/v4/objects/environments.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def stop(self, **kwargs: Any) -> Union[Dict[str, Any], requests.Response]:
3636
Returns:
3737
A dict of the result.
3838
"""
39-
path = f"{self.manager.path}/{self.get_id()}/stop"
39+
path = f"{self.manager.path}/{self.encoded_id}/stop"
4040
return self.manager.gitlab.http_post(path, **kwargs)
4141

4242

‎gitlab/v4/objects/epics.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ def save(self, **kwargs: Any) -> None:
7272
return
7373

7474
# call the manager
75-
obj_id = self.get_id()
75+
obj_id = self.encoded_id
7676
self.manager.update(obj_id, updated_data, **kwargs)
7777

7878

‎gitlab/v4/objects/files.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def delete( # type: ignore
7676
GitlabAuthenticationError: If authentication is not correct
7777
GitlabDeleteError: If the server cannot perform the request
7878
"""
79-
file_path = utils._url_encode(self.get_id())
79+
file_path = self.encoded_id
8080
self.manager.delete(file_path, branch, commit_message, **kwargs)
8181

8282

‎gitlab/v4/objects/geo_nodes.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def repair(self, **kwargs: Any) -> None:
3030
GitlabAuthenticationError: If authentication is not correct
3131
GitlabRepairError: If the server failed to perform the request
3232
"""
33-
path = f"/geo_nodes/{self.get_id()}/repair"
33+
path = f"/geo_nodes/{self.encoded_id}/repair"
3434
server_data = self.manager.gitlab.http_post(path, **kwargs)
3535
if TYPE_CHECKING:
3636
assert isinstance(server_data, dict)
@@ -51,7 +51,7 @@ def status(self, **kwargs: Any) -> Dict[str, Any]:
5151
Returns:
5252
The status of the geo node
5353
"""
54-
path = f"/geo_nodes/{self.get_id()}/status"
54+
path = f"/geo_nodes/{self.encoded_id}/status"
5555
result = self.manager.gitlab.http_get(path, **kwargs)
5656
if TYPE_CHECKING:
5757
assert isinstance(result, dict)

‎gitlab/v4/objects/groups.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ def search(
113113
A list of dicts describing the resources found.
114114
"""
115115
data = {"scope": scope, "search": search}
116-
path = f"/groups/{self.get_id()}/search"
116+
path = f"/groups/{self.encoded_id}/search"
117117
return self.manager.gitlab.http_list(path, query_data=data, **kwargs)
118118

119119
@cli.register_custom_action("Group", ("cn", "group_access", "provider"))
@@ -134,7 +134,7 @@ def add_ldap_group_link(
134134
GitlabAuthenticationError: If authentication is not correct
135135
GitlabCreateError: If the server cannot perform the request
136136
"""
137-
path = f"/groups/{self.get_id()}/ldap_group_links"
137+
path = f"/groups/{self.encoded_id}/ldap_group_links"
138138
data = {"cn": cn, "group_access": group_access, "provider": provider}
139139
self.manager.gitlab.http_post(path, post_data=data, **kwargs)
140140

@@ -154,7 +154,7 @@ def delete_ldap_group_link(
154154
GitlabAuthenticationError: If authentication is not correct
155155
GitlabDeleteError: If the server cannot perform the request
156156
"""
157-
path = f"/groups/{self.get_id()}/ldap_group_links"
157+
path = f"/groups/{self.encoded_id}/ldap_group_links"
158158
if provider is not None:
159159
path += f"/{provider}"
160160
path += f"/{cn}"
@@ -172,7 +172,7 @@ def ldap_sync(self, **kwargs: Any) -> None:
172172
GitlabAuthenticationError: If authentication is not correct
173173
GitlabCreateError: If the server cannot perform the request
174174
"""
175-
path = f"/groups/{self.get_id()}/ldap_sync"
175+
path = f"/groups/{self.encoded_id}/ldap_sync"
176176
self.manager.gitlab.http_post(path, **kwargs)
177177

178178
@cli.register_custom_action("Group", ("group_id", "group_access"), ("expires_at",))
@@ -198,7 +198,7 @@ def share(
198198
Returns:
199199
Group
200200
"""
201-
path = f"/groups/{self.get_id()}/share"
201+
path = f"/groups/{self.encoded_id}/share"
202202
data = {
203203
"group_id": group_id,
204204
"group_access": group_access,
@@ -222,7 +222,7 @@ def unshare(self, group_id: int, **kwargs: Any) -> None:
222222
GitlabAuthenticationError: If authentication is not correct
223223
GitlabDeleteError: If the server failed to perform the request
224224
"""
225-
path = f"/groups/{self.get_id()}/share/{group_id}"
225+
path = f"/groups/{self.encoded_id}/share/{group_id}"
226226
self.manager.gitlab.http_delete(path, **kwargs)
227227

228228

‎gitlab/v4/objects/issues.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ def move(self, to_project_id: int, **kwargs: Any) -> None:
132132
GitlabAuthenticationError: If authentication is not correct
133133
GitlabUpdateError: If the issue could not be moved
134134
"""
135-
path = f"{self.manager.path}/{self.get_id()}/move"
135+
path = f"{self.manager.path}/{self.encoded_id}/move"
136136
data = {"to_project_id": to_project_id}
137137
server_data = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
138138
if TYPE_CHECKING:
@@ -154,7 +154,7 @@ def related_merge_requests(self, **kwargs: Any) -> Dict[str, Any]:
154154
Returns:
155155
The list of merge requests.
156156
"""
157-
path = f"{self.manager.path}/{self.get_id()}/related_merge_requests"
157+
path = f"{self.manager.path}/{self.encoded_id}/related_merge_requests"
158158
result = self.manager.gitlab.http_get(path, **kwargs)
159159
if TYPE_CHECKING:
160160
assert isinstance(result, dict)
@@ -175,7 +175,7 @@ def closed_by(self, **kwargs: Any) -> Dict[str, Any]:
175175
Returns:
176176
The list of merge requests.
177177
"""
178-
path = f"{self.manager.path}/{self.get_id()}/closed_by"
178+
path = f"{self.manager.path}/{self.encoded_id}/closed_by"
179179
result = self.manager.gitlab.http_get(path, **kwargs)
180180
if TYPE_CHECKING:
181181
assert isinstance(result, dict)

‎gitlab/v4/objects/jobs.py

+9-9
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def cancel(self, **kwargs: Any) -> Dict[str, Any]:
2727
GitlabAuthenticationError: If authentication is not correct
2828
GitlabJobCancelError: If the job could not be canceled
2929
"""
30-
path = f"{self.manager.path}/{self.get_id()}/cancel"
30+
path = f"{self.manager.path}/{self.encoded_id}/cancel"
3131
result = self.manager.gitlab.http_post(path)
3232
if TYPE_CHECKING:
3333
assert isinstance(result, dict)
@@ -45,7 +45,7 @@ def retry(self, **kwargs: Any) -> Dict[str, Any]:
4545
GitlabAuthenticationError: If authentication is not correct
4646
GitlabJobRetryError: If the job could not be retried
4747
"""
48-
path = f"{self.manager.path}/{self.get_id()}/retry"
48+
path = f"{self.manager.path}/{self.encoded_id}/retry"
4949
result = self.manager.gitlab.http_post(path)
5050
if TYPE_CHECKING:
5151
assert isinstance(result, dict)
@@ -63,7 +63,7 @@ def play(self, **kwargs: Any) -> None:
6363
GitlabAuthenticationError: If authentication is not correct
6464
GitlabJobPlayError: If the job could not be triggered
6565
"""
66-
path = f"{self.manager.path}/{self.get_id()}/play"
66+
path = f"{self.manager.path}/{self.encoded_id}/play"
6767
self.manager.gitlab.http_post(path)
6868

6969
@cli.register_custom_action("ProjectJob")
@@ -78,7 +78,7 @@ def erase(self, **kwargs: Any) -> None:
7878
GitlabAuthenticationError: If authentication is not correct
7979
GitlabJobEraseError: If the job could not be erased
8080
"""
81-
path = f"{self.manager.path}/{self.get_id()}/erase"
81+
path = f"{self.manager.path}/{self.encoded_id}/erase"
8282
self.manager.gitlab.http_post(path)
8383

8484
@cli.register_custom_action("ProjectJob")
@@ -93,7 +93,7 @@ def keep_artifacts(self, **kwargs: Any) -> None:
9393
GitlabAuthenticationError: If authentication is not correct
9494
GitlabCreateError: If the request could not be performed
9595
"""
96-
path = f"{self.manager.path}/{self.get_id()}/artifacts/keep"
96+
path = f"{self.manager.path}/{self.encoded_id}/artifacts/keep"
9797
self.manager.gitlab.http_post(path)
9898

9999
@cli.register_custom_action("ProjectJob")
@@ -108,7 +108,7 @@ def delete_artifacts(self, **kwargs: Any) -> None:
108108
GitlabAuthenticationError: If authentication is not correct
109109
GitlabDeleteError: If the request could not be performed
110110
"""
111-
path = f"{self.manager.path}/{self.get_id()}/artifacts"
111+
path = f"{self.manager.path}/{self.encoded_id}/artifacts"
112112
self.manager.gitlab.http_delete(path)
113113

114114
@cli.register_custom_action("ProjectJob")
@@ -138,7 +138,7 @@ def artifacts(
138138
Returns:
139139
The artifacts if `streamed` is False, None otherwise.
140140
"""
141-
path = f"{self.manager.path}/{self.get_id()}/artifacts"
141+
path = f"{self.manager.path}/{self.encoded_id}/artifacts"
142142
result = self.manager.gitlab.http_get(
143143
path, streamed=streamed, raw=True, **kwargs
144144
)
@@ -175,7 +175,7 @@ def artifact(
175175
Returns:
176176
The artifacts if `streamed` is False, None otherwise.
177177
"""
178-
path = f"{self.manager.path}/{self.get_id()}/artifacts/{path}"
178+
path = f"{self.manager.path}/{self.encoded_id}/artifacts/{path}"
179179
result = self.manager.gitlab.http_get(
180180
path, streamed=streamed, raw=True, **kwargs
181181
)
@@ -210,7 +210,7 @@ def trace(
210210
Returns:
211211
The trace
212212
"""
213-
path = f"{self.manager.path}/{self.get_id()}/trace"
213+
path = f"{self.manager.path}/{self.encoded_id}/trace"
214214
result = self.manager.gitlab.http_get(
215215
path, streamed=streamed, raw=True, **kwargs
216216
)

0 commit comments

Comments
 (0)