Skip to content

Commit 438494e

Browse files
jrflatjrflat
committed
(138059777) URL.deletingLastPathComponent() should append .. in special cases
1 parent f77911a commit 438494e

File tree

2 files changed

+194
-20
lines changed

2 files changed

+194
-20
lines changed

‎Sources/FoundationEssentials/URL/URL.swift

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,11 @@ public struct URL: Equatable, Sendable, Hashable {
13571357
return URL.fileSystemPath(for: path())
13581358
}
13591359

1360+
/// True if the URL's relative path would resolve against a base URL path
1361+
private var pathResolvesAgainstBase: Bool {
1362+
return _parseInfo.scheme == nil && !hasAuthority && relativePath().utf8.first != ._slash
1363+
}
1364+
13601365
/// Returns the path component of the URL if present, otherwise returns an empty string.
13611366
///
13621367
/// - note: This function will resolve against the base `URL`.
@@ -1649,7 +1654,9 @@ public struct URL: Equatable, Sendable, Hashable {
16491654
/// Returns a URL constructed by removing the last path component of self.
16501655
///
16511656
/// This function may either remove a path component or append `/..`.
1652-
/// If the URL has an empty path (e.g., `http://www.example.com`), then this function will return the URL unchanged.
1657+
/// If the URL has an empty path that is not resolved against a base URL
1658+
/// (e.g., `http://www.example.com`),
1659+
/// then this function will return the URL unchanged.
16531660
public func deletingLastPathComponent() -> URL {
16541661
#if FOUNDATION_FRAMEWORK
16551662
guard foundation_swift_url_enabled() else {
@@ -1658,7 +1665,17 @@ public struct URL: Equatable, Sendable, Hashable {
16581665
return result
16591666
}
16601667
#endif
1661-
guard !relativePath().isEmpty else { return self }
1668+
let shouldAppendDotDot = (
1669+
pathResolvesAgainstBase && (
1670+
relativePath().isEmpty
1671+
|| relativePath().lastPathComponent == "."
1672+
|| relativePath().lastPathComponent == ".."
1673+
)
1674+
)
1675+
if shouldAppendDotDot {
1676+
return self.appending(path: "..", directoryHint: .isDirectory)
1677+
}
1678+
16621679
var components = URLComponents(parseInfo: _parseInfo)
16631680
var newPath = components.percentEncodedPath.deletingLastPathComponent()
16641681
// .deletingLastPathComponent() removes the trailing "/", but we know it's a directory
@@ -2226,7 +2243,15 @@ extension URL {
22262243
pathToAppend = String(decoding: utf8, as: UTF8.self)
22272244
}
22282245

2229-
if newPath.utf8.last != ._slash && pathToAppend.utf8.first != ._slash {
2246+
// If the current path is empty (relative), don't append a slash which
2247+
// would make the path absolute--unless we have an authority.
2248+
2249+
// If we have an authority, we must add a slash to separate the path from the authority,
2250+
// e.g. URL("http://example.com").appending(path: "path") == "http://example.com/path"
2251+
2252+
if newPath.isEmpty && !hasAuthority {
2253+
// Do nothing, path will be directly appended
2254+
} else if newPath.utf8.last != ._slash && pathToAppend.utf8.first != ._slash {
22302255
newPath += "/"
22312256
} else if newPath.utf8.last == ._slash && pathToAppend.utf8.first == ._slash {
22322257
_ = newPath.popLast()

‎Tests/FoundationEssentialsTests/URLTests.swift

Lines changed: 166 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ import TestSupport
2323
@testable import Foundation
2424
#endif
2525

26+
private func checkBehavior<T: Equatable>(_ result: T, new: T, old: T) {
27+
#if FOUNDATION_FRAMEWORK
28+
if foundation_swift_url_enabled() {
29+
XCTAssertEqual(result, new)
30+
} else {
31+
XCTAssertEqual(result, old)
32+
}
33+
#else
34+
XCTAssertEqual(result, new)
35+
#endif
36+
}
37+
2638
final class URLTests : XCTestCase {
2739

2840
func testURLBasics() throws {
@@ -87,11 +99,7 @@ final class URLTests : XCTestCase {
8799
XCTAssertEqual(relativeURLWithBase.password(), baseURL.password())
88100
XCTAssertEqual(relativeURLWithBase.host(), baseURL.host())
89101
XCTAssertEqual(relativeURLWithBase.port, baseURL.port)
90-
#if !FOUNDATION_FRAMEWORK_NSURL
91-
XCTAssertEqual(relativeURLWithBase.path(), "/base/relative/path")
92-
#else
93-
XCTAssertEqual(relativeURLWithBase.path(), "relative/path")
94-
#endif
102+
checkBehavior(relativeURLWithBase.path(), new: "/base/relative/path", old: "relative/path")
95103
XCTAssertEqual(relativeURLWithBase.relativePath, "relative/path")
96104
XCTAssertEqual(relativeURLWithBase.query(), "query")
97105
XCTAssertEqual(relativeURLWithBase.fragment(), "fragment")
@@ -565,13 +573,8 @@ final class URLTests : XCTestCase {
565573
// `appending(component:)` should explicitly treat `component` as a single
566574
// path component, meaning "/" should be encoded to "%2F" before appending
567575
appended = url.appending(component: slashComponent, directoryHint: .notDirectory)
568-
#if FOUNDATION_FRAMEWORK_NSURL
569-
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/with:slash")
570-
XCTAssertEqual(appended.relativePath, "relative/with:slash")
571-
#else
572-
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/%2Fwith:slash")
573-
XCTAssertEqual(appended.relativePath, "relative/%2Fwith:slash")
574-
#endif
576+
checkBehavior(appended.absoluteString, new: "file:///var/mobile/relative/%2Fwith:slash", old: "file:///var/mobile/relative/with:slash")
577+
checkBehavior(appended.relativePath, new: "relative/%2Fwith:slash", old: "relative/with:slash")
575578

576579
appended = url.appendingPathComponent(component, isDirectory: false)
577580
XCTAssertEqual(appended.absoluteString, "file:///var/mobile/relative/no:slash")
@@ -583,6 +586,156 @@ final class URLTests : XCTestCase {
583586
XCTAssertEqual(appended.relativePath, "relative/with:slash")
584587
}
585588

589+
func testURLDeletingLastPathComponent() throws {
590+
var absolute = URL(filePath: "/absolute/path", directoryHint: .notDirectory)
591+
// Note: .relativePath strips the trailing slash for compatibility
592+
XCTAssertEqual(absolute.relativePath, "/absolute/path")
593+
XCTAssertFalse(absolute.hasDirectoryPath)
594+
595+
absolute.deleteLastPathComponent()
596+
XCTAssertEqual(absolute.relativePath, "/absolute")
597+
XCTAssertTrue(absolute.hasDirectoryPath)
598+
599+
absolute.deleteLastPathComponent()
600+
XCTAssertEqual(absolute.relativePath, "/")
601+
XCTAssertTrue(absolute.hasDirectoryPath)
602+
603+
// The old .deleteLastPathComponent() implementation appends ".." to the
604+
// root directory "/", resulting in "/../". This resolves back to "/".
605+
// The new implementation simply leaves "/" as-is.
606+
absolute.deleteLastPathComponent()
607+
checkBehavior(absolute.relativePath, new: "/", old: "/..")
608+
XCTAssertTrue(absolute.hasDirectoryPath)
609+
610+
absolute.append(path: "absolute", directoryHint: .isDirectory)
611+
checkBehavior(absolute.path, new: "/absolute", old: "/../absolute")
612+
613+
// Reset `var absolute` to "/absolute" to prevent having
614+
// a "/../" prefix in all the old expectations.
615+
absolute = URL(filePath: "/absolute", directoryHint: .isDirectory)
616+
617+
var relative = URL(filePath: "relative/path", directoryHint: .notDirectory, relativeTo: absolute)
618+
XCTAssertEqual(relative.relativePath, "relative/path")
619+
XCTAssertFalse(relative.hasDirectoryPath)
620+
XCTAssertEqual(relative.path, "/absolute/relative/path")
621+
622+
relative.deleteLastPathComponent()
623+
XCTAssertEqual(relative.relativePath, "relative")
624+
XCTAssertTrue(relative.hasDirectoryPath)
625+
XCTAssertEqual(relative.path, "/absolute/relative")
626+
627+
relative.deleteLastPathComponent()
628+
checkBehavior(relative.relativePath, new: "", old: ".")
629+
XCTAssertTrue(relative.hasDirectoryPath)
630+
XCTAssertEqual(relative.path, "/absolute")
631+
632+
relative.deleteLastPathComponent()
633+
XCTAssertEqual(relative.relativePath, "..")
634+
XCTAssertTrue(relative.hasDirectoryPath)
635+
XCTAssertEqual(relative.path, "/")
636+
637+
relative.deleteLastPathComponent()
638+
XCTAssertEqual(relative.relativePath, "../..")
639+
XCTAssertTrue(relative.hasDirectoryPath)
640+
checkBehavior(relative.path, new:"/", old: "/..")
641+
642+
relative.append(path: "path", directoryHint: .isDirectory)
643+
XCTAssertEqual(relative.relativePath, "../../path")
644+
XCTAssertTrue(relative.hasDirectoryPath)
645+
checkBehavior(relative.path, new: "/path", old: "/../path")
646+
647+
relative.deleteLastPathComponent()
648+
XCTAssertEqual(relative.relativePath, "../..")
649+
XCTAssertTrue(relative.hasDirectoryPath)
650+
checkBehavior(relative.path, new: "/", old: "/..")
651+
652+
relative = URL(filePath: "", relativeTo: absolute)
653+
checkBehavior(relative.relativePath, new: "", old: ".")
654+
XCTAssertTrue(relative.hasDirectoryPath)
655+
XCTAssertEqual(relative.path, "/absolute")
656+
657+
relative.deleteLastPathComponent()
658+
checkBehavior(relative.relativePath, new: "..", old: "..")
659+
XCTAssertTrue(relative.hasDirectoryPath)
660+
XCTAssertEqual(relative.path, "/")
661+
662+
relative.deleteLastPathComponent()
663+
checkBehavior(relative.relativePath, new: "../..", old: "../..")
664+
XCTAssertTrue(relative.hasDirectoryPath)
665+
checkBehavior(relative.path, new: "/", old: "/..")
666+
667+
relative = URL(filePath: "relative/./", relativeTo: absolute)
668+
// According to RFC 3986, "." and ".." segments should not be removed
669+
// until the path is resolved against the base URL (when calling .path)
670+
checkBehavior(relative.relativePath, new: "relative/.", old: "relative")
671+
XCTAssertTrue(relative.hasDirectoryPath)
672+
XCTAssertEqual(relative.path, "/absolute/relative")
673+
674+
relative.deleteLastPathComponent()
675+
checkBehavior(relative.relativePath, new: "relative/./..", old: ".")
676+
XCTAssertTrue(relative.hasDirectoryPath)
677+
XCTAssertEqual(relative.path, "/absolute")
678+
679+
relative = URL(filePath: "relative/.", directoryHint: .isDirectory, relativeTo: absolute)
680+
checkBehavior(relative.relativePath, new: "relative/.", old: "relative")
681+
XCTAssertTrue(relative.hasDirectoryPath)
682+
XCTAssertEqual(relative.path, "/absolute/relative")
683+
684+
relative.deleteLastPathComponent()
685+
checkBehavior(relative.relativePath, new: "relative/./..", old: ".")
686+
XCTAssertTrue(relative.hasDirectoryPath)
687+
XCTAssertEqual(relative.path, "/absolute")
688+
689+
relative = URL(filePath: "relative/..", relativeTo: absolute)
690+
XCTAssertEqual(relative.relativePath, "relative/..")
691+
checkBehavior(relative.hasDirectoryPath, new: true, old: false)
692+
XCTAssertEqual(relative.path, "/absolute")
693+
694+
relative.deleteLastPathComponent()
695+
XCTAssertEqual(relative.relativePath, "relative/../..")
696+
XCTAssertTrue(relative.hasDirectoryPath)
697+
XCTAssertEqual(relative.path, "/")
698+
699+
relative = URL(filePath: "relative/..", directoryHint: .isDirectory, relativeTo: absolute)
700+
XCTAssertEqual(relative.relativePath, "relative/..")
701+
XCTAssertTrue(relative.hasDirectoryPath)
702+
XCTAssertEqual(relative.path, "/absolute")
703+
704+
relative.deleteLastPathComponent()
705+
XCTAssertEqual(relative.relativePath, "relative/../..")
706+
XCTAssertTrue(relative.hasDirectoryPath)
707+
XCTAssertEqual(relative.path, "/")
708+
709+
var url = try XCTUnwrap(URL(string: "scheme://host.with.no.path"))
710+
XCTAssertTrue(url.path().isEmpty)
711+
712+
url.deleteLastPathComponent()
713+
XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path")
714+
XCTAssertTrue(url.path().isEmpty)
715+
716+
let unusedBase = URL(string: "base://url")
717+
url = try XCTUnwrap(URL(string: "scheme://host.with.no.path", relativeTo: unusedBase))
718+
XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path")
719+
XCTAssertTrue(url.path().isEmpty)
720+
721+
url.deleteLastPathComponent()
722+
XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path")
723+
XCTAssertTrue(url.path().isEmpty)
724+
725+
var schemeRelative = try XCTUnwrap(URL(string: "scheme:relative/path"))
726+
// Bug in the old implementation where a relative path is not recognized
727+
checkBehavior(schemeRelative.relativePath, new: "relative/path", old: "")
728+
729+
schemeRelative.deleteLastPathComponent()
730+
checkBehavior(schemeRelative.relativePath, new: "relative", old: "")
731+
732+
schemeRelative.deleteLastPathComponent()
733+
XCTAssertEqual(schemeRelative.relativePath, "")
734+
735+
schemeRelative.deleteLastPathComponent()
736+
XCTAssertEqual(schemeRelative.relativePath, "")
737+
}
738+
586739
func testURLFilePathDropsTrailingSlashes() throws {
587740
var url = URL(filePath: "/path/slashes///")
588741
XCTAssertEqual(url.path(), "/path/slashes///")
@@ -685,12 +838,8 @@ final class URLTests : XCTestCase {
685838
XCTAssertEqual(url.path(), "/path.foo/")
686839
url.append(path: "/////")
687840
url.deletePathExtension()
688-
#if !FOUNDATION_FRAMEWORK_NSURL
689-
XCTAssertEqual(url.path(), "/path/")
690-
#else
691841
// Old behavior only searches the last empty component, so the extension isn't actually removed
692-
XCTAssertEqual(url.path(), "/path.foo///")
693-
#endif
842+
checkBehavior(url.path(), new: "/path/", old: "/path.foo///")
694843
}
695844

696845
func testURLComponentsPercentEncodedUnencodedProperties() throws {

0 commit comments

Comments
 (0)