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.

Sign up for GitHub

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

Jump to bottom

gh-67693: Fix urlunparse() and urlunsplit() for URIs with path starting with multiple slashes and no authority #113563

Merged
merged 10 commits into from
May 14, 2024
Merged

gh-67693: Fix urlunparse() and urlunsplit() for URIs with path starting with multiple slashes and no authority #113563

merged 10 commits into from
May 14, 2024

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Dec 29, 2023 •
edited by bedevere-app bot
Loading

@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Dec 29, 2023
@bedevere-app bedevere-app bot mentioned this pull request Dec 29, 2023
Closed
@serhiy-storchaka serhiy-storchaka mentioned this pull request Dec 29, 2023
Closed
@serhiy-storchaka serhiy-storchaka changed the title gh-78457: Fix urlunparse() and urlunsplit() for URIs with path starting with multiple slashes and no authority gh-67693: Fix urlunparse() and urlunsplit() for URIs with path starting with multiple slashes and no authority Dec 29, 2023
@serhiy-storchaka
Copy link
Member Author

It fixes also more serious security issue #67693.

@serhiy-storchaka serhiy-storchaka added the type-security A security issue label Dec 29, 2023
@vadmium vadmium mentioned this pull request Apr 28, 2024
Open
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024
vadmium
vadmium reviewed May 12, 2024
View reviewed changes
Copy link
Member

@vadmium vadmium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes and tests look good to me.

Misc/NEWS.d/next/Library/2019-08-27-01-16-50.gh-issue-67693.4NIAiy.rst Show resolved Hide resolved
serhiy-storchaka
serhiy-storchaka commented May 13, 2024
View reviewed changes
Lib/test/test_urlparse.py
('//path/to/file',
('', 'path', '/to/file', '', '', ''),
('', 'path', '/to/file', '', '')),
('////path/to/file',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was broken.

Lib/test/test_urlparse.py
('scheme://path/to/file',
('scheme', 'path', '/to/file', '', '', ''),
('scheme', 'path', '/to/file', '', '')),
('scheme:////path/to/file',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was broken.

Lib/test/test_urlparse.py
('file:///tmp/junk.txt',
('file', '', '/tmp/junk.txt', '', '', ''),
('file', '', '/tmp/junk.txt', '', '')),
('file:////tmp/junk.txt',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was broken.

Lib/test/test_urlparse.py
('file:////tmp/junk.txt',
('file', '', '//tmp/junk.txt', '', '', ''),
('file', '', '//tmp/junk.txt', '', '')),
('file://///tmp/junk.txt',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was broken.

sethmlarson
sethmlarson approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@serhiy-storchaka serhiy-storchaka merged commit e237b25 into python:main May 14, 2024
33 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9, 3.10, 3.11, 3.12, 3.13.
🐍🍒⛏🤖

@bedevere-app
Copy link

bedevere-app bot commented May 14, 2024

GH-119025 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label May 14, 2024
@bedevere-app
Copy link

bedevere-app bot commented May 14, 2024

GH-119026 is a backport of this pull request to the 3.10 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.10 only security fixes label May 14, 2024
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 14, 2024
@serhiy-storchaka
[3.10] pythongh-67693: Fix urlunparse() and urlunsplit() for URIs wit…
2b1df88 
…h path starting with multiple slashes and no authority (pythonGH-113563)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 14, 2024

GH-119027 is a backport of this pull request to the 3.9 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.9 only security fixes label May 14, 2024
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 14, 2024
@serhiy-storchaka
[3.9] pythongh-67693: Fix urlunparse() and urlunsplit() for URIs with…
ba0a885 
… path starting with multiple slashes and no authority (pythonGH-113563)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 14, 2024

GH-119028 is a backport of this pull request to the 3.8 branch.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 14, 2024
@serhiy-storchaka
[3.10] pythongh-67693: Fix urlunparse() and urlunsplit() for URIs wit…
87d7223 
…h path starting with multiple slashes and no authority (pythonGH-113563)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this pull request May 14, 2024
@miss-islington @serhiy-storchaka
[3.13] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path…
8720006 
… starting with multiple slashes and no authority (GH-113563) (GH-119023)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this pull request May 14, 2024
@miss-islington @serhiy-storchaka
[3.12] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path…
387ff96 
… starting with multiple slashes and no authority (GH-113563) (GH-119024)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 14, 2024
Closed
@JelleZijlstra JelleZijlstra mentioned this pull request May 28, 2024
Closed
@nineteendo nineteendo mentioned this pull request Jun 12, 2024
Closed
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request Jun 23, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour
0d33a34 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

This currently affects only 3.12 and 3.13 but there are other backports
for that change in review upstream, so we'll likely need to update this
in the future.

[1] python/cpython#113563
@matthewhughes934 matthewhughes934 mentioned this pull request Jun 23, 2024
Merged
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request Jun 23, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour
2c42802 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

This currently affects only 3.12 and 3.13 but there are other backports
for that change in review upstream, so we'll likely need to update this
in the future.

[1] python/cpython#113563
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request Jun 23, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour
c2a674f 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

This currently affects only 3.12 and 3.13 but there are other backports
for that change in review upstream, so we'll likely need to update this
in the future.

[1] python/cpython#113563
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request Jun 23, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour
ebdd028 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

This currently affects only 3.12 and 3.13 but there are other backports
for that change in review upstream, so we'll likely need to update this
in the future.

[1] python/cpython#113563
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request Jun 24, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour
16f0617 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

The sample URL used to check this behaviour was taken from a test in the
upstream change (with the new behaviour this URL will round-trip
parsing)

[1] python/cpython#113563
pradyunsg pushed a commit to pypa/pip that referenced this pull request Jun 25, 2024
@matthewhughes934
Split up Windows tests relying on urlunparse behaviour (#12788)
5c389ec 
There was a behavioural change to `urllib.parse.urlunparse`[1] that
affects some of our tests on Windows. With the understanding that the
new behaviour is indeed desired, split up some tests relying on this
behaviour depending on the version of Python.

The sample URL used to check this behaviour was taken from a test in the
upstream change (with the new behaviour this URL will round-trip
parsing)

[1] python/cpython#113563
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@serhiy-storchaka @estyxx
pythongh-67693: Fix urlunparse() and urlunsplit() for URIs with path …
73e7fcf 
…starting with multiple slashes and no authority (pythonGH-113563)
This was referenced Jul 21, 2024
Closed
Merged
ambv pushed a commit that referenced this pull request Sep 4, 2024
@serhiy-storchaka
[3.11] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path…
d0e8c10 
… starting with multiple slashes and no authority (GH-113563) (#119025)

(cherry picked from commit e237b25)
ambv pushed a commit that referenced this pull request Sep 4, 2024
@serhiy-storchaka
[3.10] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path…
2fa5d70 
… starting with multiple slashes and no authority (GH-113563) (#119026)

(cherry picked from commit e237b25)
ambv added a commit that referenced this pull request Sep 4, 2024
@serhiy-storchaka @ambv
[3.8] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path …
0a87812 
…starting with multiple slashes and no authority (GH-113563) (#119028)

(cherry picked from commit e237b25)

Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv added a commit that referenced this pull request Sep 5, 2024
@serhiy-storchaka @ambv
[3.9] gh-67693: Fix urlunparse() and urlunsplit() for URIs with path …
a5798d0 
…starting with multiple slashes and no authority (GH-113563) (#119027)

(cherry picked from commit e237b25)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vadmium vadmium vadmium left review comments

@sethmlarson sethmlarson sethmlarson approved these changes

Assignees

@serhiy-storchaka serhiy-storchaka

Labels
type-security A security issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serhiy-storchaka @vadmium @sethmlarson @epicfaace