Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix with_suffix re-encoding entire name #1468

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/1468.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix entire name being re-encoded when using `with_suffix` -- by :user:`NTFSvolume`.
26 changes: 25 additions & 1 deletion tests/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -1514,6 +1514,30 @@ def test_with_suffix():
assert url2.path == "/a/b.c"


def test_with_suffix_encoded_suffix():
url = URL("http://example.com/a/b")
url2 = url.with_suffix(". c")
assert url2.raw_parts == ("/", "a", "b.%20c")
assert url2.parts == ("/", "a", "b. c")
assert url2.raw_path == "/a/b.%20c"
assert url2.path == "/a/b. c"


def test_with_suffix_encoded_url():
url = URL("http://example.com/a/b c")
url2 = url.with_suffix(". d")
url3 = url.with_suffix(".e")
assert url2.raw_parts == ("/", "a", "b%20c.%20d")
assert url2.parts == ("/", "a", "b c. d")
assert url2.raw_path == "/a/b%20c.%20d"
assert url2.path == "/a/b c. d"

assert url3.raw_parts == ("/", "a", "b%20c.e")
assert url3.parts == ("/", "a", "b c.e")
assert url3.raw_path == "/a/b%20c.e"
assert url3.path == "/a/b c.e"


@pytest.mark.parametrize(
("original_url", "keep_query", "keep_fragment", "expected_url"),
[
Expand Down Expand Up @@ -1641,7 +1665,7 @@ def test_with_suffix_with_slash2():
with pytest.raises(ValueError) as excinfo:
URL("http://example.com/a").with_suffix(".b/.d")
(msg,) = excinfo.value.args
assert msg == "Slash in name is not allowed"
assert msg == "Invalid suffix '.b/.d'"


def test_with_suffix_replace():
Expand Down
20 changes: 18 additions & 2 deletions yarl/_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -1355,15 +1355,31 @@ def with_suffix(
"""
if not isinstance(suffix, str):
raise TypeError("Invalid suffix type")
if suffix and not suffix[0] == "." or suffix == ".":
if suffix and not suffix[0] == "." or suffix == "." or "/" in suffix:
raise ValueError(f"Invalid suffix {suffix!r}")
name = self.raw_name
if not name:
raise ValueError(f"{self!r} has an empty name")
old_suffix = self.raw_suffix
suffix = PATH_QUOTER(suffix)
name = name + suffix if not old_suffix else name[: -len(old_suffix)] + suffix
if name in (".", ".."):
raise ValueError(". and .. values are forbidden")
parts = list(self.raw_parts)
if netloc := self._netloc:
if len(parts) == 1:
parts.append(name)
else:
parts[-1] = name
parts[0] = "" # replace leading '/'
else:
parts[-1] = name
if parts[0] == "/":
parts[0] = "" # replace leading '/'

return self.with_name(name, keep_query=keep_query, keep_fragment=keep_fragment)
query = self._query if keep_query else ""
fragment = self._fragment if keep_fragment else ""
return from_parts(self._scheme, netloc, "/".join(parts), query, fragment)

def join(self, url: "URL") -> "URL":
"""Join URLs
Expand Down
Loading