From 702e41dd0674e76b292d9ea4f559c86f0a99edfe Mon Sep 17 00:00:00 2001
From: "John L. Villalovos" <john@sodarock.com>
Date: Mon, 20 Dec 2021 14:24:17 -0800
Subject: [PATCH] fix: stop encoding '.' to '%2E'
Forcing the encoding of '.' to '%2E' causes issues. It also goes
against the RFC:
https://datatracker.ietf.org/doc/html/rfc3986.html#section-2.3
From the RFC:
For consistency, percent-encoded octets in the ranges of ALPHA
(%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E),
underscore (%5F), or tilde (%7E) should not be created by URI
producers...
Closes #1006
Related #1356
Related #1561
BREAKING CHANGE: stop encoding '.' to '%2E'. This could potentially be
a breaking change for users who have incorrectly configured GitLab
servers which don't handle period '.' characters correctly.
---
gitlab/client.py | 27 ++++++++++---------------
gitlab/utils.py | 8 +-------
tests/unit/objects/test_packages.py | 8 +++-----
tests/unit/objects/test_releases.py | 11 ++++------
tests/unit/objects/test_repositories.py | 3 +--
tests/unit/test_utils.py | 10 ---------
6 files changed, 20 insertions(+), 47 deletions(-)
diff --git a/gitlab/client.py b/gitlab/client.py
index d3fdaab4e..e61fb9703 100644
--- a/gitlab/client.py
+++ b/gitlab/client.py
@@ -593,24 +593,19 @@ def http_request(
json, data, content_type = self._prepare_send_data(files, post_data, raw)
opts["headers"]["Content-type"] = content_type
- # Requests assumes that `.` should not be encoded as %2E and will make
- # changes to urls using this encoding. Using a prepped request we can
- # get the desired behavior.
- # The Requests behavior is right but it seems that web servers don't
- # always agree with this decision (this is the case with a default
- # gitlab installation)
- req = requests.Request(verb, url, json=json, data=data, params=params, **opts)
- prepped = self.session.prepare_request(req)
- if TYPE_CHECKING:
- assert prepped.url is not None
- prepped.url = utils.sanitized_url(prepped.url)
- settings = self.session.merge_environment_settings(
- prepped.url, {}, streamed, verify, None
- )
-
cur_retries = 0
while True:
- result = self.session.send(prepped, timeout=timeout, **settings)
+ result = self.session.request(
+ method=verb,
+ url=url,
+ json=json,
+ data=data,
+ params=params,
+ timeout=timeout,
+ verify=verify,
+ stream=streamed,
+ **opts,
+ )
self._check_redirects(result)
diff --git a/gitlab/utils.py b/gitlab/utils.py
index 220a8c904..a1dcb4511 100644
--- a/gitlab/utils.py
+++ b/gitlab/utils.py
@@ -16,7 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from typing import Any, Callable, Dict, Optional
-from urllib.parse import quote, urlparse
+from urllib.parse import quote
import requests
@@ -60,11 +60,5 @@ def clean_str_id(id: str) -> str:
return quote(id, safe="")
-def sanitized_url(url: str) -> str:
- parsed = urlparse(url)
- new_path = parsed.path.replace(".", "%2E")
- return parsed._replace(path=new_path).geturl()
-
-
def remove_none_from_dict(data: Dict[str, Any]) -> Dict[str, Any]:
return {k: v for k, v in data.items() if v is not None}
diff --git a/tests/unit/objects/test_packages.py b/tests/unit/objects/test_packages.py
index 13f33f7ba..e57aea68a 100644
--- a/tests/unit/objects/test_packages.py
+++ b/tests/unit/objects/test_packages.py
@@ -2,7 +2,6 @@
GitLab API: https://docs.gitlab.com/ce/api/packages.html
"""
import re
-from urllib.parse import quote_plus
import pytest
import responses
@@ -109,10 +108,9 @@
file_name = "hello.tar.gz"
file_content = "package content"
package_url = "http://localhost/api/v4/projects/1/packages/generic/{}/{}/{}".format(
- # https://datatracker.ietf.org/doc/html/rfc3986.html#section-2.3 :(
- quote_plus(package_name).replace(".", "%2E"),
- quote_plus(package_version).replace(".", "%2E"),
- quote_plus(file_name).replace(".", "%2E"),
+ package_name,
+ package_version,
+ file_name,
)
diff --git a/tests/unit/objects/test_releases.py b/tests/unit/objects/test_releases.py
index 58ab5d07b..3a4cee533 100644
--- a/tests/unit/objects/test_releases.py
+++ b/tests/unit/objects/test_releases.py
@@ -11,13 +11,12 @@
from gitlab.v4.objects import ProjectReleaseLink
tag_name = "v1.0.0"
-encoded_tag_name = "v1%2E0%2E0"
release_name = "demo-release"
release_description = "my-rel-desc"
released_at = "2019-03-15T08:00:00Z"
link_name = "hello-world"
link_url = "https://gitlab.example.com/group/hello/-/jobs/688/artifacts/raw/bin/hello-darwin-amd64"
-direct_url = f"https://gitlab.example.com/group/hello/-/releases/{encoded_tag_name}/downloads/hello-world"
+direct_url = f"https://gitlab.example.com/group/hello/-/releases/{tag_name}/downloads/hello-world"
new_link_type = "package"
link_content = {
"id": 2,
@@ -37,14 +36,12 @@
"released_at": released_at,
}
-release_url = re.compile(
- rf"http://localhost/api/v4/projects/1/releases/{encoded_tag_name}"
-)
+release_url = re.compile(rf"http://localhost/api/v4/projects/1/releases/{tag_name}")
links_url = re.compile(
- rf"http://localhost/api/v4/projects/1/releases/{encoded_tag_name}/assets/links"
+ rf"http://localhost/api/v4/projects/1/releases/{tag_name}/assets/links"
)
link_id_url = re.compile(
- rf"http://localhost/api/v4/projects/1/releases/{encoded_tag_name}/assets/links/1"
+ rf"http://localhost/api/v4/projects/1/releases/{tag_name}/assets/links/1"
)
diff --git a/tests/unit/objects/test_repositories.py b/tests/unit/objects/test_repositories.py
index 7c4d77d4f..ff2bc2335 100644
--- a/tests/unit/objects/test_repositories.py
+++ b/tests/unit/objects/test_repositories.py
@@ -29,8 +29,7 @@ def resp_get_repository_file():
"last_commit_id": "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
}
- # requests also encodes `.`
- encoded_path = quote(file_path, safe="").replace(".", "%2E")
+ encoded_path = quote(file_path, safe="")
with responses.RequestsMock() as rsps:
rsps.add(
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index dbe08380f..706285ed8 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -30,13 +30,3 @@ def test_clean_str_id():
src = "foo%bar/baz/"
dest = "foo%25bar%2Fbaz%2F"
assert dest == utils.clean_str_id(src)
-
-
-def test_sanitized_url():
- src = "http://localhost/foo/bar"
- dest = "http://localhost/foo/bar"
- assert dest == utils.sanitized_url(src)
-
- src = "http://localhost/foo.bar.baz"
- dest = "http://localhost/foo%2Ebar%2Ebaz"
- assert dest == utils.sanitized_url(src)