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

fix(router): Ensure url params are decoded consistently across expo-router #25589

Merged
merged 11 commits into from
Dec 11, 2023

Conversation

marklawlor
Copy link
Contributor

Why

useGlobalSearchParams and useLocalSearchParams treat URL search parameters differently.

Fix #25570
Fix #25323

How

I'm not 100% happy with this solution, I thought we could do it more in the getStateFromPath/getPathFromState but I couldn't get it working. Instead we just decode them directly before saving to the global store, or when accessing from the navigation context.

Test Plan

Unit tests cover multiple scenarios, ensuring the parameters are consistently decoded.

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Nov 27, 2023
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 27, 2023
@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions and removed bot: passed checks ExpoBot has nothing to complain about labels Nov 27, 2023
@marklawlor marklawlor force-pushed the marklawlor/router/decodeUriComponent branch from 17c041d to a462fe8 Compare November 27, 2023 21:59
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 27, 2023
Copy link
Member

@ide ide left a comment

Choose a reason for hiding this comment

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

Add a regression test for invalid URL-encoded paths/parameters, such as /start%end -- it's useful to define and document the behavior of invalid URLs.

packages/expo-router/src/__tests__/navigation.test.ios.tsx Outdated Show resolved Hide resolved
packages/expo-router/src/hooks.ts Outdated Show resolved Hide resolved
@marklawlor marklawlor force-pushed the marklawlor/router/decodeUriComponent branch from c69b612 to 12ed264 Compare December 4, 2023 04:34
@marklawlor marklawlor merged commit ad9e27c into main Dec 11, 2023
9 checks passed
@marklawlor marklawlor deleted the marklawlor/router/decodeUriComponent branch December 11, 2023 02:40
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
…outer (expo#25589)

# Why

`useGlobalSearchParams` and `useLocalSearchParams` treat URL search
parameters differently.

Fix expo#25570
Fix expo#25323

# How

I'm not 100% happy with this solution, I thought we could do it more in
the `getStateFromPath`/`getPathFromState` but I couldn't get it working.
Instead we just decode them directly before saving to the global store,
or when accessing from the navigation context.

# Test Plan

Unit tests cover multiple scenarios, ensuring the parameters are
consistently decoded.

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@rileysk3
Copy link

rileysk3 commented Mar 3, 2024

Noob here. Just updated to sdk50 from 49. If I pass a firebase image url through local params It becomes invalid due to the decoding removing %2f. Use case is searching a user and then passing the details to profile screen.

error for image:
error code is 400, "Bad Request.".

There a way to revert or disable decoding?

@ide
Copy link
Member

ide commented Mar 3, 2024

It sounds like the custom parameters might need to be URL-encoded when serializing them. For now you could probably work around the issue by double-encoding your URL.

@rileysk3
Copy link

rileysk3 commented Mar 3, 2024

Thanks, works for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
4 participants