Skip to content

Commit

Permalink
(138059051) URL: Appending to an empty file path results in an absolu…
Browse files Browse the repository at this point in the history 
…te path (#988)
  • Loading branch information
@jrflat
jrflat authored Oct 23, 2024
1 parent 71eefee commit f077992
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 33 deletions.
30 changes: 23 additions & 7 deletions Sources/FoundationEssentials/URL/URL.swift
View file
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
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

0 comments on commit f077992

Please sign in to comment.