gh-67693: Fix urlunparse() and urlunsplit() for URIs with path starting with multiple slashes and no authority by serhiy-storchaka · Pull Request #113563 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation20 Commits10 Checks36 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
…ltiple slashes and no authority.
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
It fixes also more serious security issue #67693.
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.
| ('//path/to/file', |
|---|
| ('', 'path', '/to/file', '', '', ''), |
| ('', 'path', '/to/file', '', '')), |
| ('////path/to/file', |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case was broken.
| ('scheme://path/to/file', |
|---|
| ('scheme', 'path', '/to/file', '', '', ''), |
| ('scheme', 'path', '/to/file', '', '')), |
| ('scheme:////path/to/file', |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case was broken.
| ('file:///tmp/junk.txt', |
|---|
| ('file', '', '/tmp/junk.txt', '', '', ''), |
| ('file', '', '/tmp/junk.txt', '', '')), |
| ('file:////tmp/junk.txt', |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case was broken.
| ('file:////tmp/junk.txt', |
|---|
| ('file', '', '//tmp/junk.txt', '', '', ''), |
| ('file', '', '//tmp/junk.txt', '', '')), |
| ('file://///tmp/junk.txt', |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case was broken.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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.
🐍🍒⛏🤖
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request
…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 to serhiy-storchaka/cpython that referenced this pull request
… 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 to serhiy-storchaka/cpython that referenced this pull request
…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
… 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
… starting with multiple slashes and no authority (GH-113563) (GH-119024)
(cherry picked from commit e237b25)
Co-authored-by: Serhiy Storchaka storchaka@gmail.com
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request
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.
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request
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.
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request
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.
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request
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.
matthewhughes934 pushed a commit to matthewhughes934/pip that referenced this pull request
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)
pradyunsg pushed a commit to pypa/pip that referenced this pull request
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)
estyxx pushed a commit to estyxx/cpython that referenced this pull request
This was referenced
Jul 21, 2024
ambv pushed a commit that referenced this pull request
ambv pushed a commit that referenced this pull request
ambv added a commit that referenced this pull request
…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
…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