Skip to content

Commit fd8a747

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 fd8a747

File tree

21 files changed

+205
-132
lines changed

21 files changed

+205
-132
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: 19 additions & 23 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:
@@ -173,7 +171,7 @@ def refresh(self, **kwargs: Any) -> None:
173171
GitlabGetError: If the server cannot perform the request
174172
"""
175173
if self._id_attr:
176-
path = f"{self.manager.path}/{self.id}"
174+
path = f"{self.manager.path}/{self.encoded_id}"
177175
else:
178176
if TYPE_CHECKING:
179177
assert self.manager.path is not None
@@ -391,7 +389,7 @@ def update(
391389
if id is None:
392390
path = self.path
393391
else:
394-
path = f"{self.path}/{id}"
392+
path = f"{self.path}/{utils.EncodedId(id)}"
395393

396394
self._check_missing_update_attrs(new_data)
397395
files = {}
@@ -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)
@@ -631,7 +627,7 @@ def approve(
631627
GitlabUpdateError: If the server fails to perform the request
632628
"""
633629

634-
path = f"{self.manager.path}/{self.id}/approve"
630+
path = f"{self.manager.path}/{self.encoded_id}/approve"
635631
data = {"access_level": access_level}
636632
server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs)
637633
if TYPE_CHECKING:
@@ -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)
@@ -967,7 +963,7 @@ def promote(self, **kwargs: Any) -> Dict[str, Any]:
967963
The updated object data (*not* a RESTObject)
968964
"""
969965

970-
path = f"{self.manager.path}/{self.id}/promote"
966+
path = f"{self.manager.path}/{self.encoded_id}/promote"
971967
http_method = self._get_update_method()
972968
result = http_method(path, **kwargs)
973969
if TYPE_CHECKING:

‎gitlab/utils.py‎

Lines changed: 34 additions & 8 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,8 +56,13 @@ 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.
61+
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.
6166
6267
Reference to documentation on why this is necessary.
6368
@@ -69,12 +74,33 @@ def _url_encode(id: str) -> str:
6974
https://docs.gitlab.com/ee/api/index.html#path-parameters
7075
7176
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.
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)
75101

76-
"""
77-
return urllib.parse.quote(id, safe="")
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,

0 commit comments

Comments
 (0)