Skip to content

Fix NSURLComponents bridging with percent-encoding #831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

jrflat
Copy link
Contributor

@jrflat jrflat commented Aug 6, 2024

Resolves #828 (thanks @giginet for filing!)

Because the bridging code assigns non-percentEncoded variants of the components (e.g. .path instead of .percentEncodedPath), an NSURLComponents that is bridged to Swift URLComponents loses its percent-encoding. For example:

let nsURLComponents = NSURLComponents(
    string: "https://example.com?url=https%3A%2F%2Fapple.com"
)!
let urlComponents = nsURLComponents as URLComponents
print(nsURLComponents.string) // "https://example.com?url=https%3A%2F%2Fapple.com"
print(urlComponents.string) // "https://example.com?url=https://apple.com"

Note we only lose percent-encoding if the percent-encoding is not absolutely required, e.g. "://" does not require encoding in the query component, but this is something we should fix regardless.

This PR first checks if we can cast the NSURLComponents to a _NSSwiftURLComponents and take the components right out of it. If the cast fails, we fall back to assigning the percentEncoded properties.

@jrflat
Copy link
Contributor Author

jrflat commented Aug 6, 2024

@swift-ci please test

@jrflat jrflat requested a review from jmschonfeld August 6, 2024 19:46
Copy link

@giginet giginet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I couldn't address this because it requires FOUNDATION_FRAMEWORK

@jrflat jrflat merged commit 843bf80 into swiftlang:main Aug 21, 2024
3 checks passed
cthielen pushed a commit to cthielen/swift-foundation that referenced this pull request Nov 8, 2024
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.

Bridging is not identical from NSURLComponents to URLComponents
3 participants