Repository navigation
Fix double encoding characters in external deep link paths - #605
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bkuhl
marked this pull request as ready for review
October 24, 2026 22:21
1 task done
gwleuverink
added a commit
that referenced
this pull request
Nov 2, 2026
Covers mobile PRs #525, #527, #528, #560, #605, #608, #609, #611, #613, #614, #615, #616, #617, #619, #621, #622, #623, #624, #625, #626, #627, #628, #629, #632, #633, #634, #635 and #636, and the commits on main without a PR: 4082538 (bounded()'s docblock), 49528ba (the Android logging fix from a fork), b23aba9, 2a773cc and b9316b6 (security policy, issue template and README links). Main b107ed9..767a7c8, 33 commits, 4.6.1 among them. Ported to core on branch sync/mobile-main-2026-11-01, which this merge needs: #608 (scrollVirtualListTo()), #625 with 4082538 (snap points on a gesture area), #621 (:context-menu on a pressable, matched through core's ConvertsToElement as :menu is), #617's farewell frame through a hook, and #614's ValidatesInput and FieldErrors, which move to core. The old names of those two resolve to core's through the alias shim. Kept on this branch: #616 in full. EVENT_SYSTEM_BACK sits on NativeComponent next to EVENT_HOT_RELOAD and EVENT_SHUTDOWN, the router half is in NativeRouter, and InteractsWithAppShell's back button handler checks the constant, as main's runLoop() does. #617's element types: NativeComponent answers core's typesClosedInFarewellFrame() hook with bottom_sheet and modal. BroadcastsGlobally extends core's marker and #609's DispatchableFromNative. Removed again in the merge: HasVirtualListWindow, GestureArea and Pressable (main changed them, core has them), and ValidatesInput and FieldErrors (main added them, core has them). Main's import of the old dispatcher name came out of DispatchEventFromAppController, where it clashed with core's. Adapted for the extraction: BroadcastsGloballyNamesTest expects DispatchableFromNative among the old marker's interfaces, as #609 makes it. #609 is merged as main has it and is not ported. Needs NativePHP/core#18. Tests, all on PHP 8.4.27, with dev dependencies updated for #628 (Laravel 12.70.2, Pest 3.8.7): mobile on this merge against the core sync branch (445049b), 2,467 passed, 4 risky, 4 skipped. PHPStan clean. Mobile main (767a7c8), 2,412 passed, 4 risky, 4 skipped. Desktop main (fd40580) against the core sync branch, test:php 2,240 passed, as against core main. Desktop #57 (977a2af), 2,245 passed against the earlier core sync head (59c73fc), which differs only by the constant moved to mobile; not rerun.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why it matters
Say someone emails a link to a page in the app, and the link has encoded characters in its path, like
https://example.com/board/2-A%2541B. Tapping it opens the app, but the page doesn't load. The app is handed a different path than the one in the link, so the page breaks. The same link works in a browser.This follows up the gap #581 left open. #581 kept
uri.pathdecoded "to keep the platforms matching," because it "only matters for links that put free text in the path, and if it ever does it should be fixed on both platforms at once." This PR makes that change on both platforms, for a case where it does matter.The inconsistency
On a server, Laravel receives the path still percent-encoded and decodes it once. mobile-air already does the same nearly everywhere:
PHPSchemeHandlerusespercentEncodedPath, andPHPWebViewClientandWebViewManageruseencodedPath.NativeServiceProviderbuilds the target fromrequest()->path(), which is encoded (profile/Ann%20Lee%3F), and sends it as a__deeplinkevent throughNativeRouter::openOnMainLane().The shells' deep link entry points are the exception. iOS
DeepLinkRouterusesurl.pathand AndroidhandleDeepLinkIntentusesuri.path, and both of those are already decoded. SoNativeRouterreceives__deeplinkURIs in two different forms: encoded from PHP, decoded from the shells. It then decodes route parameters once more (Laravel'sdecodedPath()), so a shell link is decoded twice.With
Route::native('/profile/{name}', …)on 4.6.0:/profile/Ann%20Lee,/profile/Jos%C3%A9Ann Lee,JoséAnn Lee,José(unchanged)/profile/A%2541BAABA%41B/profile/Q%3F%23xQ(cut at?)Q?#xOrdinary paths come out the same either way. Only paths that carry a literal
%, or an encoded/,?or#, change, and those are wrong today.Changes
DeepLinkRouter.handleusesurl.path(percentEncoded: true)for both custom schemes and universal links. The deployment target is 18.2, so the API is available.NativePHPAppnative-session exit has the same issue. When a warm__deeplinklands on a web route, the native session exits with aLocation, and the shell kepturl.path, which decoded the path again. It now keeps the encoded path, next to the encoded query Keep the query when a native screen exits to a web route #583 added.PHPSchemeHandlerorphaned EXIT_WEB fallback gets the same one-line change. This path only runs when the scheme task was cancelled, and I didn't hit it on a device.MainActivityusesuri.encodedPathfor the FCMurl/linkextra and for intent data (custom schemes and https app links), matching theencodedQueryfrom Android: forward deep link queries still percent-encoded #581.handleNativeSessionExit()already usesencodedPath.For paths without escapes, the old and new code give the same result. I checked on Foundation (macOS 26):
https://host""https://host//myapp://auth/callback/callbackmyapp://""Not changed: on iOS,
HotReloadServerre-runs the native session fromwebView.url.path, which is decoded and has no query. It's dev-only, so I left it out to keep this focused.Testing
I ran on an iOS simulator (iOS 26 runtime, deployment target 18.2) and an Android emulator (Medium_Phone AVD). The app was a Laravel app on NativePHP 4.6.0, with this PR's changes applied on top. It boots into a native-ui
Route::native('/app', ...)shell, and a web route at/board/{token}(where('token', '\d+-.*')) logs the token it receives. Universal links don't verify on simulator builds, so iOS used a custom scheme (bbtest://board/…), which goes through the sameurl.pathcode inDeepLinkRouter. Android used real https app links viaadb shell am start -a android.intent.action.VIEW -d https://… <package>.Synthetic token: the data is
2-A%41B%20C%2FD+E, sent as…/board/2-A%2541B%2520C%252FD%2BE. This is what the route received:__deeplink→ EXIT_WEB)2-AAB C/D+E2-AAB C/D+E2-AAB C/D+E2-A%41B%20C%2FD+E✅2-A%41B%20C%2FD+E✅2-A%41B%20C%2FD+E✅2-AAB C/D+E2-AAB C/D+E2-AAB C/D+E2-A%41B%20C%2FD+E✅2-A%41B%20C%2FD+E✅2-A%41B%20C%2FD+E✅The
NativePHPApp.swiftchange is needed on its own. With only theDeepLinkRouterchange, iOS warm-with-native-ui still gave2-AAB C/D+E:__deeplinknow carries/board/2-A%2541B…, but the exit location'surl.pathdecoded it again.Real payloads: a 3,052-character Base45 token (team) on Android, and a 1,285-character one (individual) on iOS:
400 Bad Request, cold and warm.400 Bad Requestwhen warm with a web page in front. When cold, this payload happened to come through intact, because Foundation'sURL(string:)re-escapes%XXsequences that don't decode to valid UTF-8, and all of this payload's sequences are of that kind. The synthetic%41/%20row shows that the cold path corrupts too.Links without escapes (
/r/ABC234) opened the same before and after:Two caveats:
xcrun simctl openurlcuts custom-scheme URLs to about 2,070 characters beforeDeepLinkRouter.handlesees them, so the 3,052-character payload couldn't be tested on iOS. I don't know whether universal links on a device have the same limit; it happens before NativePHP either way.main. On 4.6.0 theNativePHPAppexit line has no query handling yet (that came with Keep the query when a native screen exits to a web route #583). The PR applies the samepath(percentEncoded: true)change tomain's version of that line.No PHP changed, so I didn't run
composer test: the dev dependencies wouldn't resolve on my machine.🤖 Generated with Claude Code