Stop unquoting %2F in paths

[Dreamsorcerer]

I've added ignore so that /foo/bar%2fbaz doesn't become %2Ffoo%2Fbar%2Fbaz, but maybe the unsafe option is just wrong currently?

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 62.37%. Comparing base (24073ce) to head (a7c1d64).
Report is 356 commits behind head on master.

Files with missing lines	Patch %	Lines
tests/test_url.py	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1057      +/-   ##
==========================================
+ Coverage   62.36%   62.37%   +0.01%     
==========================================
  Files          38       38              
  Lines        6616     6626      +10     
  Branches      356      356              
==========================================
+ Hits         4126     4133       +7     
- Misses       2463     2466       +3     
  Partials       27       27              

Flag	Coverage Δ
CI-GHA	62.34% <75.00%> (+0.01%)	⬆️
MyPy	25.60% <70.00%> (+0.03%)	⬆️
OS-Linux	99.25% <100.00%> (+<0.01%)	⬆️
OS-Windows	99.58% <100.00%> (+<0.01%)	⬆️
OS-macOS	99.02% <100.00%> (+<0.01%)	⬆️
Py-3.10.11	98.90% <100.00%> (+<0.01%)	⬆️
Py-3.10.14	99.10% <100.00%> (+<0.01%)	⬆️
Py-3.11.9	99.10% <100.00%> (+<0.01%)	⬆️
Py-3.12.4	99.45% <100.00%> (+<0.01%)	⬆️
Py-3.12.5	99.10% <100.00%> (+<0.01%)	⬆️
Py-3.13.0-rc.1	99.10% <100.00%> (+<0.01%)	⬆️
Py-3.8.10	98.87% <100.00%> (+<0.01%)	⬆️
Py-3.8.18	99.07% <100.00%> (+<0.01%)	⬆️
Py-3.9.13	98.87% <100.00%> (+<0.01%)	⬆️
Py-3.9.19	99.07% <100.00%> (+<0.01%)	⬆️
Py-pypy7.3.11	99.39% <100.00%> (+<0.01%)	⬆️
Py-pypy7.3.16	99.42% <100.00%> (+<0.01%)	⬆️
VM-macos-latest	99.02% <100.00%> (+<0.01%)	⬆️
VM-ubuntu-latest	99.25% <100.00%> (+<0.01%)	⬆️
VM-windows-latest	99.58% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bdraco]

After looking at history and considering security perspective with this, I think this is the right fix

[bdraco]

skip label added as a workaround

[bdraco]

I'm going to test this on some Home Assistant production instances with ingress just to be sure.

[bdraco]

Just realized I had already updated my primary production with this since it was in my integration branch.

No obvious breakage

[Dreamsorcerer]

Suggested change

	Stopped decoding ``%2F`` (``/``) in ``URL.path``, as this could lead to code incorrectly being treated as a path separator
	Stopped decoding ``%2F`` (``/``) in ``URL.path``, as this could lead to code incorrectly treating it as a path separator

[bdraco]

I'll fix it up when I do the release

[Dreamsorcerer]

Damn, beat me by a couple of seconds.. :P

[ofekashery]

Hi @Dreamsorcerer @bdraco @webknjaz, thanks for your hard work ❤️
Please pay attention that this PR introduces BREAKING changes and might not be considered as a patch.

The %2F is frequently used by AMQP connection strings. This change led into a parsing bug in the AMQP connection protocol of mosquito/aiormq and mosquito/aio-pika, and broke our critical production system.

I believe those changes are great but could be as an optional flag. It also could reflected better in the changelog.

url = yarl.URL("amqp://localhost/%2f")
print(url.path)
# Yarl v1.9.4 -> `//`
# Yarl v1.9.5 -> `/%2F`

[webknjaz]

@ofekashery would you mind opening a new issue and maybe also submitting a PR with xfailing tests per https://blog.ganssle.io/articles/2021/11/pytest-xfail.html? Comments on closed things may end up getting lost. I'm not sure if @Dreamsorcerer and @bdraco got your message in time, for example.

[Dreamsorcerer]

Yeah, should probably have been a bigger version bump, but not too much we can do now. As it relates to security, I think it's still important to keep the change going forwards.

[webknjaz]

Yeah, security changes are always exempt from SemVer.

[Dreamsorcerer]

Sorry about that, we've decided to revert this anyway as it's caused a few more issues. If you need to know the path separators for safety, we're adding a path_safe attribute instead which skips decoding %2F and %25.