Skip to content

Commit 12435d7

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 824151c commit 12435d7

File tree

19 files changed

+141
-92
lines changed

19 files changed

+141
-92
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 & 19 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,6 +542,7 @@ def save(self, **kwargs: Any) -> None:
545542
return
546543

547544
# call the manager
545+
# Don't use `self.encoded_id` here as `self.manager.update()` will encode it.
548546
obj_id = self.get_id()
549547
if TYPE_CHECKING:
550548
assert isinstance(self.manager, UpdateMixin)
@@ -575,6 +573,7 @@ def delete(self, **kwargs: Any) -> None:
575573
"""
576574
if TYPE_CHECKING:
577575
assert isinstance(self.manager, DeleteMixin)
576+
# Don't use `self.encoded_id` here as `self.manager.delete()` will encode it.
578577
self.manager.delete(self.get_id(), **kwargs)
579578

580579

@@ -598,7 +597,7 @@ def user_agent_detail(self, **kwargs: Any) -> Dict[str, Any]:
598597
GitlabAuthenticationError: If authentication is not correct
599598
GitlabGetError: If the server cannot perform the request
600599
"""
601-
path = f"{self.manager.path}/{self.get_id()}/user_agent_detail"
600+
path = f"{self.manager.path}/{self.encoded_id}/user_agent_detail"
602601
result = self.manager.gitlab.http_get(path, **kwargs)
603602
if TYPE_CHECKING:
604603
assert not isinstance(result, requests.Response)
@@ -631,7 +630,7 @@ def approve(
631630
GitlabUpdateError: If the server fails to perform the request
632631
"""
633632

634-
path = f"{self.manager.path}/{self.id}/approve"
633+
path = f"{self.manager.path}/{self.encoded_id}/approve"
635634
data = {"access_level": access_level}
636635
server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs)
637636
if TYPE_CHECKING:
@@ -705,7 +704,7 @@ def subscribe(self, **kwargs: Any) -> None:
705704
GitlabAuthenticationError: If authentication is not correct
706705
GitlabSubscribeError: If the subscription cannot be done
707706
"""
708-
path = f"{self.manager.path}/{self.get_id()}/subscribe"
707+
path = f"{self.manager.path}/{self.encoded_id}/subscribe"
709708
server_data = self.manager.gitlab.http_post(path, **kwargs)
710709
if TYPE_CHECKING:
711710
assert not isinstance(server_data, requests.Response)
@@ -725,7 +724,7 @@ def unsubscribe(self, **kwargs: Any) -> None:
725724
GitlabAuthenticationError: If authentication is not correct
726725
GitlabUnsubscribeError: If the unsubscription cannot be done
727726
"""
728-
path = f"{self.manager.path}/{self.get_id()}/unsubscribe"
727+
path = f"{self.manager.path}/{self.encoded_id}/unsubscribe"
729728
server_data = self.manager.gitlab.http_post(path, **kwargs)
730729
if TYPE_CHECKING:
731730
assert not isinstance(server_data, requests.Response)
@@ -752,7 +751,7 @@ def todo(self, **kwargs: Any) -> None:
752751
GitlabAuthenticationError: If authentication is not correct
753752
GitlabTodoError: If the todo cannot be set
754753
"""
755-
path = f"{self.manager.path}/{self.get_id()}/todo"
754+
path = f"{self.manager.path}/{self.encoded_id}/todo"
756755
self.manager.gitlab.http_post(path, **kwargs)
757756

758757

@@ -781,7 +780,7 @@ def time_stats(self, **kwargs: Any) -> Dict[str, Any]:
781780
if "time_stats" in self.attributes:
782781
return self.attributes["time_stats"]
783782

784-
path = f"{self.manager.path}/{self.get_id()}/time_stats"
783+
path = f"{self.manager.path}/{self.encoded_id}/time_stats"
785784
result = self.manager.gitlab.http_get(path, **kwargs)
786785
if TYPE_CHECKING:
787786
assert not isinstance(result, requests.Response)
@@ -800,7 +799,7 @@ def time_estimate(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
800799
GitlabAuthenticationError: If authentication is not correct
801800
GitlabTimeTrackingError: If the time tracking update cannot be done
802801
"""
803-
path = f"{self.manager.path}/{self.get_id()}/time_estimate"
802+
path = f"{self.manager.path}/{self.encoded_id}/time_estimate"
804803
data = {"duration": duration}
805804
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
806805
if TYPE_CHECKING:
@@ -819,7 +818,7 @@ def reset_time_estimate(self, **kwargs: Any) -> Dict[str, Any]:
819818
GitlabAuthenticationError: If authentication is not correct
820819
GitlabTimeTrackingError: If the time tracking update cannot be done
821820
"""
822-
path = f"{self.manager.path}/{self.get_id()}/reset_time_estimate"
821+
path = f"{self.manager.path}/{self.encoded_id}/reset_time_estimate"
823822
result = self.manager.gitlab.http_post(path, **kwargs)
824823
if TYPE_CHECKING:
825824
assert not isinstance(result, requests.Response)
@@ -838,7 +837,7 @@ def add_spent_time(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
838837
GitlabAuthenticationError: If authentication is not correct
839838
GitlabTimeTrackingError: If the time tracking update cannot be done
840839
"""
841-
path = f"{self.manager.path}/{self.get_id()}/add_spent_time"
840+
path = f"{self.manager.path}/{self.encoded_id}/add_spent_time"
842841
data = {"duration": duration}
843842
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
844843
if TYPE_CHECKING:
@@ -857,7 +856,7 @@ def reset_spent_time(self, **kwargs: Any) -> Dict[str, Any]:
857856
GitlabAuthenticationError: If authentication is not correct
858857
GitlabTimeTrackingError: If the time tracking update cannot be done
859858
"""
860-
path = f"{self.manager.path}/{self.get_id()}/reset_spent_time"
859+
path = f"{self.manager.path}/{self.encoded_id}/reset_spent_time"
861860
result = self.manager.gitlab.http_post(path, **kwargs)
862861
if TYPE_CHECKING:
863862
assert not isinstance(result, requests.Response)
@@ -893,7 +892,7 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
893892
The list of participants
894893
"""
895894

896-
path = f"{self.manager.path}/{self.get_id()}/participants"
895+
path = f"{self.manager.path}/{self.encoded_id}/participants"
897896
result = self.manager.gitlab.http_get(path, **kwargs)
898897
if TYPE_CHECKING:
899898
assert not isinstance(result, requests.Response)
@@ -967,7 +966,7 @@ def promote(self, **kwargs: Any) -> Dict[str, Any]:
967966
The updated object data (*not* a RESTObject)
968967
"""
969968

970-
path = f"{self.manager.path}/{self.id}/promote"
969+
path = f"{self.manager.path}/{self.encoded_id}/promote"
971970
http_method = self._get_update_method()
972971
result = http_method(path, **kwargs)
973972
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)