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

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

Merged
merged 1 commit into from
Oct 23, 2024
+88 −33
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions Sources/FoundationEssentials/URL/URL.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2214,9 +2214,8 @@ extension URL {
var path = String(path)
#endif

var newPath = relativePath()
var insertedSlash = false
if !newPath.isEmpty && path.utf8.first != ._slash {
if !relativePath().isEmpty && path.utf8.first != ._slash {
// Don't treat as first path segment when encoding
path = "/" + path
insertedSlash = true
Expand All @@ -2231,13 +2230,30 @@ extension URL {
pathToAppend = String(decoding: utf8, as: UTF8.self)
}

if newPath.utf8.last != ._slash && pathToAppend.utf8.first != ._slash {
newPath += "/"
} else if newPath.utf8.last == ._slash && pathToAppend.utf8.first == ._slash {
_ = newPath.popLast()
func appendedPath() -> String {
var currentPath = relativePath()
if currentPath.isEmpty && !hasAuthority {
guard _parseInfo.scheme == nil else {
// Scheme only, append directly to the empty path, e.g.
// URL("scheme:").appending(path: "path") == scheme:path
return pathToAppend
}
// No scheme or authority, treat the empty path as "."
currentPath = "."
}

// If currentPath is empty, pathToAppend is relative, and we have an authority,
// we must append a slash to separate the path from authority, which happens below.

if currentPath.utf8.last != ._slash && pathToAppend.utf8.first != ._slash {
currentPath += "/"
} else if currentPath.utf8.last == ._slash && pathToAppend.utf8.first == ._slash {
_ = currentPath.popLast()
}
return currentPath + pathToAppend
}

newPath += pathToAppend
var newPath = appendedPath()

let hasTrailingSlash = newPath.utf8.last == ._slash
let isDirectory: Bool
Expand Down
91 changes: 65 additions & 26 deletions Tests/FoundationEssentialsTests/URLTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ import TestSupport
@testable import Foundation
#endif

private func checkBehavior<T: Equatable>(_ result: T, new: T, old: T, file: StaticString = #filePath, line: UInt = #line) {
#if FOUNDATION_FRAMEWORK
if foundation_swift_url_enabled() {
XCTAssertEqual(result, new, file: file, line: line)
} else {
XCTAssertEqual(result, old, file: file, line: line)
}
#else
XCTAssertEqual(result, new, file: file, line: line)
#endif
}

final class URLTests : XCTestCase {

func testURLBasics() throws {
Expand Down Expand Up @@ -87,11 +99,7 @@ final class URLTests : XCTestCase {
XCTAssertEqual(relativeURLWithBase.password(), baseURL.password())
XCTAssertEqual(relativeURLWithBase.host(), baseURL.host())
XCTAssertEqual(relativeURLWithBase.port, baseURL.port)
#if !FOUNDATION_FRAMEWORK_NSURL
XCTAssertEqual(relativeURLWithBase.path(), "/base/relative/path")
#else
XCTAssertEqual(relativeURLWithBase.path(), "relative/path")
#endif
checkBehavior(relativeURLWithBase.path(), new: "/base/relative/path", old: "relative/path")
XCTAssertEqual(relativeURLWithBase.relativePath, "relative/path")
XCTAssertEqual(relativeURLWithBase.query(), "query")
XCTAssertEqual(relativeURLWithBase.fragment(), "fragment")
Expand Down Expand Up @@ -154,7 +162,7 @@ final class URLTests : XCTestCase {
"http:g" : "http:g", // For strict parsers
]

#if FOUNDATION_FRAMEWORK_NSURL
#if FOUNDATION_FRAMEWORK
let testsFailingWithoutSwiftURL = Set([
"",
"../../../g",
Expand All @@ -165,8 +173,8 @@ final class URLTests : XCTestCase {
#endif

for test in tests {
#if FOUNDATION_FRAMEWORK_NSURL
if testsFailingWithoutSwiftURL.contains(test.key) {
#if FOUNDATION_FRAMEWORK
if !foundation_swift_url_enabled(), testsFailingWithoutSwiftURL.contains(test.key) {
continue
}
#endif
Expand All @@ -178,8 +186,8 @@ final class URLTests : XCTestCase {
}

func testURLPathAPIsResolveAgainstBase() throws {
#if FOUNDATION_FRAMEWORK_NSURL
try XCTSkipIf(true)
#if FOUNDATION_FRAMEWORK
try XCTSkipIf(!foundation_swift_url_enabled())
#endif
// Borrowing the same test cases from RFC 3986, but checking paths
let base = URL(string: "http://a/b/c/d;p?q")
Expand Down Expand Up @@ -246,8 +254,8 @@ final class URLTests : XCTestCase {
}

func testURLPathComponentsPercentEncodedSlash() throws {
#if FOUNDATION_FRAMEWORK_NSURL
try XCTSkipIf(true)
#if FOUNDATION_FRAMEWORK
try XCTSkipIf(!foundation_swift_url_enabled())
#endif

var url = try XCTUnwrap(URL(string: "https://example.com/https%3A%2F%2Fexample.com"))
Expand All @@ -270,8 +278,8 @@ final class URLTests : XCTestCase {
}

func testURLRootlessPath() throws {
#if FOUNDATION_FRAMEWORK_NSURL
try XCTSkipIf(true)
#if FOUNDATION_FRAMEWORK
try XCTSkipIf(!foundation_swift_url_enabled())
#endif

let paths = ["", "path"]
Expand Down Expand Up @@ -565,13 +573,8 @@ final class URLTests : XCTestCase {
// `appending(component:)` should explicitly treat `component` as a single
// path component, meaning "/" should be encoded to "%2F" before appending
appended = url.appending(component: slashComponent, directoryHint: .notDirectory)
#if FOUNDATION_FRAMEWORK_NSURL
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/with:slash")
XCTAssertEqual(appended.relativePath, "relative/with:slash")
#else
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/%2Fwith:slash")
XCTAssertEqual(appended.relativePath, "relative/%2Fwith:slash")
#endif
checkBehavior(appended.absoluteString, new: "file:///var/mobile/relative/%2Fwith:slash", old: "file:///var/mobile/relative/with:slash")
checkBehavior(appended.relativePath, new: "relative/%2Fwith:slash", old: "relative/with:slash")

appended = url.appendingPathComponent(component, isDirectory: false)
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/no:slash")
Expand Down Expand Up @@ -685,12 +688,48 @@ final class URLTests : XCTestCase {
XCTAssertEqual(url.path(), "/path.foo/")
url.append(path: "/////")
url.deletePathExtension()
#if !FOUNDATION_FRAMEWORK_NSURL
XCTAssertEqual(url.path(), "/path/")
#else
// Old behavior only searches the last empty component, so the extension isn't actually removed
XCTAssertEqual(url.path(), "/path.foo///")
#endif
checkBehavior(url.path(), new: "/path/", old: "/path.foo///")
}

func testURLAppendingToEmptyPath() throws {
let baseURL = URL(filePath: "/base/directory", directoryHint: .isDirectory)
let emptyPathURL = URL(filePath: "", relativeTo: baseURL)
let url = emptyPathURL.appending(path: "main.swift")
XCTAssertEqual(url.relativePath, "./main.swift")
XCTAssertEqual(url.path, "/base/directory/main.swift")

var example = try XCTUnwrap(URL(string: "https://example.com"))
XCTAssertEqual(example.host(), "example.com")
XCTAssertTrue(example.path().isEmpty)

// Appending to an empty path should add a slash if an authority exists
// The appended path should never become part of the host
example.append(path: "foo")
XCTAssertEqual(example.host(), "example.com")
XCTAssertEqual(example.path(), "/foo")
XCTAssertEqual(example.absoluteString, "https://example.com/foo")

var emptyHost = try XCTUnwrap(URL(string: "scheme://"))
XCTAssertTrue(emptyHost.host()?.isEmpty ?? true)
XCTAssertTrue(emptyHost.path().isEmpty)

emptyHost.append(path: "foo")
XCTAssertTrue(emptyHost.host()?.isEmpty ?? true)
// Old behavior failed to append correctly to an empty host
// Modern parsers agree that "foo" relative to "scheme://" is "scheme:///foo"
checkBehavior(emptyHost.path(), new: "/foo", old: "")
checkBehavior(emptyHost.absoluteString, new: "scheme:///foo", old: "scheme://")

var schemeOnly = try XCTUnwrap(URL(string: "scheme:"))
XCTAssertTrue(schemeOnly.host()?.isEmpty ?? true)
XCTAssertTrue(schemeOnly.path().isEmpty)

schemeOnly.append(path: "foo")
XCTAssertTrue(schemeOnly.host()?.isEmpty ?? true)
// Old behavior appends to the string, but is missing the path
checkBehavior(schemeOnly.path(), new: "foo", old: "")
XCTAssertEqual(schemeOnly.absoluteString, "scheme:foo")
}

func testURLComponentsPercentEncodedUnencodedProperties() throws {
Expand Down