Skip to content

Commit 54abe80

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 c01b7c4 commit 54abe80

18 files changed

+127
-93
lines changed

‎gitlab/base.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,15 @@ def get_id(self) -> Any:
217217
return None
218218
return getattr(self, self._id_attr)
219219

220+
@property
221+
def encoded_id(self) -> Any:
222+
"""Ensure that the ID is url-encoded so that it can be safely used in a URL
223+
path"""
224+
obj_id = self.get_id()
225+
if isinstance(obj_id, str):
226+
obj_id = gitlab.utils._url_encode(obj_id)
227+
return obj_id
228+
220229
@property
221230
def attributes(self) -> Dict[str, Any]:
222231
d = self.__dict__["_updated_attrs"].copy()

‎gitlab/mixins.py

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ def get(
9999
GitlabAuthenticationError: If authentication is not correct
100100
GitlabGetError: If the server cannot perform the request
101101
"""
102-
if not isinstance(id, int):
103-
id = utils._url_encode(id)
102+
id = utils._url_encode(id)
104103
path = f"{self.path}/{id}"
105104
if TYPE_CHECKING:
106105
assert self._obj_cls is not None
@@ -173,7 +172,7 @@ def refresh(self, **kwargs: Any) -> None:
173172
GitlabGetError: If the server cannot perform the request
174173
"""
175174
if self._id_attr:
176-
path = f"{self.manager.path}/{self.id}"
175+
path = f"{self.manager.path}/{self.encoded_id}"
177176
else:
178177
if TYPE_CHECKING:
179178
assert self.manager.path is not None
@@ -391,7 +390,7 @@ def update(
391390
if id is None:
392391
path = self.path
393392
else:
394-
path = f"{self.path}/{id}"
393+
path = f"{self.path}/{utils._url_encode(id)}"
395394

396395
self._check_missing_update_attrs(new_data)
397396
files = {}
@@ -477,9 +476,7 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
477476
if id is None:
478477
path = self.path
479478
else:
480-
if not isinstance(id, int):
481-
id = utils._url_encode(id)
482-
path = f"{self.path}/{id}"
479+
path = f"{self.path}/{utils._url_encode(id)}"
483480
self.gitlab.http_delete(path, **kwargs)
484481

485482

@@ -545,7 +542,7 @@ def save(self, **kwargs: Any) -> None:
545542
return
546543

547544
# call the manager
548-
obj_id = self.get_id()
545+
obj_id = self.encoded_id
549546
if TYPE_CHECKING:
550547
assert isinstance(self.manager, UpdateMixin)
551548
server_data = self.manager.update(obj_id, updated_data, **kwargs)
@@ -575,6 +572,7 @@ def delete(self, **kwargs: Any) -> None:
575572
"""
576573
if TYPE_CHECKING:
577574
assert isinstance(self.manager, DeleteMixin)
575+
# Don't use `self.encoded_id` here as `self.manager.delete()` will encode it.
578576
self.manager.delete(self.get_id(), **kwargs)
579577

580578

@@ -598,7 +596,7 @@ def user_agent_detail(self, **kwargs: Any) -> Dict[str, Any]:
598596
GitlabAuthenticationError: If authentication is not correct
599597
GitlabGetError: If the server cannot perform the request
600598
"""
601-
path = f"{self.manager.path}/{self.get_id()}/user_agent_detail"
599+
path = f"{self.manager.path}/{self.encoded_id}/user_agent_detail"
602600
result = self.manager.gitlab.http_get(path, **kwargs)
603601
if TYPE_CHECKING:
604602
assert not isinstance(result, requests.Response)
@@ -631,7 +629,7 @@ def approve(
631629
GitlabUpdateError: If the server fails to perform the request
632630
"""
633631

634-
path = f"{self.manager.path}/{self.id}/approve"
632+
path = f"{self.manager.path}/{self.encoded_id}/approve"
635633
data = {"access_level": access_level}
636634
server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs)
637635
if TYPE_CHECKING:
@@ -705,7 +703,7 @@ def subscribe(self, **kwargs: Any) -> None:
705703
GitlabAuthenticationError: If authentication is not correct
706704
GitlabSubscribeError: If the subscription cannot be done
707705
"""
708-
path = f"{self.manager.path}/{self.get_id()}/subscribe"
706+
path = f"{self.manager.path}/{self.encoded_id}/subscribe"
709707
server_data = self.manager.gitlab.http_post(path, **kwargs)
710708
if TYPE_CHECKING:
711709
assert not isinstance(server_data, requests.Response)
@@ -725,7 +723,7 @@ def unsubscribe(self, **kwargs: Any) -> None:
725723
GitlabAuthenticationError: If authentication is not correct
726724
GitlabUnsubscribeError: If the unsubscription cannot be done
727725
"""
728-
path = f"{self.manager.path}/{self.get_id()}/unsubscribe"
726+
path = f"{self.manager.path}/{self.encoded_id}/unsubscribe"
729727
server_data = self.manager.gitlab.http_post(path, **kwargs)
730728
if TYPE_CHECKING:
731729
assert not isinstance(server_data, requests.Response)
@@ -752,7 +750,7 @@ def todo(self, **kwargs: Any) -> None:
752750
GitlabAuthenticationError: If authentication is not correct
753751
GitlabTodoError: If the todo cannot be set
754752
"""
755-
path = f"{self.manager.path}/{self.get_id()}/todo"
753+
path = f"{self.manager.path}/{self.encoded_id}/todo"
756754
self.manager.gitlab.http_post(path, **kwargs)
757755

758756

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

784-
path = f"{self.manager.path}/{self.get_id()}/time_stats"
782+
path = f"{self.manager.path}/{self.encoded_id}/time_stats"
785783
result = self.manager.gitlab.http_get(path, **kwargs)
786784
if TYPE_CHECKING:
787785
assert not isinstance(result, requests.Response)
@@ -800,7 +798,7 @@ def time_estimate(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
800798
GitlabAuthenticationError: If authentication is not correct
801799
GitlabTimeTrackingError: If the time tracking update cannot be done
802800
"""
803-
path = f"{self.manager.path}/{self.get_id()}/time_estimate"
801+
path = f"{self.manager.path}/{self.encoded_id}/time_estimate"
804802
data = {"duration": duration}
805803
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
806804
if TYPE_CHECKING:
@@ -819,7 +817,7 @@ def reset_time_estimate(self, **kwargs: Any) -> Dict[str, Any]:
819817
GitlabAuthenticationError: If authentication is not correct
820818
GitlabTimeTrackingError: If the time tracking update cannot be done
821819
"""
822-
path = f"{self.manager.path}/{self.get_id()}/reset_time_estimate"
820+
path = f"{self.manager.path}/{self.encoded_id}/reset_time_estimate"
823821
result = self.manager.gitlab.http_post(path, **kwargs)
824822
if TYPE_CHECKING:
825823
assert not isinstance(result, requests.Response)
@@ -838,7 +836,7 @@ def add_spent_time(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
838836
GitlabAuthenticationError: If authentication is not correct
839837
GitlabTimeTrackingError: If the time tracking update cannot be done
840838
"""
841-
path = f"{self.manager.path}/{self.get_id()}/add_spent_time"
839+
path = f"{self.manager.path}/{self.encoded_id}/add_spent_time"
842840
data = {"duration": duration}
843841
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
844842
if TYPE_CHECKING:
@@ -857,7 +855,7 @@ def reset_spent_time(self, **kwargs: Any) -> Dict[str, Any]:
857855
GitlabAuthenticationError: If authentication is not correct
858856
GitlabTimeTrackingError: If the time tracking update cannot be done
859857
"""
860-
path = f"{self.manager.path}/{self.get_id()}/reset_spent_time"
858+
path = f"{self.manager.path}/{self.encoded_id}/reset_spent_time"
861859
result = self.manager.gitlab.http_post(path, **kwargs)
862860
if TYPE_CHECKING:
863861
assert not isinstance(result, requests.Response)
@@ -893,7 +891,7 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
893891
The list of participants
894892
"""
895893

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

970-
path = f"{self.manager.path}/{self.id}/promote"
968+
path = f"{self.manager.path}/{self.encoded_id}/promote"
971969
http_method = self._get_update_method()
972970
result = http_method(path, **kwargs)
973971
if TYPE_CHECKING:

‎gitlab/utils.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1717

1818
import urllib.parse
19-
from typing import Any, Callable, Dict, Optional
19+
from typing import Any, Callable, Dict, Optional, overload, Union
2020

2121
import requests
2222

@@ -56,7 +56,17 @@ def copy_dict(dest: Dict[str, Any], src: Dict[str, Any]) -> None:
5656
dest[k] = v
5757

5858

59+
@overload
60+
def _url_encode(id: int) -> int:
61+
...
62+
63+
64+
@overload
5965
def _url_encode(id: str) -> str:
66+
...
67+
68+
69+
def _url_encode(id: Union[int, str]) -> Union[int, str]:
6070
"""Encode/quote the characters in the string so that they can be used in a path.
6171
6272
Reference to documentation on why this is necessary.
@@ -74,6 +84,8 @@ def _url_encode(id: str) -> str:
7484
parameters.
7585
7686
"""
87+
if isinstance(id, int):
88+
return id
7789
return urllib.parse.quote(id, safe="")
7890

7991

‎gitlab/v4/objects/commits.py

Lines changed: 6 additions & 6 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 2 additions & 2 deletions
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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def search(
115115
A list of dicts describing the resources found.
116116
"""
117117
data = {"scope": scope, "search": search}
118-
path = f"/groups/{self.get_id()}/search"
118+
path = f"/groups/{self.encoded_id}/search"
119119
return self.manager.gitlab.http_list(path, query_data=data, **kwargs)
120120

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

@@ -156,7 +156,7 @@ def delete_ldap_group_link(
156156
GitlabAuthenticationError: If authentication is not correct
157157
GitlabDeleteError: If the server cannot perform the request
158158
"""
159-
path = f"/groups/{self.get_id()}/ldap_group_links"
159+
path = f"/groups/{self.encoded_id}/ldap_group_links"
160160
if provider is not None:
161161
path += f"/{provider}"
162162
path += f"/{cn}"
@@ -174,7 +174,7 @@ def ldap_sync(self, **kwargs: Any) -> None:
174174
GitlabAuthenticationError: If authentication is not correct
175175
GitlabCreateError: If the server cannot perform the request
176176
"""
177-
path = f"/groups/{self.get_id()}/ldap_sync"
177+
path = f"/groups/{self.encoded_id}/ldap_sync"
178178
self.manager.gitlab.http_post(path, **kwargs)
179179

180180
@cli.register_custom_action("Group", ("group_id", "group_access"), ("expires_at",))
@@ -200,7 +200,7 @@ def share(
200200
Returns:
201201
Group
202202
"""
203-
path = f"/groups/{self.get_id()}/share"
203+
path = f"/groups/{self.encoded_id}/share"
204204
data = {
205205
"group_id": group_id,
206206
"group_access": group_access,
@@ -224,7 +224,7 @@ def unshare(self, group_id: int, **kwargs: Any) -> None:
224224
GitlabAuthenticationError: If authentication is not correct
225225
GitlabDeleteError: If the server failed to perform the request
226226
"""
227-
path = f"/groups/{self.get_id()}/share/{group_id}"
227+
path = f"/groups/{self.encoded_id}/share/{group_id}"
228228
self.manager.gitlab.http_delete(path, **kwargs)
229229

230230

‎gitlab/v4/objects/issues.py

Lines changed: 3 additions & 3 deletions
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)

0 commit comments

Comments
 (0)