Skip to content

Always remove authority section when cleaning local URL path #13510

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

Merged
merged 2 commits into from
Jul 28, 2025

Conversation

barneygale
Copy link
Contributor

pathname2url() generates a URL with an authority section on Windows (all versions) and non-Windows (3.12+ for paths beginning //, 3.14+ for paths beginning /). We need to strip this prefix in order to return a URL path rather than a complete URL.

@barneygale barneygale force-pushed the always-remove-netloc-when-cleaning branch from cdad848 to beb4a8d Compare July 27, 2025 23:06
@barneygale barneygale marked this pull request as ready for review July 27, 2025 23:10
@barneygale
Copy link
Contributor Author

@ichard26 another one for you! :D

This one fixes all the test cases that were expecting file:////, which is only a valid prefix for UNC paths, not DOS drive paths like T:\. It will be interesting to re-run the 3.14 test suite after this lands - I think it will fix some tests.

# Trailing slashes are now preserved on Windows, matching POSIX behaviour.
# BPO: https://github.com/python/cpython/issues/126212
does_pathname2url_preserve_trailing_slash = pathname2url("C:/foo/").endswith("/")
does_pathname2url_preserve_trailing_slash = pathname2url("C:\\foo\\").endswith("/")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This detection didn't work correctly, but I suspect it didn't matter because the has_new_urlun_behavior value coincided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(took me flippin ages to figure this out!)

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

This looks good, thanks a lot for working on this!

@ichard26 ichard26 merged commit 4117dc7 into pypa:main Jul 28, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants