Skip to content

Commit bc48840

Browse files
nejchnejch
authored
Merge pull request #1819 from python-gitlab/jlvillal/encoded_id
fix: use url-encoded ID in all paths
2 parents 58e5b25 + b07eece commit bc48840

26 files changed

+267
-134
lines changed

‎gitlab/base.py

+9
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.EncodedId(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

+20-22
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ 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+
if isinstance(id, str):
103+
id = utils.EncodedId(id)
104104
path = f"{self.path}/{id}"
105105
if TYPE_CHECKING:
106106
assert self._obj_cls is not None
@@ -173,7 +173,7 @@ def refresh(self, **kwargs: Any) -> None:
173173
GitlabGetError: If the server cannot perform the request
174174
"""
175175
if self._id_attr:
176-
path = f"{self.manager.path}/{self.id}"
176+
path = f"{self.manager.path}/{self.encoded_id}"
177177
else:
178178
if TYPE_CHECKING:
179179
assert self.manager.path is not None
@@ -391,7 +391,7 @@ def update(
391391
if id is None:
392392
path = self.path
393393
else:
394-
path = f"{self.path}/{id}"
394+
path = f"{self.path}/{utils.EncodedId(id)}"
395395

396396
self._check_missing_update_attrs(new_data)
397397
files = {}
@@ -444,7 +444,7 @@ def set(self, key: str, value: str, **kwargs: Any) -> base.RESTObject:
444444
Returns:
445445
The created/updated attribute
446446
"""
447-
path = f"{self.path}/{utils._url_encode(key)}"
447+
path = f"{self.path}/{utils.EncodedId(key)}"
448448
data = {"value": value}
449449
server_data = self.gitlab.http_put(path, post_data=data, **kwargs)
450450
if TYPE_CHECKING:
@@ -477,9 +477,7 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
477477
if id is None:
478478
path = self.path
479479
else:
480-
if not isinstance(id, int):
481-
id = utils._url_encode(id)
482-
path = f"{self.path}/{id}"
480+
path = f"{self.path}/{utils.EncodedId(id)}"
483481
self.gitlab.http_delete(path, **kwargs)
484482

485483

@@ -545,7 +543,7 @@ def save(self, **kwargs: Any) -> None:
545543
return
546544

547545
# call the manager
548-
obj_id = self.get_id()
546+
obj_id = self.encoded_id
549547
if TYPE_CHECKING:
550548
assert isinstance(self.manager, UpdateMixin)
551549
server_data = self.manager.update(obj_id, updated_data, **kwargs)
@@ -575,7 +573,7 @@ def delete(self, **kwargs: Any) -> None:
575573
"""
576574
if TYPE_CHECKING:
577575
assert isinstance(self.manager, DeleteMixin)
578-
self.manager.delete(self.get_id(), **kwargs)
576+
self.manager.delete(self.encoded_id, **kwargs)
579577

580578

581579
class UserAgentDetailMixin(_RestObjectBase):
@@ -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

+22-15
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,32 @@ 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.
63-
64-
https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
65-
66-
If using namespaced API requests, make sure that the NAMESPACE/PROJECT_PATH is
67-
URL-encoded. For example, / is represented by %2F
62+
* Using it recursively will only url-encode the value once.
63+
* Can accept either `str` or `int` as input value.
64+
* Can be used in an f-string and output the URL-encoded string.
6865
69-
https://docs.gitlab.com/ee/api/index.html#path-parameters
66+
Reference to documentation on why this is necessary.
7067
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.
68+
See::
7569
70+
https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding
71+
https://docs.gitlab.com/ee/api/index.html#path-parameters
7672
"""
77-
return urllib.parse.quote(id, safe="")
73+
74+
# mypy complains if return type other than the class type. So we ignore issue.
75+
def __new__( # type: ignore
76+
cls, value: Union[str, int, "EncodedId"]
77+
) -> Union[int, "EncodedId"]:
78+
if isinstance(value, (int, EncodedId)):
79+
return value
80+
81+
if not isinstance(value, str):
82+
raise TypeError(f"Unsupported type received: {type(value)}")
83+
value = urllib.parse.quote(value, safe="")
84+
return super().__new__(cls, value)
7885

7986

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

‎gitlab/v4/cli.py

+1-1
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] = 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

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def set(
5252
Returns:
5353
The created/updated attribute
5454
"""
55-
name = utils._url_encode(name)
55+
name = utils.EncodedId(name)
5656
path = f"{self.path}/{name}"
5757
data = {
5858
"value": value,

‎gitlab/v4/objects/files.py

+7-7
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 = 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,7 +173,7 @@ def update( # type: ignore
173173
"""
174174
new_data = new_data or {}
175175
data = new_data.copy()
176-
file_path = utils._url_encode(file_path)
176+
file_path = utils.EncodedId(file_path)
177177
data["file_path"] = file_path
178178
path = f"{self.path}/{file_path}"
179179
self._check_missing_update_attrs(data)
@@ -203,7 +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)
206+
file_path = utils.EncodedId(file_path)
207207
path = f"{self.path}/{file_path}"
208208
data = {"branch": branch, "commit_message": commit_message}
209209
self.gitlab.http_delete(path, query_data=data, **kwargs)
@@ -239,7 +239,7 @@ def raw(
239239
Returns:
240240
The file content
241241
"""
242-
file_path = utils._url_encode(file_path)
242+
file_path = utils.EncodedId(file_path)
243243
path = f"{self.path}/{file_path}/raw"
244244
query_data = {"ref": ref}
245245
result = self.gitlab.http_get(
@@ -266,7 +266,7 @@ def blame(self, file_path: str, ref: str, **kwargs: Any) -> List[Dict[str, Any]]
266266
Returns:
267267
A list of commits/lines matching the file
268268
"""
269-
file_path = utils._url_encode(file_path)
269+
file_path = utils.EncodedId(file_path)
270270
path = f"{self.path}/{file_path}/blame"
271271
query_data = {"ref": ref}
272272
result = self.gitlab.http_list(path, query_data, **kwargs)

0 commit comments

Comments
 (0)