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

URL: Appending to an empty file path results in an absolute path #988

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

jrflat
Copy link
Contributor

@jrflat jrflat commented Oct 18, 2024

When appending path to a URL with an empty relative path, the current code does: "" + "/" + path. This is problematic because it makes the relative path absolute, so it no longer resolves against the base URL.

E.g. for the following code,

let currentDir = URL(filePath: "")
print(currentDir.path) // "/my/cwd"
let file = currentDir.appending(path: "main.swift")
print(file.path) // "/main.swift"

we should get an output of "/my/cwd/main.swift", not "/main.swift".

This PR updates URL.appending(path:) so if the current path is empty (and thus relative), we don't append a slash which would make the path absolute. That is unless we have an authority. If we have an authority, we must add a slash to separate the path from the authority, e.g. URL("http://example.com").appending(path: "path") == http://example.com/path.

Resolves #921 (thanks @omochi for filing!)

@jrflat
Copy link
Contributor Author

jrflat commented Oct 18, 2024

@swift-ci please test

@omochi
Copy link

omochi commented Oct 18, 2024

Thank you for fixing this!​

@jrflat
Copy link
Contributor Author

jrflat commented Oct 18, 2024

Thanks again for bringing this to our attention! I believe this also resolves #922 (the relative path will still be empty, but we will handle it correctly afterwards). I've opened #989 to fix some other special cases of .deletingLastPathComponent() as well

@jrflat
Copy link
Contributor Author

jrflat commented Oct 23, 2024

Updated the behavior slightly to be more compatible for clients expecting the relative path to start with "./".

@jrflat
Copy link
Contributor Author

jrflat commented Oct 23, 2024

@swift-ci please test

@jrflat jrflat merged commit f077992 into swiftlang:main Oct 23, 2024
3 checks passed
tshortli added a commit to tshortli/swift-foundation that referenced this pull request Oct 24, 2024
jrflat added a commit to jrflat/swift-foundation that referenced this pull request Oct 25, 2024
cthielen pushed a commit to cthielen/swift-foundation that referenced this pull request Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appending to an empty file path results in an absolute path
3 participants