Skip to content

Commit 37dff7e

Browse files
JohnVillalovosJohnVillalovos
committed
fix: use url-encoded ID in all paths ALTERNATE METHOD
An alternative to #1819 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 37dff7e

21 files changed

+207
-134
lines changed

‎gitlab/base.py

Lines changed: 9 additions & 0 deletions
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.EncodedId(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

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +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)
104-
path = f"{self.path}/{id}"
102+
path = f"{self.path}/{utils.EncodedId(id)}"
105103
if TYPE_CHECKING:
106104
assert self._obj_cls is not None
107105
if lazy is True:
@@ -444,7 +442,7 @@ def set(self, key: str, value: str, **kwargs: Any) -> base.RESTObject:
444442
Returns:
445443
The created/updated attribute
446444
"""
447-
path = f"{self.path}/{utils._url_encode(key)}"
445+
path = f"{self.path}/{utils.EncodedId(key)}"
448446
data = {"value": value}
449447
server_data = self.gitlab.http_put(path, post_data=data, **kwargs)
450448
if TYPE_CHECKING:
@@ -477,9 +475,7 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
477475
if id is None:
478476
path = self.path
479477
else:
480-
if not isinstance(id, int):
481-
id = utils._url_encode(id)
482-
path = f"{self.path}/{id}"
478+
path = f"{self.path}/{utils.EncodedId(id)}"
483479
self.gitlab.http_delete(path, **kwargs)
484480

485481

@@ -545,7 +541,7 @@ def save(self, **kwargs: Any) -> None:
545541
return
546542

547543
# call the manager
548-
obj_id = self.get_id()
544+
obj_id = self.encoded_id
549545
if TYPE_CHECKING:
550546
assert isinstance(self.manager, UpdateMixin)
551547
server_data = self.manager.update(obj_id, updated_data, **kwargs)
@@ -575,7 +571,7 @@ def delete(self, **kwargs: Any) -> None:
575571
"""
576572
if TYPE_CHECKING:
577573
assert isinstance(self.manager, DeleteMixin)
578-
self.manager.delete(self.get_id(), **kwargs)
574+
self.manager.delete(self.encoded_id, **kwargs)
579575

580576

581577
class UserAgentDetailMixin(_RestObjectBase):
@@ -598,7 +594,7 @@ def user_agent_detail(self, **kwargs: Any) -> Dict[str, Any]:
598594
GitlabAuthenticationError: If authentication is not correct
599595
GitlabGetError: If the server cannot perform the request
600596
"""
601-
path = f"{self.manager.path}/{self.get_id()}/user_agent_detail"
597+
path = f"{self.manager.path}/{self.encoded_id}/user_agent_detail"
602598
result = self.manager.gitlab.http_get(path, **kwargs)
603599
if TYPE_CHECKING:
604600
assert not isinstance(result, requests.Response)
@@ -705,7 +701,7 @@ def subscribe(self, **kwargs: Any) -> None:
705701
GitlabAuthenticationError: If authentication is not correct
706702
GitlabSubscribeError: If the subscription cannot be done
707703
"""
708-
path = f"{self.manager.path}/{self.get_id()}/subscribe"
704+
path = f"{self.manager.path}/{self.encoded_id}/subscribe"
709705
server_data = self.manager.gitlab.http_post(path, **kwargs)
710706
if TYPE_CHECKING:
711707
assert not isinstance(server_data, requests.Response)
@@ -725,7 +721,7 @@ def unsubscribe(self, **kwargs: Any) -> None:
725721
GitlabAuthenticationError: If authentication is not correct
726722
GitlabUnsubscribeError: If the unsubscription cannot be done
727723
"""
728-
path = f"{self.manager.path}/{self.get_id()}/unsubscribe"
724+
path = f"{self.manager.path}/{self.encoded_id}/unsubscribe"
729725
server_data = self.manager.gitlab.http_post(path, **kwargs)
730726
if TYPE_CHECKING:
731727
assert not isinstance(server_data, requests.Response)
@@ -752,7 +748,7 @@ def todo(self, **kwargs: Any) -> None:
752748
GitlabAuthenticationError: If authentication is not correct
753749
GitlabTodoError: If the todo cannot be set
754750
"""
755-
path = f"{self.manager.path}/{self.get_id()}/todo"
751+
path = f"{self.manager.path}/{self.encoded_id}/todo"
756752
self.manager.gitlab.http_post(path, **kwargs)
757753

758754

@@ -781,7 +777,7 @@ def time_stats(self, **kwargs: Any) -> Dict[str, Any]:
781777
if "time_stats" in self.attributes:
782778
return self.attributes["time_stats"]
783779

784-
path = f"{self.manager.path}/{self.get_id()}/time_stats"
780+
path = f"{self.manager.path}/{self.encoded_id}/time_stats"
785781
result = self.manager.gitlab.http_get(path, **kwargs)
786782
if TYPE_CHECKING:
787783
assert not isinstance(result, requests.Response)
@@ -800,7 +796,7 @@ def time_estimate(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
800796
GitlabAuthenticationError: If authentication is not correct
801797
GitlabTimeTrackingError: If the time tracking update cannot be done
802798
"""
803-
path = f"{self.manager.path}/{self.get_id()}/time_estimate"
799+
path = f"{self.manager.path}/{self.encoded_id}/time_estimate"
804800
data = {"duration": duration}
805801
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
806802
if TYPE_CHECKING:
@@ -819,7 +815,7 @@ def reset_time_estimate(self, **kwargs: Any) -> Dict[str, Any]:
819815
GitlabAuthenticationError: If authentication is not correct
820816
GitlabTimeTrackingError: If the time tracking update cannot be done
821817
"""
822-
path = f"{self.manager.path}/{self.get_id()}/reset_time_estimate"
818+
path = f"{self.manager.path}/{self.encoded_id}/reset_time_estimate"
823819
result = self.manager.gitlab.http_post(path, **kwargs)
824820
if TYPE_CHECKING:
825821
assert not isinstance(result, requests.Response)
@@ -838,7 +834,7 @@ def add_spent_time(self, duration: str, **kwargs: Any) -> Dict[str, Any]:
838834
GitlabAuthenticationError: If authentication is not correct
839835
GitlabTimeTrackingError: If the time tracking update cannot be done
840836
"""
841-
path = f"{self.manager.path}/{self.get_id()}/add_spent_time"
837+
path = f"{self.manager.path}/{self.encoded_id}/add_spent_time"
842838
data = {"duration": duration}
843839
result = self.manager.gitlab.http_post(path, post_data=data, **kwargs)
844840
if TYPE_CHECKING:
@@ -857,7 +853,7 @@ def reset_spent_time(self, **kwargs: Any) -> Dict[str, Any]:
857853
GitlabAuthenticationError: If authentication is not correct
858854
GitlabTimeTrackingError: If the time tracking update cannot be done
859855
"""
860-
path = f"{self.manager.path}/{self.get_id()}/reset_spent_time"
856+
path = f"{self.manager.path}/{self.encoded_id}/reset_spent_time"
861857
result = self.manager.gitlab.http_post(path, **kwargs)
862858
if TYPE_CHECKING:
863859
assert not isinstance(result, requests.Response)
@@ -893,7 +889,7 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
893889
The list of participants
894890
"""
895891

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

‎gitlab/utils.py

Lines changed: 40 additions & 14 deletions
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, Union
2020

2121
import requests
2222

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

5858

59-
def _url_encode(id: str) -> str:
60-
"""Encode/quote the characters in the string so that they can be used in a path.
59+
class EncodedId(str):
60+
"""A custom `str` class that will return the URL-encoded value of the string.
6161
62-
Reference to documentation on why this is necessary.
62+
Features:
63+
* Using it recursively will only url-encode the value once.
64+
* Can accept either `str` or `int` as input value.
65+
* Can be used in an f-string and output the URL-encoded string.
6366
64-
https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
67+
Reference to documentation on why this is necessary.
6568
66-
If using namespaced API requests, make sure that the NAMESPACE/PROJECT_PATH is
67-
URL-encoded. For example, / is represented by %2F
69+
https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
6870
69-
https://docs.gitlab.com/ee/api/index.html#path-parameters
71+
If using namespaced API requests, make sure that the NAMESPACE/PROJECT_PATH is
72+
URL-encoded. For example, / is represented by %2F
7073
71-
Path parameters that are required to be URL-encoded must be followed. If not, it
72-
doesn’t match an API endpoint and responds with a 404. If there’s something in front
73-
of the API (for example, Apache), ensure that it doesn’t decode the URL-encoded path
74-
parameters.
74+
https://docs.gitlab.com/ee/api/index.html#path-parameters
7575
76-
"""
77-
return urllib.parse.quote(id, safe="")
76+
Path parameters that are required to be URL-encoded must be followed. If not, it
77+
doesn’t match an API endpoint and responds with a 404. If there’s something in
78+
front of the API (for example, Apache), ensure that it doesn’t decode the
79+
URL-encoded path parameters.
80+
"""
81+
def __init__(self, value: Union[int, str]) -> None:
82+
if isinstance(value, int):
83+
value = str(value)
84+
elif isinstance(value, str):
85+
pass
86+
else:
87+
raise ValueError(f"Unsupported type received: {type(value)}")
88+
self.str_val = value
89+
super().__init__()
90+
91+
def __new__(cls, value: Union[str, int, "EncodedId"]) -> "EncodedId":
92+
if isinstance(value, int):
93+
value = str(value)
94+
elif isinstance(value, str):
95+
pass
96+
elif isinstance(value, EncodedId):
97+
value = value.str_val
98+
else:
99+
raise ValueError(f"Unsupported type received: {type(value)}")
100+
return str.__new__(cls, value)
101+
102+
def __str__(self) -> str:
103+
return urllib.parse.quote(self.str_val, safe="")
78104

79105

80106
def remove_none_from_dict(data: Dict[str, Any]) -> Dict[str, Any]:

‎gitlab/v4/cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def _process_from_parent_attrs(self) -> None:
7575
if key not in self.args:
7676
continue
7777

78-
self.parent_args[key] = gitlab.utils._url_encode(self.args[key])
78+
self.parent_args[key] = str(gitlab.utils.EncodedId(self.args[key]))
7979
# If we don't delete it then it will be added to the URL as a query-string
8080
del self.args[key]
8181

‎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/features.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ def set(
5252
Returns:
5353
The created/updated attribute
5454
"""
55-
name = utils._url_encode(name)
56-
path = f"{self.path}/{name}"
55+
path = f"{self.path}/{utils.EncodedId(name)}"
5756
data = {
5857
"value": value,
5958
"feature_group": feature_group,

‎gitlab/v4/objects/files.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def save( # type: ignore
5656
"""
5757
self.branch = branch
5858
self.commit_message = commit_message
59-
self.file_path = utils._url_encode(self.file_path)
59+
self.file_path = str(utils.EncodedId(self.file_path))
6060
super(ProjectFile, self).save(**kwargs)
6161

6262
@exc.on_http_error(exc.GitlabDeleteError)
@@ -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

@@ -144,7 +144,7 @@ def create(
144144
assert data is not None
145145
self._check_missing_create_attrs(data)
146146
new_data = data.copy()
147-
file_path = utils._url_encode(new_data.pop("file_path"))
147+
file_path = utils.EncodedId(new_data.pop("file_path"))
148148
path = f"{self.path}/{file_path}"
149149
server_data = self.gitlab.http_post(path, post_data=new_data, **kwargs)
150150
if TYPE_CHECKING:
@@ -173,9 +173,9 @@ def update( # type: ignore
173173
"""
174174
new_data = new_data or {}
175175
data = new_data.copy()
176-
file_path = utils._url_encode(file_path)
177-
data["file_path"] = file_path
178-
path = f"{self.path}/{file_path}"
176+
encoded_file_path = utils.EncodedId(file_path)
177+
data["file_path"] = encoded_file_path
178+
path = f"{self.path}/{encoded_file_path}"
179179
self._check_missing_update_attrs(data)
180180
result = self.gitlab.http_put(path, post_data=data, **kwargs)
181181
if TYPE_CHECKING:
@@ -203,8 +203,7 @@ def delete( # type: ignore
203203
GitlabAuthenticationError: If authentication is not correct
204204
GitlabDeleteError: If the server cannot perform the request
205205
"""
206-
file_path = utils._url_encode(file_path)
207-
path = f"{self.path}/{file_path}"
206+
path = f"{self.path}/{utils.EncodedId(file_path)}"
208207
data = {"branch": branch, "commit_message": commit_message}
209208
self.gitlab.http_delete(path, query_data=data, **kwargs)
210209

@@ -239,8 +238,7 @@ def raw(
239238
Returns:
240239
The file content
241240
"""
242-
file_path = utils._url_encode(file_path)
243-
path = f"{self.path}/{file_path}/raw"
241+
path = f"{self.path}/{utils.EncodedId(file_path)}/raw"
244242
query_data = {"ref": ref}
245243
result = self.gitlab.http_get(
246244
path, query_data=query_data, streamed=streamed, raw=True, **kwargs
@@ -266,8 +264,7 @@ def blame(self, file_path: str, ref: str, **kwargs: Any) -> List[Dict[str, Any]]
266264
Returns:
267265
A list of commits/lines matching the file
268266
"""
269-
file_path = utils._url_encode(file_path)
270-
path = f"{self.path}/{file_path}/blame"
267+
path = f"{self.path}/{utils.EncodedId(file_path)}/blame"
271268
query_data = {"ref": ref}
272269
result = self.gitlab.http_list(path, query_data, **kwargs)
273270
if TYPE_CHECKING:

0 commit comments

Comments
 (0)