Skip to content

Add baseURL to URL's hash calculation#1799

Open
marcprux wants to merge 3 commits intoswiftlang:mainfrom
swift-android-sdk:URL-hash-baseURL
Open

Add baseURL to URL's hash calculation#1799
marcprux wants to merge 3 commits intoswiftlang:mainfrom
swift-android-sdk:URL-hash-baseURL

Conversation

@marcprux
Copy link
Copy Markdown
Contributor

@marcprux marcprux commented Mar 6, 2026

Include baseURL in URL's hash calculation along with relativeString

Motivation:

In scenarios where a URL is expressing a common path (e.g., https://example.com/robots.txt), multiple individual URLs might be created with the same relativeString but different baseURL. Currently, the hash of these instances will all be the same, despite them having a different baseURL. If URLs like these are used as keys in a Dictionary, this will lead to O(N) lookup costs which has performance implications and could introduce a denial-of-service attack vector.

marcprux@skipbox:~$ swiftly run swift repl
Welcome to Swift version 6.3-dev (LLVM 4f7a3dbb7e97ac4, Swift 477d32c08627ddd).
Type :help for assistance.
  1> import Foundation
  2> URL(string: "robots.txt", relativeTo: URL(string: "https://example.com")).hashValue
$R0: Int = 4059451536836375387
  3> URL(string: "robots.txt", relativeTo: URL(string: "https://example.org")).hashValue
$R1: Int = 4059451536836375387

Modifications:

Add baseURL to URL's hash calculation.

Result:

The hash will be improved.

Testing:

hashIncludesBaseURL in Tests/FoundationEssentialsTests/URLTests.swift

@marcprux marcprux requested a review from a team as a code owner March 6, 2026 22:07
@marcprux
Copy link
Copy Markdown
Contributor Author

marcprux commented Mar 6, 2026

I'll also note that this behavior matches CFURL's __CFURLHash , which hashes on the entire string representation of the URL (CFHash(CFURLGetString((CFURLRef)cf))) rather than just the relativeString. Oops, it doesn't, as the subsequent comment points out.

@jrflat
Copy link
Copy Markdown
Contributor

jrflat commented Mar 6, 2026

CFURLGetString only returns the relative string, so we would be deviating from that, but that's probably okay. This should also be okay performance-wise since the baseURL is guaranteed to be absolute, i.e. we won't do a ton of recursing. Though we could also be explicit about using baseURL?.relativeString if necessary.

@itingliu itingliu requested a review from jrflat April 7, 2026 21:37
@jrflat
Copy link
Copy Markdown
Contributor

jrflat commented Apr 7, 2026

The motivation makes sense and should be good implementation-wise. @marcprux do you want to resolve the merge conflict (probably a test I added, sorry!), then we'll get this tested and merged?

@marcprux
Copy link
Copy Markdown
Contributor Author

marcprux commented Apr 8, 2026

Resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants