Skip to content
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

[core] fix incorrect conversion of valid url #23331

Merged

Conversation

alanjhughes
Copy link
Collaborator

Why

Closes #23282
Some unusual but valid URLs will fail to be created with URL(string:)
example: https://matrix.to/#/#sh.itjust.works:matrix.org
When this happens the URL is assumed to be a file which then crashes the app once it's passed to the browser

How

Create the URL with URLComponents which ensures it is RFC 3986 compliant.

Test Plan

Tested with multiple variations of URL and the one causing the issue. All work correctly.

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jul 6, 2023
@alanjhughes alanjhughes changed the title fix: incorrect conversion of valid url [core] fix incorrect conversion of valid url Jul 6, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jul 6, 2023
@alanjhughes alanjhughes marked this pull request as ready for review July 6, 2023 13:11
Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

One related unit test fails. We can consider removing it though. What do you think?

packages/expo-modules-core/CHANGELOG.md Outdated Show resolved Hide resolved
@alanjhughes
Copy link
Collaborator Author

Yes, I think it's fine to remove it. The encoding get's handled correctly so we don't need that one

@alanjhughes alanjhughes requested a review from tsapeta July 6, 2023 19:35
@alanjhughes alanjhughes merged commit c377b25 into expo:main Jul 7, 2023
8 of 10 checks passed
@alanjhughes alanjhughes deleted the @alanhughes/ios/core-url-conversion branch July 7, 2023 08:26
tsapeta added a commit that referenced this pull request Jul 7, 2023
tsapeta added a commit that referenced this pull request Jul 7, 2023
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expo-web-browser cannot handle links with more than one # in the URL on iOS
4 participants