From b1a63e3a4036fba9b1f90f919dface677ae93142 Mon Sep 17 00:00:00 2001 From: Jonathan Flat <50605158+jrflat@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:46:53 -0600 Subject: [PATCH] (138059777) URL.deletingLastPathComponent() should append .. in special cases (#989) --- Sources/FoundationEssentials/URL/URL.swift | 34 +++- .../FoundationEssentialsTests/URLTests.swift | 150 ++++++++++++++++++ 2 files changed, 179 insertions(+), 5 deletions(-) diff --git a/Sources/FoundationEssentials/URL/URL.swift b/Sources/FoundationEssentials/URL/URL.swift index 063d5f9f..e68facd5 100644 --- a/Sources/FoundationEssentials/URL/URL.swift +++ b/Sources/FoundationEssentials/URL/URL.swift @@ -1351,6 +1351,11 @@ public struct URL: Equatable, Sendable, Hashable { return URL.fileSystemPath(for: path()) } + /// True if the URL's relative path would resolve against a base URL path + private var pathResolvesAgainstBase: Bool { + return _parseInfo.scheme == nil && !hasAuthority && relativePath().utf8.first != ._slash + } + /// Returns the path component of the URL if present, otherwise returns an empty string. /// /// - note: This function will resolve against the base `URL`. @@ -1643,7 +1648,9 @@ public struct URL: Equatable, Sendable, Hashable { /// Returns a URL constructed by removing the last path component of self. /// /// This function may either remove a path component or append `/..`. - /// If the URL has an empty path (e.g., `http://www.example.com`), then this function will return the URL unchanged. + /// If the URL has an empty path that is not resolved against a base URL + /// (e.g., `http://www.example.com`), + /// then this function will return the URL unchanged. public func deletingLastPathComponent() -> URL { #if FOUNDATION_FRAMEWORK guard foundation_swift_url_enabled() else { @@ -1652,13 +1659,30 @@ public struct URL: Equatable, Sendable, Hashable { return result } #endif - guard !relativePath().isEmpty else { return self } - var components = URLComponents(parseInfo: _parseInfo) - var newPath = components.percentEncodedPath.deletingLastPathComponent() + let path = relativePath() + let shouldAppendDotDot = ( + pathResolvesAgainstBase && ( + path.isEmpty + || path.lastPathComponent == "." + || path.lastPathComponent == ".." + ) + ) + + var newPath = path + if newPath.lastPathComponent != ".." { + newPath = newPath.deletingLastPathComponent() + } + if shouldAppendDotDot { + newPath = newPath.appendingPathComponent("..") + } + if newPath.isEmpty && pathResolvesAgainstBase { + newPath = "." + } // .deletingLastPathComponent() removes the trailing "/", but we know it's a directory - if !newPath.isEmpty, newPath.utf8.last != UInt8(ascii: "/") { + if !newPath.isEmpty && newPath.utf8.last != ._slash { newPath += "/" } + var components = URLComponents(parseInfo: _parseInfo) components.percentEncodedPath = newPath return components.url(relativeTo: baseURL)! } diff --git a/Tests/FoundationEssentialsTests/URLTests.swift b/Tests/FoundationEssentialsTests/URLTests.swift index 437a1806..7feaddc4 100644 --- a/Tests/FoundationEssentialsTests/URLTests.swift +++ b/Tests/FoundationEssentialsTests/URLTests.swift @@ -586,6 +586,156 @@ final class URLTests : XCTestCase { XCTAssertEqual(appended.relativePath, "relative/with:slash") } + func testURLDeletingLastPathComponent() throws { + var absolute = URL(filePath: "/absolute/path", directoryHint: .notDirectory) + // Note: .relativePath strips the trailing slash for compatibility + XCTAssertEqual(absolute.relativePath, "/absolute/path") + XCTAssertFalse(absolute.hasDirectoryPath) + + absolute.deleteLastPathComponent() + XCTAssertEqual(absolute.relativePath, "/absolute") + XCTAssertTrue(absolute.hasDirectoryPath) + + absolute.deleteLastPathComponent() + XCTAssertEqual(absolute.relativePath, "/") + XCTAssertTrue(absolute.hasDirectoryPath) + + // The old .deleteLastPathComponent() implementation appends ".." to the + // root directory "/", resulting in "/../". This resolves back to "/". + // The new implementation simply leaves "/" as-is. + absolute.deleteLastPathComponent() + checkBehavior(absolute.relativePath, new: "/", old: "/..") + XCTAssertTrue(absolute.hasDirectoryPath) + + absolute.append(path: "absolute", directoryHint: .isDirectory) + checkBehavior(absolute.path, new: "/absolute", old: "/../absolute") + + // Reset `var absolute` to "/absolute" to prevent having + // a "/../" prefix in all the old expectations. + absolute = URL(filePath: "/absolute", directoryHint: .isDirectory) + + var relative = URL(filePath: "relative/path", directoryHint: .notDirectory, relativeTo: absolute) + XCTAssertEqual(relative.relativePath, "relative/path") + XCTAssertFalse(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute/relative/path") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "relative") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute/relative") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, ".") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "..") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "../..") + XCTAssertTrue(relative.hasDirectoryPath) + checkBehavior(relative.path, new:"/", old: "/..") + + relative.append(path: "path", directoryHint: .isDirectory) + XCTAssertEqual(relative.relativePath, "../../path") + XCTAssertTrue(relative.hasDirectoryPath) + checkBehavior(relative.path, new: "/path", old: "/../path") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "../..") + XCTAssertTrue(relative.hasDirectoryPath) + checkBehavior(relative.path, new: "/", old: "/..") + + relative = URL(filePath: "", relativeTo: absolute) + checkBehavior(relative.relativePath, new: "", old: ".") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "..") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "../..") + XCTAssertTrue(relative.hasDirectoryPath) + checkBehavior(relative.path, new: "/", old: "/..") + + relative = URL(filePath: "relative/./", relativeTo: absolute) + // According to RFC 3986, "." and ".." segments should not be removed + // until the path is resolved against the base URL (when calling .path) + checkBehavior(relative.relativePath, new: "relative/.", old: "relative") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute/relative") + + relative.deleteLastPathComponent() + checkBehavior(relative.relativePath, new: "relative/..", old: ".") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute") + + relative = URL(filePath: "relative/.", directoryHint: .isDirectory, relativeTo: absolute) + checkBehavior(relative.relativePath, new: "relative/.", old: "relative") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute/relative") + + relative.deleteLastPathComponent() + checkBehavior(relative.relativePath, new: "relative/..", old: ".") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute") + + relative = URL(filePath: "relative/..", relativeTo: absolute) + XCTAssertEqual(relative.relativePath, "relative/..") + checkBehavior(relative.hasDirectoryPath, new: true, old: false) + XCTAssertEqual(relative.path, "/absolute") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "relative/../..") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/") + + relative = URL(filePath: "relative/..", directoryHint: .isDirectory, relativeTo: absolute) + XCTAssertEqual(relative.relativePath, "relative/..") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/absolute") + + relative.deleteLastPathComponent() + XCTAssertEqual(relative.relativePath, "relative/../..") + XCTAssertTrue(relative.hasDirectoryPath) + XCTAssertEqual(relative.path, "/") + + var url = try XCTUnwrap(URL(string: "scheme://host.with.no.path")) + XCTAssertTrue(url.path().isEmpty) + + url.deleteLastPathComponent() + XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path") + XCTAssertTrue(url.path().isEmpty) + + let unusedBase = URL(string: "base://url") + url = try XCTUnwrap(URL(string: "scheme://host.with.no.path", relativeTo: unusedBase)) + XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path") + XCTAssertTrue(url.path().isEmpty) + + url.deleteLastPathComponent() + XCTAssertEqual(url.absoluteString, "scheme://host.with.no.path") + XCTAssertTrue(url.path().isEmpty) + + var schemeRelative = try XCTUnwrap(URL(string: "scheme:relative/path")) + // Bug in the old implementation where a relative path is not recognized + checkBehavior(schemeRelative.relativePath, new: "relative/path", old: "") + + schemeRelative.deleteLastPathComponent() + checkBehavior(schemeRelative.relativePath, new: "relative", old: "") + + schemeRelative.deleteLastPathComponent() + XCTAssertEqual(schemeRelative.relativePath, "") + + schemeRelative.deleteLastPathComponent() + XCTAssertEqual(schemeRelative.relativePath, "") + } + func testURLFilePathDropsTrailingSlashes() throws { var url = URL(filePath: "/path/slashes///") XCTAssertEqual(url.path(), "/path/slashes///")