From 4eedff5d128f5eb5ced4dfd8bb8705dbcb62b9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Tue, 11 Jun 2024 16:09:51 +0200 Subject: [PATCH 1/2] Improve URL equality --- .../Toolkit/URL/Absolute URL/FileURL.swift | 6 ++ .../Toolkit/URL/Absolute URL/HTTPURL.swift | 8 ++ .../URL/Absolute URL/UnknownAbsoluteURL.swift | 10 +++ Sources/Shared/Toolkit/URL/RelativeURL.swift | 6 ++ .../URL/Absolute URL/FileURLTests.swift | 56 ++++++++++-- .../URL/Absolute URL/HTTPURLTests.swift | 85 ++++++++++++++++++- .../UnknownAbsoluteURLTests.swift | 81 +++++++++++++++++- .../Toolkit/URL/RelativeURLTests.swift | 31 ++++--- 8 files changed, 254 insertions(+), 29 deletions(-) diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift index bd892b4d3..631d30590 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift @@ -66,6 +66,12 @@ public struct FileURL: AbsoluteURL, Hashable, Sendable { public func isDirectory() throws -> Bool { try (url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) ?? false } + + public static func == (lhs: Self, rhs: Self) -> Bool { + lhs.origin == rhs.origin + && lhs.path == rhs.path + && lhs.url.user == rhs.url.user + } } public extension URLConvertible { diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift index 76257bea8..8c1198967 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift @@ -35,6 +35,14 @@ public struct HTTPURL: AbsoluteURL, Hashable, Sendable { } return o } + + public static func == (lhs: Self, rhs: Self) -> Bool { + lhs.origin == rhs.origin + && lhs.path == rhs.path + && lhs.query == rhs.query + && lhs.fragment == rhs.fragment + && lhs.url.user == rhs.url.user + } } public extension URLConvertible { diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift index ab5b3b727..7457a74ff 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift @@ -23,4 +23,14 @@ struct UnknownAbsoluteURL: AbsoluteURL, Hashable { let url: URL let scheme: URLScheme let origin: String? = nil + + public static func == (lhs: Self, rhs: Self) -> Bool { + lhs.scheme == rhs.scheme + && lhs.host == rhs.host + && lhs.url.port == rhs.url.port + && lhs.path == rhs.path + && lhs.query == rhs.query + && lhs.fragment == rhs.fragment + && lhs.url.user == rhs.url.user + } } diff --git a/Sources/Shared/Toolkit/URL/RelativeURL.swift b/Sources/Shared/Toolkit/URL/RelativeURL.swift index 646488ecd..72a32eba5 100644 --- a/Sources/Shared/Toolkit/URL/RelativeURL.swift +++ b/Sources/Shared/Toolkit/URL/RelativeURL.swift @@ -96,6 +96,12 @@ public struct RelativeURL: URLProtocol, Hashable { .removingPrefix("/") ) } + + public static func == (lhs: Self, rhs: Self) -> Bool { + lhs.path == rhs.path + && lhs.query == rhs.query + && lhs.fragment == rhs.fragment + } } /// Implements `URLConvertible`. diff --git a/Tests/SharedTests/Toolkit/URL/Absolute URL/FileURLTests.swift b/Tests/SharedTests/Toolkit/URL/Absolute URL/FileURLTests.swift index 5547b48c1..af38a1842 100644 --- a/Tests/SharedTests/Toolkit/URL/Absolute URL/FileURLTests.swift +++ b/Tests/SharedTests/Toolkit/URL/Absolute URL/FileURLTests.swift @@ -10,22 +10,60 @@ import XCTest class FileURLTests: XCTestCase { func testEquality() { + // Paths must be equal. XCTAssertEqual( FileURL(string: "file:///foo/bar")!, - FileURL(string: "file:///foo/bar")! + FileURL(string: "file:///foo/bar") ) - // Fragments are ignored. + XCTAssertNotEqual( + FileURL(string: "file:///foo/baz")!, + FileURL(string: "file:///foo/bar") + ) + + // Paths is compared percent and entity-decoded. XCTAssertEqual( - FileURL(string: "file:///foo/bar")!, - FileURL(string: "file:///foo/bar#fragment")! + FileURL(string: "file:///c%27est%20valide")!, + FileURL(string: "file:///c%27est%20valide") ) - XCTAssertNotEqual( - FileURL(string: "file:///foo/bar")!, - FileURL(string: "file:///foo/baz")! + XCTAssertEqual( + FileURL(string: "file:///c'est%20valide")!, + FileURL(string: "file:///c%27est%20valide") + ) + + // Authority must be equal. + XCTAssertEqual( + FileURL(string: "file://user:password@host/foo")!, + FileURL(string: "file://user:password@host/foo") ) XCTAssertNotEqual( - FileURL(string: "file:///foo/bar")!, - FileURL(string: "file:///foo/bar/")! + FileURL(string: "file://foo"), + FileURL(string: "file://host/foo") + ) + + // Query parameters are ignored. + XCTAssertEqual( + FileURL(string: "file:///foo/bar?b=b&a=a")!, + FileURL(string: "file:///foo/bar?a=a&b=b") + ) + XCTAssertEqual( + FileURL(string: "file:///foo/bar?b=b")!, + FileURL(string: "file:///foo/bar?a=a") + ) + + // Scheme is case insensitive. + XCTAssertEqual( + FileURL(string: "FILE:///foo")!, + FileURL(string: "file:///foo") + ) + + // Fragment is ignored. + XCTAssertEqual( + FileURL(string: "file:///foo")!, + FileURL(string: "file:///foo#fragment") + ) + XCTAssertEqual( + FileURL(string: "file:///foo#other")!, + FileURL(string: "file:///foo#fragment") ) } diff --git a/Tests/SharedTests/Toolkit/URL/Absolute URL/HTTPURLTests.swift b/Tests/SharedTests/Toolkit/URL/Absolute URL/HTTPURLTests.swift index 869eed1aa..3fc21dfd3 100644 --- a/Tests/SharedTests/Toolkit/URL/Absolute URL/HTTPURLTests.swift +++ b/Tests/SharedTests/Toolkit/URL/Absolute URL/HTTPURLTests.swift @@ -10,13 +10,90 @@ import XCTest class HTTPURLTests: XCTestCase { func testEquality() { + // Paths must be equal. XCTAssertEqual( - HTTPURL(string: "http://domain.com")!, - HTTPURL(string: "http://domain.com")! + HTTPURL(string: "http://example.com/foo/bar")!, + HTTPURL(string: "http://example.com/foo/bar") ) XCTAssertNotEqual( - HTTPURL(string: "http://domain.com")!, - HTTPURL(string: "http://domain.com#fragment")! + HTTPURL(string: "http://example.com/foo/baz")!, + HTTPURL(string: "http://example.com/foo/bar") + ) + + // Paths is compared percent and entity-decoded. + XCTAssertEqual( + HTTPURL(string: "http://example.com/c%27est%20valide")!, + HTTPURL(string: "http://example.com/c%27est%20valide") + ) + XCTAssertEqual( + HTTPURL(string: "http://example.com/c'est%20valide")!, + HTTPURL(string: "http://example.com/c%27est%20valide") + ) + + // Authority must be equal. + XCTAssertEqual( + HTTPURL(string: "http://example.com/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://example.com:80/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://example.com:80/foo")!, + HTTPURL(string: "http://example.com:443/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://example.com:80/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://domain.com/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://user:password@example.com/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://user:password@example.com/foo")!, + HTTPURL(string: "http://other:password@example.com/foo") + ) + + // Order of query parameters is important. + XCTAssertNotEqual( + HTTPURL(string: "http://example.com/foo/bar?b=b&a=a")!, + HTTPURL(string: "http://example.com/foo/bar?a=a&b=b") + ) + + // Content of parameters is important. + XCTAssertEqual( + HTTPURL(string: "http://example.com/foo/bar?a=a&b=b")!, + HTTPURL(string: "http://example.com/foo/bar?a=a&b=b") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://example.com/foo/bar?b=b")!, + HTTPURL(string: "http://example.com/foo/bar?a=a") + ) + + // Scheme is case insensitive. + XCTAssertEqual( + HTTPURL(string: "HTTP://example.com/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + XCTAssertNotEqual( + HTTPURL(string: "https://example.com/foo")!, + HTTPURL(string: "http://example.com/foo") + ) + + // Fragment is relevant. + XCTAssertEqual( + HTTPURL(string: "http://example.com/foo#fragment")!, + HTTPURL(string: "http://example.com/foo#fragment") + ) + XCTAssertNotEqual( + HTTPURL(string: "http://example.com/foo#other")!, + HTTPURL(string: "http://example.com/foo#fragment") ) } diff --git a/Tests/SharedTests/Toolkit/URL/Absolute URL/UnknownAbsoluteURLTests.swift b/Tests/SharedTests/Toolkit/URL/Absolute URL/UnknownAbsoluteURLTests.swift index 3ca1f5989..3c81e2278 100644 --- a/Tests/SharedTests/Toolkit/URL/Absolute URL/UnknownAbsoluteURLTests.swift +++ b/Tests/SharedTests/Toolkit/URL/Absolute URL/UnknownAbsoluteURLTests.swift @@ -10,13 +10,86 @@ import XCTest class UnknownAbsoluteURLTests: XCTestCase { func testEquality() { + // Paths must be equal. XCTAssertEqual( - UnknownAbsoluteURL(string: "opds://domain.com")!, - UnknownAbsoluteURL(string: "opds://domain.com")! + UnknownAbsoluteURL(string: "opds://example.com/foo/bar")!, + UnknownAbsoluteURL(string: "opds://example.com/foo/bar") ) XCTAssertNotEqual( - UnknownAbsoluteURL(string: "opds://domain.com")!, - UnknownAbsoluteURL(string: "opds://domain.com#fragment")! + UnknownAbsoluteURL(string: "opds://example.com/foo/baz")!, + UnknownAbsoluteURL(string: "opds://example.com/foo/bar") + ) + + // Paths is compared percent and entity-decoded. + XCTAssertEqual( + UnknownAbsoluteURL(string: "opds://example.com/c%27est%20valide")!, + UnknownAbsoluteURL(string: "opds://example.com/c%27est%20valide") + ) + XCTAssertEqual( + UnknownAbsoluteURL(string: "opds://example.com/c'est%20valide")!, + UnknownAbsoluteURL(string: "opds://example.com/c%27est%20valide") + ) + + // Authority must be equal. + XCTAssertEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com:80/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com:80/foo")!, + UnknownAbsoluteURL(string: "opds://example.com:443/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com:80/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://domain.com/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://user:password@example.com/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://user:password@example.com/foo")!, + UnknownAbsoluteURL(string: "opds://other:password@example.com/foo") + ) + + // Order of query parameters is important. + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?b=b&a=a")!, + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?a=a&b=b") + ) + + // Content of parameters is important. + XCTAssertEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?a=a&b=b")!, + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?a=a&b=b") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?b=b")!, + UnknownAbsoluteURL(string: "opds://example.com/foo/bar?a=a") + ) + + // Scheme is case insensitive. + XCTAssertEqual( + UnknownAbsoluteURL(string: "OPDS://example.com/foo")!, + UnknownAbsoluteURL(string: "opds://example.com/foo") + ) + + // Fragment is relevant. + XCTAssertEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo#fragment")!, + UnknownAbsoluteURL(string: "opds://example.com/foo#fragment") + ) + XCTAssertNotEqual( + UnknownAbsoluteURL(string: "opds://example.com/foo#other")!, + UnknownAbsoluteURL(string: "opds://example.com/foo#fragment") ) } diff --git a/Tests/SharedTests/Toolkit/URL/RelativeURLTests.swift b/Tests/SharedTests/Toolkit/URL/RelativeURLTests.swift index aae6e7923..7c4ada1af 100644 --- a/Tests/SharedTests/Toolkit/URL/RelativeURLTests.swift +++ b/Tests/SharedTests/Toolkit/URL/RelativeURLTests.swift @@ -12,18 +12,25 @@ import XCTest class RelativeURLTests: XCTestCase { func testEquality() { - XCTAssertEqual( - RelativeURL(string: "dir/file")!, - RelativeURL(string: "dir/file")! - ) - XCTAssertNotEqual( - RelativeURL(string: "dir/file/")!, - RelativeURL(string: "dir/file")! - ) - XCTAssertNotEqual( - RelativeURL(string: "dir")!, - RelativeURL(string: "dir/file")! - ) + // Paths must be equal. + XCTAssertEqual(RelativeURL(string: "foo/bar")!, RelativeURL(string: "foo/bar")) + XCTAssertNotEqual(RelativeURL(string: "foo/bar")!, RelativeURL(string: "foo/bar/")) + XCTAssertNotEqual(RelativeURL(string: "foo/baz")!, RelativeURL(string: "foo/bar")) + + // Paths is compared percent and entity-decoded. + XCTAssertEqual(RelativeURL(string: "c%27est%20valide")!, RelativeURL(string: "c%27est%20valide")) + XCTAssertEqual(RelativeURL(string: "c'est%20valide")!, RelativeURL(string: "c%27est%20valide")) + + // Order of query parameters is important. + XCTAssertNotEqual(RelativeURL(string: "foo/bar?b=b&a=a")!, RelativeURL(string: "foo/bar?a=a&b=b")) + + // Content of parameters is important. + XCTAssertEqual(RelativeURL(string: "foo/bar?a=a&b=b")!, RelativeURL(string: "foo/bar?a=a&b=b")) + XCTAssertNotEqual(RelativeURL(string: "foo/bar?b=b")!, RelativeURL(string: "foo/bar?a=a")) + + // Fragment is relevant. + XCTAssertEqual(RelativeURL(string: "foo/bar#fragment")!, RelativeURL(string: "foo/bar#fragment")) + XCTAssertNotEqual(RelativeURL(string: "foo/bar#other")!, RelativeURL(string: "foo/bar#fragment")) } // MARK: - URLProtocol From b62f9696e162b9797af314a644912423bf853fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Menu?= Date: Tue, 11 Jun 2024 16:22:32 +0200 Subject: [PATCH 2/2] Fix hash value --- Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift | 8 ++++++-- Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift | 8 ++++++++ .../Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift | 10 ++++++++++ Sources/Shared/Toolkit/URL/RelativeURL.swift | 6 ++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift index 631d30590..0dd0f6200 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/FileURL.swift @@ -67,9 +67,13 @@ public struct FileURL: AbsoluteURL, Hashable, Sendable { try (url.resourceValues(forKeys: [.isDirectoryKey]).isDirectory) ?? false } + public func hash(into hasher: inout Hasher) { + hasher.combine(path) + hasher.combine(url.user) + } + public static func == (lhs: Self, rhs: Self) -> Bool { - lhs.origin == rhs.origin - && lhs.path == rhs.path + lhs.path == rhs.path && lhs.url.user == rhs.url.user } } diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift index 8c1198967..18273e042 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/HTTPURL.swift @@ -36,6 +36,14 @@ public struct HTTPURL: AbsoluteURL, Hashable, Sendable { return o } + public func hash(into hasher: inout Hasher) { + hasher.combine(origin) + hasher.combine(path) + hasher.combine(query) + hasher.combine(fragment) + hasher.combine(url.user) + } + public static func == (lhs: Self, rhs: Self) -> Bool { lhs.origin == rhs.origin && lhs.path == rhs.path diff --git a/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift b/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift index 7457a74ff..d23175d7c 100644 --- a/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift +++ b/Sources/Shared/Toolkit/URL/Absolute URL/UnknownAbsoluteURL.swift @@ -24,6 +24,16 @@ struct UnknownAbsoluteURL: AbsoluteURL, Hashable { let scheme: URLScheme let origin: String? = nil + public func hash(into hasher: inout Hasher) { + hasher.combine(scheme) + hasher.combine(host) + hasher.combine(url.port) + hasher.combine(path) + hasher.combine(query) + hasher.combine(fragment) + hasher.combine(url.user) + } + public static func == (lhs: Self, rhs: Self) -> Bool { lhs.scheme == rhs.scheme && lhs.host == rhs.host diff --git a/Sources/Shared/Toolkit/URL/RelativeURL.swift b/Sources/Shared/Toolkit/URL/RelativeURL.swift index 72a32eba5..e37354d45 100644 --- a/Sources/Shared/Toolkit/URL/RelativeURL.swift +++ b/Sources/Shared/Toolkit/URL/RelativeURL.swift @@ -97,6 +97,12 @@ public struct RelativeURL: URLProtocol, Hashable { ) } + public func hash(into hasher: inout Hasher) { + hasher.combine(path) + hasher.combine(query) + hasher.combine(fragment) + } + public static func == (lhs: Self, rhs: Self) -> Bool { lhs.path == rhs.path && lhs.query == rhs.query