Skip to content

Commit 3d49e5e

Browse files
JohnVillalovosJohnVillalovos
committed
fix: remove custom URL encoding
We were using `str.replace()` calls to take care of URL encoding issues. Switch them to use our `utils._url_encode()` function which itself uses `urllib.parse.quote()` Closes: #1356
1 parent 0dba899 commit 3d49e5e

File tree

8 files changed

+50
-22
lines changed

8 files changed

+50
-22
lines changed

‎gitlab/mixins.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ def get(
100100
GitlabGetError: If the server cannot perform the request
101101
"""
102102
if not isinstance(id, int):
103-
id = utils.clean_str_id(id)
103+
id = utils._url_encode(id)
104104
path = f"{self.path}/{id}"
105105
if TYPE_CHECKING:
106106
assert self._obj_cls is not None
@@ -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.clean_str_id(key)}"
447+
path = f"{self.path}/{utils._url_encode(key)}"
448448
data = {"value": value}
449449
server_data = self.gitlab.http_put(path, post_data=data, **kwargs)
450450
if TYPE_CHECKING:
@@ -478,7 +478,7 @@ def delete(self, id: Union[str, int], **kwargs: Any) -> None:
478478
path = self.path
479479
else:
480480
if not isinstance(id, int):
481-
id = utils.clean_str_id(id)
481+
id = utils._url_encode(id)
482482
path = f"{self.path}/{id}"
483483
self.gitlab.http_delete(path, **kwargs)
484484

‎gitlab/utils.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
# You should have received a copy of the GNU Lesser General Public License
1616
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1717

18+
import urllib.parse
1819
from typing import Any, Callable, Dict, Optional
19-
from urllib.parse import quote
2020

2121
import requests
2222

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

5858

59-
def clean_str_id(id: str) -> str:
60-
return quote(id, safe="")
59+
def _url_encode(id: str) -> str:
60+
"""Encode/quote the characters in the string so that they can be used in a path.
61+
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
68+
69+
https://docs.gitlab.com/ee/api/index.html#path-parameters
70+
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.
75+
76+
"""
77+
return urllib.parse.quote(id, safe="")
6178

6279

6380
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.clean_str_id(self.args[key])
78+
self.parent_args[key] = gitlab.utils._url_encode(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/features.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ def set(
5252
Returns:
5353
The created/updated attribute
5454
"""
55-
path = f"{self.path}/{name.replace('/', '%2F')}"
55+
name = utils._url_encode(name)
56+
path = f"{self.path}/{name}"
5657
data = {
5758
"value": value,
5859
"feature_group": feature_group,

‎gitlab/v4/objects/files.py

Lines changed: 8 additions & 7 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 = self.file_path.replace("/", "%2F")
59+
self.file_path = utils._url_encode(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 = self.get_id().replace("/", "%2F")
79+
file_path = utils._url_encode(self.get_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 = new_data.pop("file_path").replace("/", "%2F")
147+
file_path = utils._url_encode(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 = file_path.replace("/", "%2F")
176+
file_path = utils._url_encode(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,8 @@ def delete( # type: ignore
203203
GitlabAuthenticationError: If authentication is not correct
204204
GitlabDeleteError: If the server cannot perform the request
205205
"""
206-
path = f"{self.path}/{file_path.replace('/', '%2F')}"
206+
file_path = utils._url_encode(file_path)
207+
path = f"{self.path}/{file_path}"
207208
data = {"branch": branch, "commit_message": commit_message}
208209
self.gitlab.http_delete(path, query_data=data, **kwargs)
209210

@@ -238,7 +239,7 @@ def raw(
238239
Returns:
239240
The file content
240241
"""
241-
file_path = file_path.replace("/", "%2F").replace(".", "%2E")
242+
file_path = utils._url_encode(file_path)
242243
path = f"{self.path}/{file_path}/raw"
243244
query_data = {"ref": ref}
244245
result = self.gitlab.http_get(
@@ -265,7 +266,7 @@ def blame(self, file_path: str, ref: str, **kwargs: Any) -> List[Dict[str, Any]]
265266
Returns:
266267
A list of commits/lines matching the file
267268
"""
268-
file_path = file_path.replace("/", "%2F").replace(".", "%2E")
269+
file_path = utils._url_encode(file_path)
269270
path = f"{self.path}/{file_path}/blame"
270271
query_data = {"ref": ref}
271272
result = self.gitlab.http_list(path, query_data, **kwargs)

‎gitlab/v4/objects/repositories.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def update_submodule(
3939
GitlabPutError: If the submodule could not be updated
4040
"""
4141

42-
submodule = submodule.replace("/", "%2F") # .replace('.', '%2E')
42+
submodule = utils._url_encode(submodule)
4343
path = f"/projects/{self.get_id()}/repository/submodules/{submodule}"
4444
data = {"branch": branch, "commit_sha": commit_sha}
4545
if "commit_message" in kwargs:

‎tests/functional/api/test_repository.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import base64
2+
import os
23
import sys
34
import tarfile
45
import time
@@ -13,13 +14,13 @@
1314
def test_repository_files(project):
1415
project.files.create(
1516
{
16-
"file_path": "README",
17+
"file_path": "README.md",
1718
"branch": "main",
1819
"content": "Initial content",
1920
"commit_message": "Initial commit",
2021
}
2122
)
22-
readme = project.files.get(file_path="README", ref="main")
23+
readme = project.files.get(file_path="README.md", ref="main")
2324
readme.content = base64.b64encode(b"Improved README").decode()
2425

2526
time.sleep(2)
@@ -42,6 +43,9 @@ def test_repository_files(project):
4243
blame = project.files.blame(file_path="README.rst", ref="main")
4344
assert blame
4445

46+
raw_file = project.files.raw(file_path="README.rst", ref="main")
47+
assert os.fsdecode(raw_file) == "Initial content"
48+
4549

4650
def test_repository_tree(project):
4751
tree = project.repository_tree()

‎tests/unit/test_utils.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@
1818
from gitlab import utils
1919

2020

21-
def test_clean_str_id():
21+
def test_url_encode():
2222
src = "nothing_special"
2323
dest = "nothing_special"
24-
assert dest == utils.clean_str_id(src)
24+
assert dest == utils._url_encode(src)
2525

2626
src = "foo#bar/baz/"
2727
dest = "foo%23bar%2Fbaz%2F"
28-
assert dest == utils.clean_str_id(src)
28+
assert dest == utils._url_encode(src)
2929

3030
src = "foo%bar/baz/"
3131
dest = "foo%25bar%2Fbaz%2F"
32-
assert dest == utils.clean_str_id(src)
32+
assert dest == utils._url_encode(src)
33+
34+
# periods/dots should not be modified
35+
src = "docs/README.md"
36+
dest = "docs%2FREADME.md"
37+
assert dest == utils._url_encode(src)

0 commit comments

Comments
 (0)