diff --git a/config/scripts/mobile-web-app-render.test.mjs b/config/scripts/mobile-web-app-render.test.mjs index 79cc08f5c9b..8bbd13ac9d0 100644 --- a/config/scripts/mobile-web-app-render.test.mjs +++ b/config/scripts/mobile-web-app-render.test.mjs @@ -103,7 +103,7 @@ async function readBridgeFaultGrant() { * place domain behaviour is decided, and every screen below already has a state for an RPC that * failed. The one message that matters here is the one that lets the tree mount. */ -function installShellDouble({ version, sessionId, buildId, faultGrant }) { +function installShellDouble({ version, sessionId, buildId, route, faultGrant }) { // Where the page's own fault reports land. Read back after the render, so a route that threw // under the boundary names itself instead of timing out as a page that never mounted. globalThis.__orcaRenderCheckFaults = [] @@ -133,7 +133,9 @@ function installShellDouble({ version, sessionId, buildId, faultGrant }) { grants: { rpc: { maxPendingRequests: 64, maxSubscriptions: 32 }, native: [faultGrant] - } + }, + // Omitted for a shell too old to name one, which is the case the page has a panel for. + ...(route === null ? {} : { route }) }) return } @@ -249,16 +251,20 @@ const UNMATCHED = 'Unmatched Route' * A page with every signal the checks below read: uncaught errors, console errors, and the script * paths the browser actually fetched. The last one is how a client-side navigation proves it * pulled the next route's chunk rather than painting out of what the entry already had. + * + * No `shellRoute` installs no double at all, which is the page that never mounts; a null one + * installs a shell that named no screen. */ -async function openPage({ shell = true } = {}) { +async function openPage({ shellRoute } = {}) { const page = await browser.newPage({ viewport: { width: 390, height: 844 } }) - if (shell) { + if (shellRoute !== undefined) { // At document start, where the native shell installs the real channel: the entry reads it // while its own script runs, so a channel added after `load` would already be too late. await page.addInitScript(installShellDouble, { version: bridgeVersion, sessionId: SHELL_SESSION_ID, buildId: SHELL_BUILD_ID, + route: shellRoute, faultGrant }) } @@ -339,9 +345,14 @@ async function waitForRoute({ page, errors, uncaught }, route, awaitText) { } } -async function render(route, awaitText) { - const opened = await openPage() - await opened.page.goto(`${origin}${route}`, { waitUntil: 'load' }) +/** + * Opens the document the way the shell does — at `/`, the one path it serves — and lets the page + * route itself from what the double names. Navigating straight to the route would hide exactly the + * step this check exists to prove. + */ +async function render(route, awaitText, { shellRoute = { pathname: route } } = {}) { + const opened = await openPage({ shellRoute }) + await opened.page.goto(`${origin}/`, { waitUntil: 'load' }) await waitForRoute(opened, route, awaitText) const text = await opened.page.evaluate(() => document.body.innerText) // What the page believes it is: read off the document rather than off the double, so a tree that @@ -350,6 +361,9 @@ async function render(route, awaitText) { sessionId: document.documentElement.dataset.orcaWebSessionId ?? null, buildId: document.documentElement.dataset.orcaWebBuildId ?? null })) + // The document is served at "/" and the page rewrites its own path before it renders; without + // that, every route below would be expo-router's Unmatched screen. + const url = await opened.page.evaluate(() => location.pathname + location.search) await opened.page.close() // A CSP refusal reaches the page as a console error, so the caller's empty-errors assertion is // also the policy assertion; name it here so a failure says which one broke. @@ -357,20 +371,23 @@ async function render(route, awaitText) { errors: opened.errors, cspErrors: opened.errors.filter((entry) => entry.includes('Content Security Policy')), text, - session + session, + url } } -/** The entry's state and what it painted, for a page that is never going to mount. */ -async function renderUnbridged(route) { - const { page, errors } = await openPage({ shell: false }) +/** The entry's state and what it painted, for a page that is never going to mount a route tree. */ +async function renderWithoutTree({ shellRoute } = {}) { + const { page, errors } = await openPage({ shellRoute }) // Read straight after `load` and not polled: the entry decides this synchronously, inside the // script `load` waits for, so a state that is not settled by now is never going to settle. - await page.goto(`${origin}${route}`, { waitUntil: 'load' }) + await page.goto(`${origin}/`, { waitUntil: 'load' }) const entry = await page.evaluate(() => document.documentElement.dataset.orcaWebEntry ?? 'absent') const rootChildren = await page.evaluate(() => document.getElementById('root').childElementCount) + const text = await page.evaluate(() => document.body.innerText) + const url = await page.evaluate(() => location.pathname + location.search) await page.close() - return { entry, errors, rootChildren } + return { entry, errors, rootChildren, text, url } } describe('the shell policy this page is tested under', () => { @@ -436,11 +453,13 @@ describeRender('the page server this check runs against', () => { describeRender('the Route A page in a real browser', () => { it('mounts the worktree list route, not the unmatched screen', async () => { - const { errors, cspErrors, text, session } = await render(HOST_ROUTE, 'Host not found') + const { errors, cspErrors, text, session, url } = await render(HOST_ROUTE, 'Host not found') expect(cspErrors).toEqual([]) expect(errors).toEqual([]) // The tree that mounted is the one the shell handed a session to, and it says which. expect(session).toEqual({ sessionId: SHELL_SESSION_ID, buildId: SHELL_BUILD_ID }) + // The document was served at `/`; the page put itself on the route the shell named. + expect(url).toBe(HOST_ROUTE) // app/h/[hostId]/index.tsx: expo-secure-store is {} on web, so loadHosts() finds no profile // and the list paints its not-found state. Only that route's own component produces this // string, and C1.4's host-store.web.ts is what replaces it with a real row. @@ -467,21 +486,39 @@ describeRender('the Route A page in a real browser', () => { expect(text).toContain(UNMATCHED) }, 60_000) - it('mounts nothing at all when no shell answered, which is what makes the three above real', async () => { - const { entry, errors, rootChildren } = await renderUnbridged(HOST_ROUTE) + it('carries the params the shell named into the url the screen reads', async () => { + const { errors, url } = await render(HOST_ROUTE, 'Host not found', { + shellRoute: { pathname: HOST_ROUTE, params: { from: 'render check' } } + }) + expect(errors).toEqual([]) + expect(url).toBe(`${HOST_ROUTE}?from=render+check`) + }, 60_000) + + it('mounts nothing at all when no shell answered, which is what makes the rest real', async () => { // Without this the checks above would pass against a page that ignores `init` entirely. + const { entry, errors, rootChildren } = await renderWithoutTree() expect(entry).toBe('unbridged') expect(rootChildren).toBe(0) expect(errors).toEqual([]) }, 60_000) + it('says to update the app when the shell that opened it named no screen', async () => { + const { entry, errors, text, url } = await renderWithoutTree({ shellRoute: null }) + expect(entry).toBe('shell-too-old') + expect(errors).toEqual([]) + expect(text).toContain('Update Orca to open this workspace') + // Never the route tree at `/`: that is the Unmatched screen with a worse explanation. + expect(text).not.toContain(UNMATCHED) + expect(url).toBe('/') + }, 60_000) + it('tells the shell when a route chunk throws, rather than sitting on a blank page', async () => { const chunk = routeChunks['./h/[hostId]/index.tsx'] expect(chunk, Object.keys(routeChunks).join(' ')).toBeTruthy() poisonedChunks.add(`/assets/${chunk}`) try { - const opened = await openPage() - await opened.page.goto(`${origin}${HOST_ROUTE}`, { waitUntil: 'load' }) + const opened = await openPage({ shellRoute: { pathname: HOST_ROUTE } }) + await opened.page.goto(`${origin}/`, { waitUntil: 'load' }) const reported = await opened.page .waitForFunction( () => { @@ -507,9 +544,9 @@ describeRender('the Route A page in a real browser', () => { }, 60_000) it("fetches the next route's chunks on a client-side navigation", async () => { - const opened = await openPage() + const opened = await openPage({ shellRoute: { pathname: HOST_ROUTE } }) const { page, errors, scripts } = opened - await page.goto(`${origin}${HOST_ROUTE}`, { waitUntil: 'load' }) + await page.goto(`${origin}/`, { waitUntil: 'load' }) await waitForRoute(opened, HOST_ROUTE, 'Host not found') const loadedForFirstRoute = [...scripts] // What the shell will do in C1.2: the document is fetched once and every later route is a diff --git a/mobile/app/h/[hostId]/web.tsx b/mobile/app/h/[hostId]/web.tsx index 2425b1e00f1..9d32a8628d0 100644 --- a/mobile/app/h/[hostId]/web.tsx +++ b/mobile/app/h/[hostId]/web.tsx @@ -43,7 +43,17 @@ export default function MobileWebShellRoute() { if (!enabled || !hostId) { return } - return + // The screen the page stands in for. The document is served at `/`, which matches no route in + // the tree the page carries, so this is the only thing that tells it which one to open. + // Encoded, not interpolated raw: `hostId` arrives decoded from the URL, so one carrying `?`, `#` + // or whitespace would build a pathname the page refuses and never mount anything. The page + // decodes it back when it matches `[hostId]`, so the screen it opens is the same one. + return ( + + ) } const styles = StyleSheet.create({ diff --git a/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellLoadState.kt b/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellLoadState.kt index 7836824a87c..431b557f1bb 100644 --- a/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellLoadState.kt +++ b/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellLoadState.kt @@ -65,7 +65,18 @@ internal class MobileWebShellLoadStateMachine { fun started(): MobileWebShellLoadEmission? = emit(MobileWebShellLoadEmission("loading", null)) - fun finished(): MobileWebShellLoadEmission? = emit(MobileWebShellLoadEmission("ready", null)) + /** + * A load that never committed did not finish. + * + * This is the whole guard, and it is deliberately not the document's URL: the page rewrites its + * own path with `history.replaceState` before its first render, so the document that committed at + * "/" reports finishing at "/h/". Reading the path here withheld `ready` forever and left + * the WebView hidden behind it. + */ + fun finished(): MobileWebShellLoadEmission? { + if (!hasCommittedDocument) return null + return emit(MobileWebShellLoadEmission("ready", null)) + } fun failed(reason: MobileWebShellFailureReason): MobileWebShellLoadEmission? { val emission = emit(MobileWebShellLoadEmission("failed", reason.wireName)) diff --git a/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellView.kt b/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellView.kt index 866d05cf77d..fcce85d24fd 100644 --- a/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellView.kt +++ b/mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellView.kt @@ -373,9 +373,14 @@ internal class OrcaMobileWebShellView( emit(loadState.started()) } + // No URL check: the page rewrites its own path with history.replaceState before its first + // render, so the document that committed at "/" finishes at the route it opened. What is left + // is whether this is the document the caller was told about, which is what committing means. override fun onPageFinished(view: WebView, url: String) { - if (documentFailed || !isDocumentUrl(Uri.parse(url))) return + if (documentFailed || !loadState.hasCommittedDocument) return view.visibility = View.VISIBLE + // After the rewrite as well as before it: the back-forward list is the page's, and the shell + // gives it no way back to a document it has already replaced. view.clearHistory() emit(loadState.finished()) } diff --git a/mobile/modules/orca-mobile-web-shell/android/src/test/java/expo/modules/orcamobilewebshell/MobileWebShellLoadStateTest.kt b/mobile/modules/orca-mobile-web-shell/android/src/test/java/expo/modules/orcamobilewebshell/MobileWebShellLoadStateTest.kt index 71188f49657..70b7fe68c1f 100644 --- a/mobile/modules/orca-mobile-web-shell/android/src/test/java/expo/modules/orcamobilewebshell/MobileWebShellLoadStateTest.kt +++ b/mobile/modules/orca-mobile-web-shell/android/src/test/java/expo/modules/orcamobilewebshell/MobileWebShellLoadStateTest.kt @@ -52,6 +52,7 @@ class MobileWebShellLoadStateTest { fun `reports a load in progress and then a load that finished`() { val machine = MobileWebShellLoadStateMachine() assertEquals(MobileWebShellLoadEmission("loading", null), machine.started()) + machine.committed() assertEquals(MobileWebShellLoadEmission("ready", null), machine.finished()) } @@ -60,10 +61,31 @@ class MobileWebShellLoadStateTest { val machine = MobileWebShellLoadStateMachine() assertNotNull(machine.started()) assertNull(machine.started()) + machine.committed() assertNotNull(machine.finished()) assertNull(machine.finished()) } + // The page rewrites its own path with history.replaceState before its first render, so + // onPageFinished arrives at a URL the navigation policy would refuse. The path is deliberately + // not an input: what is asked is whether this load committed. + @Test + fun `a load that finished without committing reports nothing`() { + val machine = MobileWebShellLoadStateMachine() + machine.started() + assertNull(machine.finished()) + machine.committed() + assertEquals(MobileWebShellLoadEmission("ready", null), machine.finished()) + } + + @Test + fun `a load whose document was replaced mid-flight reports nothing`() { + val machine = MobileWebShellLoadStateMachine() + machine.committed() + machine.documentEnded() + assertNull(machine.finished()) + } + // Chromium commits its error document after onReceivedError returns, so onPageFinished arrives // after the failure; reporting `ready` there would also un-hide the error page. @Test @@ -99,6 +121,7 @@ class MobileWebShellLoadStateTest { val epoch = machine.epoch machine.reset() assertNull(machine.failedDuring(epoch, MobileWebShellFailureReason.DOCUMENT_LOAD_FAILED)) + machine.committed() assertEquals(MobileWebShellLoadEmission("ready", null), machine.finished()) } diff --git a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellLoadState.swift b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellLoadState.swift index 200b5330c00..800202a5015 100644 --- a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellLoadState.swift +++ b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellLoadState.swift @@ -50,8 +50,14 @@ final class MobileWebShellLoadStateMachine { emit(MobileWebShellLoadEmission(state: "loading", reason: nil)) } + /// A load that never committed did not finish. + /// + /// This is the whole guard, and it is deliberately not the document's URL: the page rewrites its + /// own path with `history.replaceState` before its first render, so the document that committed + /// at "/" reports finishing at "/h/". Reading the path here withheld `ready` forever. func finished() -> MobileWebShellLoadEmission? { - emit(MobileWebShellLoadEmission(state: "ready", reason: nil)) + guard hasCommittedDocument else { return nil } + return emit(MobileWebShellLoadEmission(state: "ready", reason: nil)) } func failed(_ reason: MobileWebShellFailureReason) -> MobileWebShellLoadEmission? { diff --git a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift index c6ad6891ffa..c404236ecf5 100644 --- a/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift +++ b/mobile/modules/orca-mobile-web-shell/ios/MobileWebShellView.swift @@ -495,8 +495,9 @@ final class OrcaMobileWebShellView: ExpoView, WKNavigationDelegate, WKUIDelegate loadState.committed() } + /// No URL check: the page rewrites its own path before its first render, so the document that + /// committed at "/" finishes at the route it opened. `finished()` holds the rule that is left. func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - guard isDocumentUrl(webView.url) else { return } emit(loadState.finished()) } diff --git a/mobile/modules/orca-mobile-web-shell/tests/MobileWebShellChecks.swift b/mobile/modules/orca-mobile-web-shell/tests/MobileWebShellChecks.swift index f7f3fdded09..4748b6c3310 100644 --- a/mobile/modules/orca-mobile-web-shell/tests/MobileWebShellChecks.swift +++ b/mobile/modules/orca-mobile-web-shell/tests/MobileWebShellChecks.swift @@ -232,9 +232,25 @@ import Foundation let progress = MobileWebShellLoadStateMachine() precondition(progress.started()?.state == "loading") precondition(progress.started() == nil) + progress.committed() precondition(progress.finished()?.state == "ready") precondition(progress.finished() == nil) + // The document's path is not an input here, and that is the point: the page rewrites its own + // with history.replaceState before its first render, so `didFinish` arrives at a URL no policy + // would allow. What is asked instead is whether this load committed. + let unseated = MobileWebShellLoadStateMachine() + _ = unseated.started() + precondition(unseated.finished() == nil) + unseated.committed() + precondition(unseated.finished()?.state == "ready") + + // A document replaced mid-load: the finish belongs to the one that is already gone. + let replaced = MobileWebShellLoadStateMachine() + replaced.committed() + replaced.documentEnded() + precondition(replaced.finished() == nil) + // A rule list compiles asynchronously, so it can fail after the generation was already refused. let refused = MobileWebShellLoadStateMachine() precondition(refused.failed(.generationUnreadable)?.reason == "generation-unreadable") diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx index ca73006671e..dbcdb98845b 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.test.tsx @@ -89,7 +89,9 @@ async function render(state: MobileWebShellSessionState): Promise { - rendered.tree = create(createElement(MobileWebShellScreen, { hostId: 'host-1' })) + rendered.tree = create( + createElement(MobileWebShellScreen, { hostId: 'host-1', route: { pathname: '/h/host-1' } }) + ) }) if (rendered.tree === null) { throw new Error('screen did not render') @@ -111,7 +113,9 @@ function readyState(sessionId: string): MobileWebShellSessionState { async function update(tree: ReactTestRenderer, state: MobileWebShellSessionState): Promise { dependencies.state = state await act(async () => { - tree.update(createElement(MobileWebShellScreen, { hostId: 'host-1' })) + tree.update( + createElement(MobileWebShellScreen, { hostId: 'host-1', route: { pathname: '/h/host-1' } }) + ) }) } diff --git a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx index 5e4f756dfd6..50e3ee03d23 100644 --- a/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx +++ b/mobile/src/mobile-web-shell/MobileWebShellScreen.tsx @@ -7,6 +7,7 @@ import { } from '../../modules/orca-mobile-web-shell/src' import { ProtocolBlockScreen } from '../components/ProtocolBlockScreen' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import type { BridgeInitRoute } from './bridge/bridge-envelope' import type { MobileWebShellFailureCause, MobileWebShellSessionState @@ -109,6 +110,8 @@ function DevFacts({ state }: { state: Extract { + console.warn('[web-shell] refused to open this screen', issue) + reportShellFailure('document-load-failed') + } }) if (state.kind === 'wall') { diff --git a/mobile/src/mobile-web-shell/bridge-diagnostic-log.test.ts b/mobile/src/mobile-web-shell/bridge-diagnostic-log.test.ts index cf0ec0abf96..683426cf8fa 100644 --- a/mobile/src/mobile-web-shell/bridge-diagnostic-log.test.ts +++ b/mobile/src/mobile-web-shell/bridge-diagnostic-log.test.ts @@ -39,8 +39,16 @@ describe('the bridge diagnostic log', () => { const report = createBridgeDiagnosticReporter() report({ kind: 'frame-after-dispose' }) report({ kind: 'notify-refused', name: 'fault', why: 'before-ready' }) + report({ kind: 'route-refused', issue: 'the shell named no screen' }) expect(lines()[0]).toContain('outlived') expect(lines()[1]).not.toContain('outlived') + expect(lines()[2]).not.toContain('outlived') + }) + + it('carries what was wrong with the screen the shell named', () => { + const report = createBridgeDiagnosticReporter() + report({ kind: 'route-refused', issue: 'the shell named no screen' }) + expect(lines()[0]).toContain('the shell named no screen') }) it('carries the cause of the kinds that have one', () => { diff --git a/mobile/src/mobile-web-shell/bridge-diagnostic-log.ts b/mobile/src/mobile-web-shell/bridge-diagnostic-log.ts index c6e90ede345..51ed6d69b34 100644 --- a/mobile/src/mobile-web-shell/bridge-diagnostic-log.ts +++ b/mobile/src/mobile-web-shell/bridge-diagnostic-log.ts @@ -41,6 +41,13 @@ export function createBridgeDiagnosticReporter(): (diagnostic: BridgeHostDiagnos }) return } + if (diagnostic.kind === 'route-refused') { + // The shell's own bug, not the page's: this host serves no session at all until it is fixed. + console.warn('[web-shell-bridge] refused to open the screen this shell named', { + issue: diagnostic.issue + }) + return + } if (diagnostic.kind === 'post-failed') { console.warn('[web-shell-bridge] the page could not be posted to', diagnostic.error) return diff --git a/mobile/src/mobile-web-shell/bridge-host-contract.ts b/mobile/src/mobile-web-shell/bridge-host-contract.ts new file mode 100644 index 00000000000..fa08969e3b6 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge-host-contract.ts @@ -0,0 +1,64 @@ +import type { RpcClient } from '../transport/rpc-client' +import type { BridgeRefusal } from './bridge/bridge-caps' +import type { BridgeInitRoute } from './bridge/bridge-envelope' +import type { BridgeErrorCapture } from './bridge/bridge-error-capture' +import type { BridgeNotifyRefusal } from './bridge/bridge-notify-grants' + +/** What a caller owes one bridge host, and everything it will be told back. + * Separate from the host itself so the shape of the contract reads without the machinery. */ + +/** Nothing here is recoverable in place; each is worth a line in a log and none of them is retried. */ +export type BridgeHostDiagnostic = + | { kind: 'refused'; refusal: BridgeRefusal } + | { kind: 'post-failed'; error: unknown } + /** A page posting into a host that has already been disposed, which its own view is the only + * thing that can do. Dropping it silently is what hides a leaked view. */ + | { kind: 'frame-after-dispose' } + /** A listener that threw where the bridge only forwards. Nothing is owed to the page for a + * notify, so the throw is reported rather than answered. */ + | { kind: 'notify-failed'; error: unknown } + /** A frame that arrived between a page's `close` and the next document's `ready`. It belongs to + * the closed document, and serving it would answer into whatever loads in next. */ + | { kind: 'frame-after-close' } + /** A `notify` the host will not act on: a grant-gated name it never issued, or any name from a + * page that has not asked for a session yet. Nothing is owed back, so it is logged and dropped. */ + | { kind: 'notify-refused'; name: string; why: BridgeNotifyRefusal } + /** The shell asked this host to open a screen the protocol does not allow. The host serves no + * session at all in that state: an `init` the page refuses is worse than no `init`. */ + | { kind: 'route-refused'; issue: string } + +export type BridgeHostOptions = { + client: RpcClient + /** + * Rejects when there is nowhere to post. Resolving proves the message was handed over, never that + * the page received it, so nothing here treats a resolve as an acknowledgement. + */ + post: (json: string) => Promise + buildId: string + sessionId: string + /** + * Which screen the page should open. Required of a caller in this build and optional on the wire: + * an older shell sends no route at all, and the page has a state for that which nothing here can + * reach. + */ + route: BridgeInitRoute + /** + * The page could not render the generation it was handed. Required, because the page has no + * recovery of its own: the generation is on disk and was hash-checked before the view loaded it, + * so the same bytes throw again, and the only thing left is for the shell to stop showing them. + */ + onPageFault: (error: BridgeErrorCapture) => void + /** + * The page asked for a session, which is the only proof its bundle evaluated at all. Required for + * the same reason as the fault: the shell bounds the wait for it, and a host built without this + * would leave a document that never spoke looking exactly like one still starting up. + */ + onPageReady: () => void + /** + * The route this shell was built with is not one the protocol allows, so no honest `init` can be + * sent and the page will never mount. Loud on purpose: the page's own refusal is a `console.warn` + * inside a WebView nobody is reading, and the alternative is a blank screen that retries forever. + */ + onRouteRefused: (issue: string) => void + onDiagnostic?: (diagnostic: BridgeHostDiagnostic) => void +} diff --git a/mobile/src/mobile-web-shell/bridge-host-init.test.ts b/mobile/src/mobile-web-shell/bridge-host-init.test.ts new file mode 100644 index 00000000000..f504d68e8e7 --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge-host-init.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import { clientFrame, createFakeRpcClient } from './bridge-host-test-fakes' +import { harness, ROUTE } from './bridge-host-test-harness' +import { + BRIDGE_MAX_PENDING_REQUESTS, + BRIDGE_MAX_ROUTE_PATHNAME_CHARS, + BRIDGE_MAX_SUBSCRIPTIONS +} from './bridge/bridge-caps' +import { BRIDGE_FAULT_GRANT } from './bridge/bridge-envelope' + +describe('init and state', () => { + it('answers ready with the getters, the caps it enforces, and the one native grant', () => { + const client = createFakeRpcClient({ + getState: () => 'reconnecting', + getReconnectAttempt: () => 3, + getLastConnectedAt: () => 1_700_000_000_000, + getLastInboundAt: () => 1_700_000_000_500, + getGeneration: () => 7 + }) + const bridge = harness({ client }) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.last()).toEqual({ + v: 1, + type: 'init', + sessionId: 'session-a', + buildId: 'build-a', + connection: { + state: 'reconnecting', + reconnectAttempt: 3, + lastConnectedAt: 1_700_000_000_000, + lastInboundAt: 1_700_000_000_500, + generation: 7 + }, + grants: { + rpc: { + maxPendingRequests: BRIDGE_MAX_PENDING_REQUESTS, + maxSubscriptions: BRIDGE_MAX_SUBSCRIPTIONS + }, + native: [BRIDGE_FAULT_GRANT] + }, + route: ROUTE + }) + }) + + it('names the screen the page is standing in for, which its own `/` cannot tell it', () => { + const route = { pathname: '/h/host-a/session/wt-1', params: { name: 'a branch' } } + const bridge = harness({ route }) + bridge.host.receive(clientFrame({ type: 'ready' })) + const init = bridge.last() + expect(init.type === 'init' && init.route).toEqual(route) + }) + + it('refuses to open a session at all for a route the protocol does not allow', () => { + // The producer interpolates a host id into this pathname, so every one of these is reachable + // from a deep link. Without the check the page refuses the whole `init`, asks again on its + // backoff forever, and the shell un-hides a WebView that never paints. + for (const pathname of [ + '/h/a?b', + '/h/a#b', + '/h/a b', + '/h/..', + '/../../etc', + '/h/a\\b', + '//evil', + `/h/${'a'.repeat(BRIDGE_MAX_ROUTE_PATHNAME_CHARS)}` + ]) { + const bridge = harness({ route: { pathname } }) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.posted, pathname).toEqual([]) + expect(bridge.routeRefusals, pathname).toHaveLength(1) + expect( + bridge.diagnostics.map((diagnostic) => diagnostic.kind), + pathname + ).toEqual(['route-refused']) + } + }) + + it('opens a session for the routes a screen actually produces', () => { + for (const pathname of ['/h/host-a', '/h/host-a/tasks', '/h/a%20b', '/']) { + const bridge = harness({ route: { pathname } }) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.last().type, pathname).toBe('init') + expect(bridge.routeRefusals, pathname).toEqual([]) + } + }) + + it('reports a client without the optional getters as null rather than omitting the field', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'ready' })) + const init = bridge.last() + expect(init.type === 'init' && init.connection).toEqual({ + state: 'connected', + reconnectAttempt: 0, + lastConnectedAt: null, + lastInboundAt: null, + generation: null + }) + }) + + it('re-answers ready, which is how a page that missed a state frame recovers', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'ready' })) + bridge.host.receive(clientFrame({ type: 'ready' })) + expect(bridge.frames().filter((frame) => frame.type === 'init')).toHaveLength(2) + }) + + it('tells the shell the page spoke, on the first ask and on every re-ask', () => { + const bridge = harness() + expect(bridge.pageReadyCount()).toBe(0) + bridge.host.receive(clientFrame({ type: 'ready' })) + bridge.host.receive(clientFrame({ type: 'ready' })) + // The shell bounds the wait for the first of these; a page on its backoff must not have to + // land a particular one to end it. + expect(bridge.pageReadyCount()).toBe(2) + }) + + it('says nothing about a page that never asked, however much else it posts', () => { + const bridge = harness() + bridge.host.receive(clientFrame({ type: 'notify', name: 'foreground' })) + expect(bridge.pageReadyCount()).toBe(0) + }) + + it('pushes the event state, not the getter a listener can outrun', () => { + const bridge = harness() + bridge.client.pushState('disconnected') + const pushed = bridge.last() + expect(pushed.type === 'state' && pushed.connection.state).toBe('disconnected') + }) + + it('drops the state listener on dispose', () => { + const bridge = harness() + expect(bridge.client.stateListeners()).toBe(1) + bridge.host.dispose() + expect(bridge.client.stateListeners()).toBe(0) + }) +}) diff --git a/mobile/src/mobile-web-shell/bridge-host-test-harness.ts b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts new file mode 100644 index 00000000000..e1d74bb9a0c --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge-host-test-harness.ts @@ -0,0 +1,99 @@ +/** One bridge host wired to a fake client, read back through the page's own reader. + * Shared because the suites that exercise it are split by concern, not by fixture. */ +import { + bridgeId, + clientFrame, + createFakeRpcClient, + type FakeRpcClient +} from './bridge-host-test-fakes' +import { createBridgeHost, type BridgeHost, type BridgeHostDiagnostic } from './bridge-host' +import { + readBridgeHostMessage, + type BridgeHostMessage, + type BridgeInitRoute +} from './bridge/bridge-envelope' +import type { BridgeErrorCapture } from './bridge/bridge-error-capture' + +export const ID = bridgeId(1) +export const OTHER = bridgeId(2) + +export type Harness = { + host: BridgeHost + client: FakeRpcClient + posted: string[] + diagnostics: BridgeHostDiagnostic[] + pageFaults: BridgeErrorCapture[] + pageReadyCount: () => number + routeRefusals: string[] + frames: () => BridgeHostMessage[] + last: () => BridgeHostMessage +} + +export const ROUTE = { pathname: '/h/host-a' } + +export function harness( + options: { + client?: FakeRpcClient + post?: (json: string) => Promise + route?: BridgeInitRoute + onPageFault?: (error: BridgeErrorCapture) => void + } = {} +): Harness { + const client = options.client ?? createFakeRpcClient() + const posted: string[] = [] + const diagnostics: BridgeHostDiagnostic[] = [] + const pageFaults: BridgeErrorCapture[] = [] + let pageReadies = 0 + const routeRefusals: string[] = [] + const host = createBridgeHost({ + client, + post: (json) => { + posted.push(json) + return options.post?.(json) ?? Promise.resolve() + }, + buildId: 'build-a', + sessionId: 'session-a', + route: options.route ?? ROUTE, + onPageFault: (error) => { + pageFaults.push(error) + options.onPageFault?.(error) + }, + onPageReady: () => { + pageReadies += 1 + }, + onRouteRefused: (issue) => routeRefusals.push(issue), + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic) + }) + // Read back through the page's own reader: a frame the host sends that the page would refuse is + // a frame that never arrives, and this is the only place both halves meet in one test. + const frames = (): BridgeHostMessage[] => + posted.map((json) => { + const read = readBridgeHostMessage(json) + if (!read.ok) { + throw new Error(`the page would refuse this frame: ${read.refusal}`) + } + return read.message + }) + return { + host, + client, + posted, + diagnostics, + pageFaults, + pageReadyCount: () => pageReadies, + routeRefusals, + frames, + last: () => { + const all = frames() + const tail = all.at(-1) + if (tail === undefined) { + throw new Error('nothing was posted') + } + return tail + } + } +} + +export function subscribeFrame(id: string, method = 'terminal.subscribe'): string { + return clientFrame({ type: 'subscribe', id, method, params: { terminal: 't' } }) +} diff --git a/mobile/src/mobile-web-shell/bridge-host.test.ts b/mobile/src/mobile-web-shell/bridge-host.test.ts index f02d6cecdf1..cd182971057 100644 --- a/mobile/src/mobile-web-shell/bridge-host.test.ts +++ b/mobile/src/mobile-web-shell/bridge-host.test.ts @@ -1,189 +1,23 @@ import { describe, expect, it } from 'vitest' import type { RpcResponse } from '../transport/types' +import { harness, ID, OTHER, subscribeFrame, type Harness } from './bridge-host-test-harness' import { BRIDGE_MAX_UNACKED_BYTES, BRIDGE_MAX_UNACKED_FRAMES } from './bridge-host-subscriptions' import { bridgeId, clientFrame, createFakeRpcClient, flushBridge, - rpcSuccess, - type FakeRpcClient + rpcSuccess } from './bridge-host-test-fakes' -import { createBridgeHost, type BridgeHost, type BridgeHostDiagnostic } from './bridge-host' import { BRIDGE_MAX_MESSAGE_BYTES, BRIDGE_MAX_PENDING_REQUESTS, BRIDGE_MAX_REPLY_BYTES, BRIDGE_MAX_SUBSCRIPTIONS } from './bridge/bridge-caps' -import { - BRIDGE_FAULT_GRANT, - readBridgeHostMessage, - type BridgeHostMessage -} from './bridge/bridge-envelope' -import type { BridgeErrorCapture } from './bridge/bridge-error-capture' +import { BRIDGE_FAULT_GRANT, type BridgeHostMessage } from './bridge/bridge-envelope' import { BridgeReplyAssembler } from './bridge/bridge-reply-chunking' -const ID = bridgeId(1) -const OTHER = bridgeId(2) - -type Harness = { - host: BridgeHost - client: FakeRpcClient - posted: string[] - diagnostics: BridgeHostDiagnostic[] - pageFaults: BridgeErrorCapture[] - pageReadyCount: () => number - frames: () => BridgeHostMessage[] - last: () => BridgeHostMessage -} - -function harness( - options: { - client?: FakeRpcClient - post?: (json: string) => Promise - onPageFault?: (error: BridgeErrorCapture) => void - } = {} -): Harness { - const client = options.client ?? createFakeRpcClient() - const posted: string[] = [] - const diagnostics: BridgeHostDiagnostic[] = [] - const pageFaults: BridgeErrorCapture[] = [] - let pageReadies = 0 - const host = createBridgeHost({ - client, - post: (json) => { - posted.push(json) - return options.post?.(json) ?? Promise.resolve() - }, - buildId: 'build-a', - sessionId: 'session-a', - onPageFault: (error) => { - pageFaults.push(error) - options.onPageFault?.(error) - }, - onPageReady: () => { - pageReadies += 1 - }, - onDiagnostic: (diagnostic) => diagnostics.push(diagnostic) - }) - // Read back through the page's own reader: a frame the host sends that the page would refuse is - // a frame that never arrives, and this is the only place both halves meet in one test. - const frames = (): BridgeHostMessage[] => - posted.map((json) => { - const read = readBridgeHostMessage(json) - if (!read.ok) { - throw new Error(`the page would refuse this frame: ${read.refusal}`) - } - return read.message - }) - return { - host, - client, - posted, - diagnostics, - pageFaults, - pageReadyCount: () => pageReadies, - frames, - last: () => { - const all = frames() - const tail = all.at(-1) - if (tail === undefined) { - throw new Error('nothing was posted') - } - return tail - } - } -} - -function subscribeFrame(id: string, method = 'terminal.subscribe'): string { - return clientFrame({ type: 'subscribe', id, method, params: { terminal: 't' } }) -} - -describe('init and state', () => { - it('answers ready with the getters, the caps it enforces, and the one native grant', () => { - const client = createFakeRpcClient({ - getState: () => 'reconnecting', - getReconnectAttempt: () => 3, - getLastConnectedAt: () => 1_700_000_000_000, - getLastInboundAt: () => 1_700_000_000_500, - getGeneration: () => 7 - }) - const bridge = harness({ client }) - bridge.host.receive(clientFrame({ type: 'ready' })) - expect(bridge.last()).toEqual({ - v: 1, - type: 'init', - sessionId: 'session-a', - buildId: 'build-a', - connection: { - state: 'reconnecting', - reconnectAttempt: 3, - lastConnectedAt: 1_700_000_000_000, - lastInboundAt: 1_700_000_000_500, - generation: 7 - }, - grants: { - rpc: { - maxPendingRequests: BRIDGE_MAX_PENDING_REQUESTS, - maxSubscriptions: BRIDGE_MAX_SUBSCRIPTIONS - }, - native: [BRIDGE_FAULT_GRANT] - } - }) - }) - - it('reports a client without the optional getters as null rather than omitting the field', () => { - const bridge = harness() - bridge.host.receive(clientFrame({ type: 'ready' })) - const init = bridge.last() - expect(init.type === 'init' && init.connection).toEqual({ - state: 'connected', - reconnectAttempt: 0, - lastConnectedAt: null, - lastInboundAt: null, - generation: null - }) - }) - - it('re-answers ready, which is how a page that missed a state frame recovers', () => { - const bridge = harness() - bridge.host.receive(clientFrame({ type: 'ready' })) - bridge.host.receive(clientFrame({ type: 'ready' })) - expect(bridge.frames().filter((frame) => frame.type === 'init')).toHaveLength(2) - }) - - it('tells the shell the page spoke, on the first ask and on every re-ask', () => { - const bridge = harness() - expect(bridge.pageReadyCount()).toBe(0) - bridge.host.receive(clientFrame({ type: 'ready' })) - bridge.host.receive(clientFrame({ type: 'ready' })) - // The shell bounds the wait for the first of these; a page on its backoff must not have to - // land a particular one to end it. - expect(bridge.pageReadyCount()).toBe(2) - }) - - it('says nothing about a page that never asked, however much else it posts', () => { - const bridge = harness() - bridge.host.receive(clientFrame({ type: 'notify', name: 'foreground' })) - expect(bridge.pageReadyCount()).toBe(0) - }) - - it('pushes the event state, not the getter a listener can outrun', () => { - const bridge = harness() - bridge.client.pushState('disconnected') - const pushed = bridge.last() - expect(pushed.type === 'state' && pushed.connection.state).toBe('disconnected') - }) - - it('drops the state listener on dispose', () => { - const bridge = harness() - expect(bridge.client.stateListeners()).toBe(1) - bridge.host.dispose() - expect(bridge.client.stateListeners()).toBe(0) - }) -}) - describe('requests', () => { it('replays the arity the page used', () => { const bridge = harness() diff --git a/mobile/src/mobile-web-shell/bridge-host.ts b/mobile/src/mobile-web-shell/bridge-host.ts index 99246f86a29..a63f923ed5d 100644 --- a/mobile/src/mobile-web-shell/bridge-host.ts +++ b/mobile/src/mobile-web-shell/bridge-host.ts @@ -1,4 +1,3 @@ -import type { RpcClient } from '../transport/rpc-client' import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' import type { ConnectionState, RpcResponse } from '../transport/types' import { @@ -7,23 +6,24 @@ import { BridgeReplyUndeliverableError } from './bridge-host-errors' import { BridgeHostSubscriptions } from './bridge-host-subscriptions' -import { - BRIDGE_MAX_PENDING_REQUESTS, - BRIDGE_MAX_SUBSCRIPTIONS, - type BridgeRefusal -} from './bridge/bridge-caps' +import { BRIDGE_MAX_PENDING_REQUESTS, BRIDGE_MAX_SUBSCRIPTIONS } from './bridge/bridge-caps' import { BRIDGE_FAULT_GRANT, BRIDGE_PROTOCOL_VERSION, + BridgeInitRouteSchema, readBridgeClientMessage, type BridgeClientMessage, type BridgeConnectionSnapshot, type BridgeHostMessage } from './bridge/bridge-envelope' -import { captureBridgeError, type BridgeErrorCapture } from './bridge/bridge-error-capture' +import { captureBridgeError } from './bridge/bridge-error-capture' import { BRIDGE_NATIVE_GRANTS, createBridgeInitFrame } from './bridge/bridge-init-frame' -import { bridgeNotifyRefusal, type BridgeNotifyRefusal } from './bridge/bridge-notify-grants' +import { bridgeNotifyRefusal } from './bridge/bridge-notify-grants' import { splitBridgeReply } from './bridge/bridge-reply-chunking' +import type { BridgeHostOptions } from './bridge-host-contract' + +// Re-exported so a caller reaches the host and what it reports through one module. +export type { BridgeHostDiagnostic, BridgeHostOptions } from './bridge-host-contract' type RequestMessage = Extract type SubscribeMessage = Extract @@ -33,47 +33,6 @@ type NotifyMessage = Extract * being posted under an id the page has moved on from. */ type PendingRequest = { live: boolean } -/** Nothing here is recoverable in place; each is worth a line in a log and none of them is retried. */ -export type BridgeHostDiagnostic = - | { kind: 'refused'; refusal: BridgeRefusal } - | { kind: 'post-failed'; error: unknown } - /** A page posting into a host that has already been disposed, which its own view is the only - * thing that can do. Dropping it silently is what hides a leaked view. */ - | { kind: 'frame-after-dispose' } - /** A listener that threw where the bridge only forwards. Nothing is owed to the page for a - * notify, so the throw is reported rather than answered. */ - | { kind: 'notify-failed'; error: unknown } - /** A frame that arrived between a page's `close` and the next document's `ready`. It belongs to - * the closed document, and serving it would answer into whatever loads in next. */ - | { kind: 'frame-after-close' } - /** A `notify` the host will not act on: a grant-gated name it never issued, or any name from a - * page that has not asked for a session yet. Nothing is owed back, so it is logged and dropped. */ - | { kind: 'notify-refused'; name: string; why: BridgeNotifyRefusal } - -export type BridgeHostOptions = { - client: RpcClient - /** - * Rejects when there is nowhere to post. Resolving proves the message was handed over, never that - * the page received it, so nothing here treats a resolve as an acknowledgement. - */ - post: (json: string) => Promise - buildId: string - sessionId: string - /** - * The page could not render the generation it was handed. Required, because the page has no - * recovery of its own: the generation is on disk and was hash-checked before the view loaded it, - * so the same bytes throw again, and the only thing left is for the shell to stop showing them. - */ - onPageFault: (error: BridgeErrorCapture) => void - /** - * The page asked for a session, which is the only proof its bundle evaluated at all. Required for - * the same reason as the fault: the shell bounds the wait for it, and a host built without this - * would leave a document that never spoke looking exactly like one still starting up. - */ - onPageReady: () => void - onDiagnostic?: (diagnostic: BridgeHostDiagnostic) => void -} - export type BridgeHost = { receive: (json: string) => void dispose: () => void @@ -89,6 +48,12 @@ export type BridgeHost = { */ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { const { client, buildId, sessionId } = options + // Parsed here, once, against the same schema the page reads it with. The producer interpolates a + // host id into a pathname, so a host id carrying `?`, `#`, whitespace or a dot segment reaches + // the wire as a route no page will accept; without this the page refuses the whole `init`, asks + // again on its backoff forever, and the shell un-hides a WebView that will never paint. + const parsedRoute = BridgeInitRouteSchema.safeParse(options.route) + const route = parsedRoute.success ? parsedRoute.data : null const pending = new Map() let closed = false // Requests the client is still running. `pending` is the page's view and empties on a cancel or a @@ -162,8 +127,11 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { // Answered every time it is asked: a page that saw a `state` older than the one it holds recovers // by asking again rather than by living with a cache it knows is wrong. function sendInit(): void { + if (route === null) { + return + } initSent = true - send(createBridgeInitFrame({ sessionId, buildId, connection: snapshot() })) + send(createBridgeInitFrame({ sessionId, buildId, connection: snapshot(), route })) } function settle(id: string, record: PendingRequest): boolean { @@ -379,6 +347,16 @@ export function createBridgeHost(options: BridgeHostOptions): BridgeHost { send({ v: BRIDGE_PROTOCOL_VERSION, type: 'state', connection: snapshot(state) }) }) + if (route === null) { + // At construction rather than on the first `ready`: the verdict does not depend on the page + // behaving, and a shell that waited for a frame would hold a blank view until one arrived. + const issue = parsedRoute.success + ? 'unknown' + : (parsedRoute.error.issues[0]?.message ?? 'unknown') + options.onDiagnostic?.({ kind: 'route-refused', issue }) + options.onRouteRefused(issue) + } + return { receive(json: string): void { if (closed) { diff --git a/mobile/src/mobile-web-shell/bridge/bridge-caps.ts b/mobile/src/mobile-web-shell/bridge/bridge-caps.ts index d8ddedc4539..873d0d7c44b 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-caps.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-caps.ts @@ -28,6 +28,39 @@ export const BRIDGE_MAX_NODES = 20_000 /** Longest method name accepted. The desktop's mobile-scope allowlist owns which names exist. */ export const BRIDGE_MAX_METHOD_CHARS = 64 +/** + * The initial route bounds. + * + * The page writes this path into its own history before it renders, so it is held to what a path + * may be rather than to what a screen may want: rooted, carrying neither a query nor a fragment + * because the params are a field of their own, and made of segments that name something. + * + * Shape alone is not enough, because `replaceState` normalises what it is given and the page then + * renders whatever came out. A protocol-relative `//host` throws a cross-origin `SecurityError` and + * takes the mount down; `/../../etc` resolves to `/etc` and `/h/a\b` to `/h/a/b`, both of which + * escape the `/h/` prefix the page's route tree starts at and land on a screen nobody asked for. + * So: no empty segment, no dot segment, no backslash anywhere — none of which a route can produce. + * A dot segment counts however it is spelled: a URL parser percent-decodes the path before it + * resolves it, so `/h/%2e%2e/x` climbs out of the prefix exactly as `/h/../x` does. An escape + * inside a segment that names something (`/h/a%20b`, `/h/%2ex`) is text and stays allowed. + */ +export const BRIDGE_MAX_ROUTE_PATHNAME_CHARS = 1024 +export const BRIDGE_MAX_ROUTE_PARAMS = 32 +export const BRIDGE_MAX_ROUTE_PARAM_CHARS = 1024 + +/** + * One segment of a route path, and the only place the rule is written. + * + * Exported as source rather than as a regex because it is embedded in more than one pattern: the + * `init` pathname below and the hrefs a page hands back to the shell are the same vocabulary, and + * two spellings of it would be two rules that drift. + */ +export const BRIDGE_ROUTE_SEGMENT_SOURCE = String.raw`(?!(?:\.|%2[eE]){1,2}(?:/|$))[^/\\?#\s]+` + +export const BRIDGE_ROUTE_PATHNAME_PATTERN = new RegExp( + `^/(?:${BRIDGE_ROUTE_SEGMENT_SOURCE}(?:/${BRIDGE_ROUTE_SEGMENT_SOURCE})*/?)?$` +) + /** * In-flight bounds. The RN host is authoritative for both; the page holds the same numbers only to * refuse at the call site instead of after a round trip. diff --git a/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts b/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts new file mode 100644 index 00000000000..6eead9f8e5d --- /dev/null +++ b/mobile/src/mobile-web-shell/bridge/bridge-client-session.ts @@ -0,0 +1,27 @@ +import type { BridgeGrants, BridgeHostMessage, BridgeInitRoute } from './bridge-envelope' + +/** What `init` said this page is attached to. `grants` is what a call site checks before it posts. */ +export type BridgeShellSession = { + sessionId: string + buildId: string + grants: BridgeGrants + /** Null for a shell too old to name one. The page has no other way to know which screen to open. */ + route: BridgeInitRoute | null +} + +/** + * The session an `init` describes. + * + * `route` is absent on the wire rather than null, because a field written as `undefined` and a + * field nobody sent are the same frame; the page reads one shape from here and never both. + */ +export function readShellSession( + message: Extract +): BridgeShellSession { + return { + sessionId: message.sessionId, + buildId: message.buildId, + grants: message.grants, + route: message.route ?? null + } +} diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts index 171916625f6..ac78198f09c 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.test.ts @@ -13,6 +13,9 @@ import { BRIDGE_MAX_MESSAGE_BYTES, BRIDGE_MAX_METHOD_CHARS, BRIDGE_MAX_REPLY_PARTS, + BRIDGE_MAX_ROUTE_PARAM_CHARS, + BRIDGE_MAX_ROUTE_PARAMS, + BRIDGE_MAX_ROUTE_PATHNAME_CHARS, BRIDGE_MAX_VIEWPORT_COLS, BRIDGE_MAX_VIEWPORT_ROWS } from './bridge-caps' @@ -212,11 +215,60 @@ describe('client messages', () => { }) describe('host messages', () => { + /** An otherwise valid `init`, so a refusal below is the route's and not the frame's. */ + function initRoute(route: unknown): Record { + return client({ + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + route + }) + } + const accepted = [ [ 'init', { type: 'init', sessionId: 's1', buildId: 'b1', connection: CONNECTION, grants: GRANTS } ], + [ + 'an init naming the screen the page should open', + { + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + route: { pathname: '/h/host-a/session/wt-1', params: { name: 'a branch' } } + } + ], + [ + 'an init whose segments merely contain dots, which are names and not navigation', + { + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + // Without this the refusals above would also pass a rule that banned the character. + route: { pathname: '/h/a..b/...' } + } + ], + [ + 'an init whose segments merely carry percent escapes, which are text and not navigation', + { + type: 'init', + sessionId: 's1', + buildId: 'b1', + connection: CONNECTION, + grants: GRANTS, + // An encoded space, a segment that starts with an encoded dot, and an encoded slash, which + // the router reads as one segment's text. Without these the refusals above would pass a + // rule that banned the escape rather than the dot segment it spells. + route: { pathname: '/h/a%20b/%2ex/a%2fb' } + } + ], ['state', { type: 'state', connection: CONNECTION }], ['a whole reply', { type: 'reply', id: ID, payload: SUCCESS_PAYLOAD }], [ @@ -324,7 +376,65 @@ describe('host messages', () => { [ 'an end for a reason that is not one of the three', client({ type: 'end', id: ID, reason: 'done' }) - ] + ], + // Every one of these reaches `history.replaceState`. A protocol-relative path makes it throw a + // cross-origin SecurityError and takes the mount down; the other three are a URL the page + // would have to parse to separate again, which is what `params` exists to avoid. + ['an init route that is not rooted', initRoute({ pathname: 'h/host-a' })], + ['an init route that is protocol-relative', initRoute({ pathname: '//evil.example/h' })], + ['an init route that is backslash-relative', initRoute({ pathname: '/\\evil.example/h' })], + ['an init route carrying its own query', initRoute({ pathname: '/h/a?name=b' })], + ['an init route carrying a fragment', initRoute({ pathname: '/h/a#top' })], + // `replaceState` normalises each of these and the page then renders whatever came out: + // `/../../etc` resolves to `/etc`, `/h/a/../x` to `/h/x`, and `/h/a\\b` to `/h/a/b`. All three + // leave the `/h/` prefix the page's tree starts at, which is the whole point of refusing + // shape rather than trusting the router to be handed one. + ['an init route that climbs out of its prefix', initRoute({ pathname: '/../../etc' })], + [ + 'an init route with an interior dot segment', + initRoute({ pathname: '/h/a/../render-check-host' }) + ], + ['an init route ending in a dot segment', initRoute({ pathname: '/h/a/..' })], + ['an init route with a single dot segment', initRoute({ pathname: '/h/./a' })], + ['an init route with an interior backslash', initRoute({ pathname: '/h/a\\b' })], + // The same climb, spelled the way a URL parser still reads as a dot segment: it percent-decodes + // the path before it resolves it, so `%2e%2e` escapes the prefix exactly as `..` does. + [ + 'an init route that climbs out of its prefix percent-encoded', + initRoute({ pathname: '/h/%2e%2e/render-check-host' }) + ], + [ + 'an init route that climbs out of its prefix in capitals', + initRoute({ pathname: '/h/%2E%2E/render-check-host' }) + ], + ['an init route with a half-encoded dot segment', initRoute({ pathname: '/h/.%2e/a' })], + ['an init route with a single encoded dot segment', initRoute({ pathname: '/h/%2e/a' })], + ['an init route with an empty interior segment', initRoute({ pathname: '/h//a' })], + ['an init route with an empty pathname', initRoute({ pathname: '' })], + [ + 'an init route over the pathname cap', + initRoute({ pathname: `/${'h'.repeat(BRIDGE_MAX_ROUTE_PATHNAME_CHARS)}` }) + ], + [ + 'an init route with more params than the cap', + initRoute({ + pathname: '/h/a', + params: Object.fromEntries( + Array.from({ length: BRIDGE_MAX_ROUTE_PARAMS + 1 }, (_value, index) => [ + `k${String(index)}`, + 'v' + ]) + ) + }) + ], + [ + 'an init route with a param value over the cap', + initRoute({ + pathname: '/h/a', + params: { name: 'v'.repeat(BRIDGE_MAX_ROUTE_PARAM_CHARS + 1) } + }) + ], + ['an init route whose param is not a string', initRoute({ pathname: '/h/a', params: { n: 1 } })] ] as const for (const [name, message] of refused) { diff --git a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts index de9b1a0402a..1739322b4e7 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-envelope.ts @@ -5,8 +5,12 @@ import { BridgeErrorCaptureSchema } from './bridge-error-capture' import { BRIDGE_MAX_METHOD_CHARS, BRIDGE_MAX_REPLY_PARTS, + BRIDGE_MAX_ROUTE_PARAM_CHARS, + BRIDGE_MAX_ROUTE_PARAMS, + BRIDGE_MAX_ROUTE_PATHNAME_CHARS, BRIDGE_MAX_VIEWPORT_COLS, BRIDGE_MAX_VIEWPORT_ROWS, + BRIDGE_ROUTE_PATHNAME_PATTERN, parseBridgeMessage, type BridgeDirection, type BridgeRead @@ -84,6 +88,34 @@ export const BridgeGrantsSchema = z.object({ export type BridgeGrants = z.infer +/** + * Which screen the shell opened this page for. + * + * Additive, and optional for that reason: a shell built before C1.2 sends no `route`, and the page + * says so rather than painting expo-router's Unmatched screen. It has to cross, because the + * document is served at `/` and refuses every other path, so the page's own location matches no + * route in the tree it carries and there is nothing else to derive the screen from. + * + * `params` is the search half, kept out of `pathname` so neither side has to parse a URL: the page + * builds one, once, and writes it into its history before the first render. + */ +export const BridgeInitRouteSchema = z.object({ + pathname: z + .string() + .min(1) + .max(BRIDGE_MAX_ROUTE_PATHNAME_CHARS) + .regex(BRIDGE_ROUTE_PATHNAME_PATTERN), + params: z + .record( + z.string().min(1).max(BRIDGE_MAX_ROUTE_PARAM_CHARS), + z.string().max(BRIDGE_MAX_ROUTE_PARAM_CHARS) + ) + .refine((params) => Object.keys(params).length <= BRIDGE_MAX_ROUTE_PARAMS) + .optional() +}) + +export type BridgeInitRoute = z.infer + /** * The one grant negotiated for the protocol itself rather than for a screen: the shell saying it * will act on a `fault` report. @@ -261,7 +293,8 @@ const BridgeHostMessageSchema = z.union([ sessionId: z.string().min(1), buildId: z.string().min(1), connection: BridgeConnectionSnapshotSchema, - grants: BridgeGrantsSchema + grants: BridgeGrantsSchema, + route: BridgeInitRouteSchema.optional() }) ]) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts index 55a9b96dfbb..4aeca6dd11e 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-init-frame.ts @@ -3,7 +3,8 @@ import { BRIDGE_FAULT_GRANT, BRIDGE_PROTOCOL_VERSION, type BridgeConnectionSnapshot, - type BridgeHostMessage + type BridgeHostMessage, + type BridgeInitRoute } from './bridge-envelope' /** @@ -20,6 +21,8 @@ export function createBridgeInitFrame(args: { sessionId: string buildId: string connection: BridgeConnectionSnapshot + /** The screen this page stands in for, which the document's own `/` cannot tell it. */ + route: BridgeInitRoute }): Extract { return { v: BRIDGE_PROTOCOL_VERSION, @@ -35,6 +38,7 @@ export function createBridgeInitFrame(args: { // Copied, not shared: the list the host enforces must not be reachable through a frame it // hands out. native: [...BRIDGE_NATIVE_GRANTS] - } + }, + route: args.route } } diff --git a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts index a5e27fd443b..242c9ece839 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.test.ts @@ -32,7 +32,10 @@ describe('the bridge port pair', () => { expect(pair.client.getShellSession()).toEqual({ sessionId: 'session-a', buildId: 'build-a', - grants: expect.anything() + grants: expect.anything(), + // The screen the shell says this page stands in for; the pair names one so the session it + // hands back is the shape a page on a route actually holds. + route: expect.objectContaining({ pathname: expect.any(String) }) }) }) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts index adb26bc795b..49a88595bc4 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-port-pair-test-harness.ts @@ -5,7 +5,8 @@ import { readBridgeClientMessage, readBridgeHostMessage, type BridgeClientMessage, - type BridgeHostMessage + type BridgeHostMessage, + type BridgeInitRoute } from './bridge-envelope' import type { BridgeErrorCapture } from './bridge-error-capture' import { @@ -40,6 +41,8 @@ export type BridgePortPair = { pageFaults: BridgeErrorCapture[] /** How many times the page asked for a session; it re-asks on a backoff until one lands. */ readonly pageReadyCount: () => number + /** Why the host refused to open a session at all, if it did. */ + readonly routeRefusals: string[] /** Runs both lanes until a full round moves nothing. */ flush: () => Promise /** @@ -59,6 +62,7 @@ export type BridgePortPairOptions = { rpc: TRpc sessionId?: string buildId?: string + route?: BridgeInitRoute /** * Rewrites each frame on its way to the page, for asking the page a counterfactual it cannot be * asked any other way: would this run have gone differently had the shell sent one more field? @@ -137,6 +141,7 @@ export function createBridgePortPair( const hostDiagnostics: BridgeHostDiagnostic[] = [] const pageFaults: BridgeErrorCapture[] = [] let pageReadies = 0 + const routeRefusals: string[] = [] let receiveOnPage: ((json: string) => void) | null = null const rewrite = options.rewriteToPage ?? ((json: string) => json) @@ -151,10 +156,12 @@ export function createBridgePortPair( }, buildId: options.buildId ?? 'build-a', sessionId: options.sessionId ?? 'session-a', + route: options.route ?? { pathname: '/h/host-a' }, onPageFault: (error) => pageFaults.push(error), onPageReady: () => { pageReadies += 1 }, + onRouteRefused: (issue) => routeRefusals.push(issue), onDiagnostic: (diagnostic) => hostDiagnostics.push(diagnostic) }) const toShell = createLane((json) => { @@ -183,6 +190,7 @@ export function createBridgePortPair( hostDiagnostics, pageFaults, pageReadyCount: () => pageReadies, + routeRefusals, async flush(): Promise { for (let round = 0; round < 64; round += 1) { const moved = toShell.sent.length + toPage.sent.length diff --git a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts index 7ffb86f8eb1..9b1e5daa3dd 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client-frames.test.ts @@ -122,10 +122,19 @@ describe('bridge client handshake', () => { expect(page.client.getShellSession()).toEqual({ sessionId: 'session-a', buildId: 'build-a', - grants: INIT.grants + grants: INIT.grants, + // A shell too old to name a screen, which is a state the page has an answer for. + route: null }) }) + it('carries the screen the shell opened this page for', () => { + const page = createPageClient() + const route = { pathname: '/h/host-a/session/wt-1', params: { name: 'a branch' } } + page.deliver({ ...INIT, route }) + expect(page.client.getShellSession()?.route).toEqual(route) + }) + it('answers a generation the shell does not keep with a constant epoch', () => { const page = createPageClient() page.deliver({ ...INIT, connection: { ...CONNECTION, generation: null } }) diff --git a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts index 132ffe3fa78..24cecb0f382 100644 --- a/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts +++ b/mobile/src/mobile-web-shell/bridge/bridge-rpc-client.ts @@ -4,6 +4,7 @@ import type { ConnectionState, RpcResponse } from '../../transport/types' import { BRIDGE_MAX_PENDING_REQUESTS, BRIDGE_MAX_SUBSCRIPTIONS } from './bridge-caps' import { BridgeConnectionCache } from './bridge-client-connection-cache' import type { BridgeRpcClientDiagnostic } from './bridge-client-diagnostics' +import { readShellSession, type BridgeShellSession } from './bridge-client-session' import { createBridgeInitHandshake } from './bridge-client-init-handshake' import { BridgeClientCapExceededError, @@ -20,10 +21,11 @@ import { BRIDGE_PROTOCOL_VERSION, type BridgeClientMessage, type BridgeConnectionSnapshot, - type BridgeGrants, type BridgeHostMessage } from './bridge-envelope' +export type { BridgeShellSession } from './bridge-client-session' + export { BridgeClientCapExceededError, BridgeClientClosedError, @@ -38,13 +40,6 @@ const BRIDGE_ID_CHARS = 22 export type { BridgeRpcClientDiagnostic } from './bridge-client-diagnostics' -/** What `init` said this page is attached to. `grants` is what a call site checks before it posts. */ -export type BridgeShellSession = { - sessionId: string - buildId: string - grants: BridgeGrants -} - export type BridgeRpcClientOptions = { /** Posts one frame to the shell. May throw; nothing about returning proves delivery. */ send: (json: string) => void @@ -155,7 +150,7 @@ export function createBridgeRpcClient(options: BridgeRpcClientOptions): BridgeRp requests.closeAll(replaced) subscriptions.failAll(replaced.message) } - session = { sessionId: message.sessionId, buildId: message.buildId, grants: message.grants } + session = readShellSession(message) cache.prime(message.connection) for (const listener of readyListeners) { listener() diff --git a/mobile/src/mobile-web-shell/bridge/page-bootstrap.test.ts b/mobile/src/mobile-web-shell/bridge/page-bootstrap.test.ts index 105e93b0a81..657e77519ab 100644 --- a/mobile/src/mobile-web-shell/bridge/page-bootstrap.test.ts +++ b/mobile/src/mobile-web-shell/bridge/page-bootstrap.test.ts @@ -7,6 +7,7 @@ import { PAGE_BUILD_ID_KEY, PAGE_MOUNT_STATE_KEY, PAGE_SESSION_ID_KEY, + shellRouteHref, stampPageMountState, type PageMountTarget } from './page-bootstrap' @@ -24,7 +25,8 @@ const INIT = { lastInboundAt: 1800, generation: 3 }, - grants: { rpc: { maxPendingRequests: 64, maxSubscriptions: 32 }, native: [] } + grants: { rpc: { maxPendingRequests: 64, maxSubscriptions: 32 }, native: [] }, + route: { pathname: '/h/host-a' } } function createTarget(): PageMountTarget { @@ -51,20 +53,36 @@ function installChannel(): { posted: string[]; deliver: (frame: unknown) => void type Mounted = { client: BridgeRpcClient; session: BridgeShellSession } -function bootstrap(target: PageMountTarget): { +/** Everything the page did to its document, in the order it did it. */ +type Page = { mounts: Mounted[] + urls: string[] + refusals: number + /** One list, because what matters is which came first: routing after a render is a render at `/`. */ + order: string[] client: BridgeRpcClient | null -} { - const mounts: Mounted[] = [] - const client = createShellPageClient() +} + +function bootstrap(target: PageMountTarget): Page { + const page: Page = { mounts: [], urls: [], refusals: 0, order: [], client: null } + page.client = createShellPageClient() bootstrapShellPage({ target, - client, - mount: (mountedClient, session) => { - mounts.push({ client: mountedClient, session }) + client: page.client, + replaceUrl: (href) => { + page.urls.push(href) + page.order.push('replaceUrl') + }, + mount: (client, session) => { + page.mounts.push({ client, session }) + page.order.push('mount') + }, + refuseUnroutedShell: () => { + page.refusals += 1 + page.order.push('refuse') } }) - return { mounts, client } + return page } beforeEach(() => { @@ -77,47 +95,62 @@ afterEach(() => { }) describe('the page bootstrap inside the shell', () => { - it('asks for a session and mounts nothing until the shell answers', () => { + it('asks for a session and does nothing to the document until the shell answers', () => { const channel = installChannel() const target = createTarget() - const { mounts } = bootstrap(target) + const page = bootstrap(target) expect(channel.posted.map((json) => JSON.parse(json).type)).toEqual(['ready']) - expect(mounts).toHaveLength(0) + expect(page.order).toEqual([]) expect(target.dataset[PAGE_MOUNT_STATE_KEY]).toBeUndefined() // The handshake keeps asking rather than waiting out an `init` that has been and gone, and - // still nothing is mounted while it does. + // still nothing is routed or mounted while it does. vi.advanceTimersByTime(5_000) expect(channel.posted.length).toBeGreaterThan(1) - expect(mounts).toHaveLength(0) + expect(page.order).toEqual([]) }) - it('mounts the client it was given once init lands, and stamps the session on the document', () => { + it('routes before it mounts, so the router never reads the one path no screen claims', () => { const channel = installChannel() const target = createTarget() - const { mounts, client } = bootstrap(target) + const page = bootstrap(target) channel.deliver(INIT) - expect(mounts).toHaveLength(1) - expect(mounts[0]?.client).toBe(client) - expect(mounts[0]?.session.sessionId).toBe('session-a') + expect(page.order).toEqual(['replaceUrl', 'mount']) + expect(page.urls).toEqual(['/h/host-a']) + expect(page.mounts[0]?.client).toBe(page.client) + expect(page.mounts[0]?.session.route).toEqual({ pathname: '/h/host-a' }) expect(target.dataset[PAGE_MOUNT_STATE_KEY]).toBe('shell-ready') expect(target.dataset[PAGE_SESSION_ID_KEY]).toBe('session-a') expect(target.dataset[PAGE_BUILD_ID_KEY]).toBe('build-a') }) - it('stamps the session before it mounts, so a tree that throws still names its build', () => { + it('carries the params the screen was opened with into the url it writes', () => { + const channel = installChannel() + const page = bootstrap(createTarget()) + + channel.deliver({ + ...INIT, + route: { pathname: '/h/host-a/session/wt-1', params: { name: 'fix the bug' } } + }) + + expect(page.urls).toEqual(['/h/host-a/session/wt-1?name=fix+the+bug']) + }) + + it('stamps the session before it routes, so a tree that throws still names its build', () => { const channel = installChannel() const target = createTarget() const client = createShellPageClient() bootstrapShellPage({ target, client, + replaceUrl: () => {}, mount: () => { expect(target.dataset[PAGE_BUILD_ID_KEY]).toBe('build-a') throw new Error('the route tree threw') - } + }, + refuseUnroutedShell: () => {} }) expect(() => { @@ -129,12 +162,12 @@ describe('the page bootstrap inside the shell', () => { it('mounts one tree for one document, whatever the shell sends next', () => { const channel = installChannel() const target = createTarget() - const { mounts } = bootstrap(target) + const page = bootstrap(target) channel.deliver(INIT) - channel.deliver({ ...INIT, sessionId: 'session-b', buildId: 'build-b' }) + channel.deliver({ ...INIT, sessionId: 'session-b', route: { pathname: '/h/host-b' } }) - expect(mounts).toHaveLength(1) + expect(page.order).toEqual(['replaceUrl', 'mount']) expect(target.dataset[PAGE_SESSION_ID_KEY]).toBe('session-a') }) @@ -143,36 +176,76 @@ describe('the page bootstrap inside the shell', () => { const client = createShellPageClient() channel.deliver(INIT) const target = createTarget() - const mounts: Mounted[] = [] + const order: string[] = [] bootstrapShellPage({ target, client, - mount: (mountedClient, session) => { - mounts.push({ client: mountedClient, session }) - } + replaceUrl: () => order.push('replaceUrl'), + mount: () => order.push('mount'), + refuseUnroutedShell: () => order.push('refuse') }) - expect(mounts).toHaveLength(1) + expect(order).toEqual(['replaceUrl', 'mount']) expect(target.dataset[PAGE_MOUNT_STATE_KEY]).toBe('shell-ready') }) }) +describe('the page bootstrap under a shell that named no screen', () => { + it('refuses instead of mounting the tree at a path no route claims', () => { + const channel = installChannel() + const target = createTarget() + const page = bootstrap(target) + + const { route: _route, ...withoutRoute } = INIT + channel.deliver(withoutRoute) + + expect(page.order).toEqual(['refuse']) + expect(target.dataset[PAGE_MOUNT_STATE_KEY]).toBe('shell-too-old') + // Still stamped: the build it could not open is the fact worth reading off the document. + expect(target.dataset[PAGE_BUILD_ID_KEY]).toBe('build-a') + }) + + it('does not go on waiting for a second init that says more', () => { + const channel = installChannel() + const page = bootstrap(createTarget()) + + const { route: _route, ...withoutRoute } = INIT + channel.deliver(withoutRoute) + channel.deliver(INIT) + + expect(page.order).toEqual(['refuse']) + }) +}) + describe('the page bootstrap outside the shell', () => { it('builds no client when nothing installed a channel', () => { expect(createShellPageClient()).toBeNull() }) - it('says so and mounts nothing, because no init is ever coming', () => { + it('says so and does nothing to the document, because no init is ever coming', () => { const target = createTarget() - const { mounts } = bootstrap(target) + const page = bootstrap(target) - expect(mounts).toHaveLength(0) + expect(page.order).toEqual([]) expect(target.dataset[PAGE_MOUNT_STATE_KEY]).toBe('unbridged') expect(target.dataset[PAGE_SESSION_ID_KEY]).toBeUndefined() }) }) +describe('the url the page writes for a route', () => { + it('is the pathname alone when the screen was opened with no params', () => { + expect(shellRouteHref({ pathname: '/h/host-a' })).toBe('/h/host-a') + expect(shellRouteHref({ pathname: '/h/host-a', params: {} })).toBe('/h/host-a') + }) + + it('escapes what a param holds rather than pasting it into a path', () => { + expect(shellRouteHref({ pathname: '/h/a', params: { name: 'a&b=c?d#e' } })).toBe( + '/h/a?name=a%26b%3Dc%3Fd%23e' + ) + }) +}) + describe('the mount state attribute', () => { it('records the last state reached, so the entry can say its script ran', () => { const target = createTarget() diff --git a/mobile/src/mobile-web-shell/bridge/page-bootstrap.ts b/mobile/src/mobile-web-shell/bridge/page-bootstrap.ts index 796a54028b2..f4e85f08c1c 100644 --- a/mobile/src/mobile-web-shell/bridge/page-bootstrap.ts +++ b/mobile/src/mobile-web-shell/bridge/page-bootstrap.ts @@ -4,6 +4,7 @@ import { type BridgeRpcClientDiagnostic, type BridgeShellSession } from './bridge-rpc-client' +import type { BridgeInitRoute } from './bridge-envelope' import { createOrcaBridgePageTransport, readOrcaBridgePageChannel @@ -13,10 +14,11 @@ import { * How far the page's bootstrap got, in one attribute. * * The state is the only thing that tells a document which never ran its script from one that ran - * it and threw, and from one still waiting on a shell that has not answered. `unbridged` is - * terminal: nothing is coming, because nothing installed a channel on this document. + * it and threw, and from one still waiting on a shell that has not answered. Two of the four are + * terminal: `unbridged`, because nothing installed a channel on this document, and `shell-too-old`, + * because the shell that did install one never said which screen to open. */ -export type PageMountState = 'started' | 'unbridged' | 'shell-ready' | 'mounted' +export type PageMountState = 'started' | 'unbridged' | 'shell-too-old' | 'shell-ready' | 'mounted' /** `dataset` keys, so a screenshot, the render check and a device console read the same three facts. */ export const PAGE_MOUNT_STATE_KEY = 'orcaWebEntry' @@ -59,41 +61,67 @@ export function createShellPageClient(): BridgeRpcClient | null { }) } +/** The route as the one URL the page writes into its history. Params are the search half. */ +export function shellRouteHref(route: BridgeInitRoute): string { + const search = new URLSearchParams(route.params ?? {}).toString() + return search === '' ? route.pathname : `${route.pathname}?${search}` +} + +export type ShellPageBootstrapOptions = { + target: PageMountTarget + client: BridgeRpcClient | null + /** `history.replaceState(null, '', href)`. Separated so a test reads what the page claimed to be. */ + replaceUrl: (href: string) => void + mount: (client: BridgeRpcClient, session: BridgeShellSession) => void + /** The terminal panel for a shell that named no screen. Nothing mounts after it. */ + refuseUnroutedShell: () => void +} + /** - * Mounts the route tree once `init` has landed, and never before it. + * Routes and mounts the page once `init` has landed, and does neither before it. * * Nothing mounts against a session-less client: the page's getters are synchronous reads of a cache * `init` primes, so a screen that rendered first would record its first frame against a client that * knows no host, no state and no build. A document with no channel is not inside the shell and no * `init` is ever coming, so it says so and stops rather than waiting out a backoff nobody answers. + * + * The URL is rewritten before the tree is handed over, never after: expo-router reads the location + * when its root mounts, and the location it would read is `/`, the one path the shell serves and + * the one path no screen in this bundle claims. A shell too old to name a route leaves the page + * with nothing to open, which is a thing to say and not a screen to guess at. */ -export function bootstrapShellPage(options: { - target: PageMountTarget - client: BridgeRpcClient | null - mount: (client: BridgeRpcClient, session: BridgeShellSession) => void -}): void { - const { target, client, mount } = options +export function bootstrapShellPage(options: ShellPageBootstrapOptions): void { + const { target, client, replaceUrl, mount, refuseUnroutedShell } = options if (client === null) { stampPageMountState(target, 'unbridged') return } - const mountWithSession = (): boolean => { + const settleWithSession = (): boolean => { const session = client.getShellSession() if (session === null) { return false } target.dataset[PAGE_SESSION_ID_KEY] = session.sessionId target.dataset[PAGE_BUILD_ID_KEY] = session.buildId + if (session.route === null) { + // Terminal, and correctly so. A shell that names no screen is one built before `init` carried + // a route, and it will not learn one: a later `init` from that same shell names no screen + // either. Waiting for one would leave a blank document behind a panel nobody replaces. + stampPageMountState(target, 'shell-too-old') + refuseUnroutedShell() + return true + } + replaceUrl(shellRouteHref(session.route)) stampPageMountState(target, 'shell-ready') mount(client, session) return true } - if (mountWithSession()) { + if (settleWithSession()) { return } // `onReady` fires once and clears its listeners, so one document mounts one tree: a later `init` // under a new session id fails what the page held rather than mounting a second route tree over it. client.onReady(() => { - mountWithSession() + settleWithSession() }) } diff --git a/mobile/src/mobile-web-shell/mobile-web-shell-route.test.tsx b/mobile/src/mobile-web-shell/mobile-web-shell-route.test.tsx index b2753ae54f2..da47619ea9c 100644 --- a/mobile/src/mobile-web-shell/mobile-web-shell-route.test.tsx +++ b/mobile/src/mobile-web-shell/mobile-web-shell-route.test.tsx @@ -2,9 +2,20 @@ import { createElement } from 'react' import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' import { beforeEach, describe, expect, it, vi } from 'vitest' -type RouteDependencies = { storage: Map; mounted: string[] } +type RouteDependencies = { + storage: Map + mounted: string[] + /** The pathname each mount was told to open, which is the only thing the page can route on. */ + pathnames: string[] + hostId: string +} -const dependencies = vi.hoisted((): RouteDependencies => ({ storage: new Map(), mounted: [] })) +const dependencies = vi.hoisted((): RouteDependencies => ({ + storage: new Map(), + mounted: [], + pathnames: [], + hostId: 'host-1' +})) vi.mock('@react-native-async-storage/async-storage', () => ({ default: { @@ -23,16 +34,18 @@ vi.mock('react-native', () => ({ vi.mock('expo-router', () => ({ Redirect: 'Redirect', - useLocalSearchParams: () => ({ hostId: 'host-1' }) + useLocalSearchParams: () => ({ hostId: dependencies.hostId }) })) vi.mock('./MobileWebShellScreen', () => ({ - MobileWebShellScreen: (props: { hostId: string }) => { + MobileWebShellScreen: (props: { hostId: string; route: { pathname: string } }) => { dependencies.mounted.push(props.hostId) + dependencies.pathnames.push(props.route.pathname) return null } })) +import { BRIDGE_ROUTE_PATHNAME_PATTERN } from './bridge/bridge-caps' import MobileWebShellRoute from '../../app/h/[hostId]/web' /** Host elements are matched by name, not by `findAllByType`: React's `ElementType` does not admit @@ -66,6 +79,8 @@ describe('the hybrid shell route', () => { beforeEach(() => { dependencies.storage.clear() dependencies.mounted.length = 0 + dependencies.pathnames.length = 0 + dependencies.hostId = 'host-1' setDevelopmentBuild(true) }) @@ -89,6 +104,34 @@ describe('the hybrid shell route', () => { expect(dependencies.mounted).toEqual(['host-1']) }) + it('encodes the host id into the pathname, so no host id can bend the route', async () => { + dependencies.storage.set('orca:mobileWebShellEnabled', 'true') + // Every shape the bridge's pathname rule refuses, reached through a host id the app will + // happily route to: a query, a fragment, whitespace, a separator and a backslash. + for (const hostId of ['a?b', 'a#b', 'a b', 'a/b', 'a\\b']) { + dependencies.hostId = hostId + dependencies.pathnames.length = 0 + await renderRoute() + const pathname = dependencies.pathnames[0] + expect(pathname, hostId).toBe(`/h/${encodeURIComponent(hostId)}`) + expect(BRIDGE_ROUTE_PATHNAME_PATTERN.test(pathname ?? ''), hostId).toBe(true) + // And it still names the host it was opened for. + expect(decodeURIComponent((pathname ?? '').slice('/h/'.length)), hostId).toBe(hostId) + } + }) + + it('cannot encode a dot-segment host id away, and does not pretend to', async () => { + dependencies.storage.set('orca:mobileWebShellEnabled', 'true') + dependencies.hostId = '..' + await renderRoute() + // `encodeURIComponent` leaves a dot alone, and percent-escaping one would not help either: the + // URL parser treats `%2e%2e` as a dot segment too. So this one reaches the bridge as a route + // the pattern refuses, and the host is what turns it into a failure screen rather than a blank + // WebView. Deep links are the way in, which is why it is worth having a verdict for. + expect(dependencies.pathnames).toEqual(['/h/..']) + expect(BRIDGE_ROUTE_PATHNAME_PATTERN.test('/h/..')).toBe(false) + }) + it('redirects a store build whose container kept a flag a development build set', async () => { setDevelopmentBuild(undefined) dependencies.storage.set('orca:mobileWebShellEnabled', 'true') diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts index 57f8e78e0f2..935a1b852da 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.test.ts @@ -102,9 +102,12 @@ function Harness(props: { const view = useMobileWebShellBridge({ hostId: 'host-1', session: props.session, + // Built inline on every render, as a caller writes it: the host is not rebuilt for it. + route: { pathname: '/h/host-1' }, // A fresh closure every render, which is the shape a screen passes and the one a ref must // absorb: rebuilding the host here would settle every pending request on each render. onPageFault: (error) => props.faults.push(error), + onRouteRefused: () => {}, onPageReady: () => { props.readies.push( props.session.kind === 'ready' ? props.session.sessionId : props.session.kind @@ -215,6 +218,23 @@ describe('the bridge channel', () => { ]) }) + it('names the screen the page stands in for, so its document at `/` is not what it opens', async () => { + const mounted = await mount(readyState('session-one')) + await mounted.deliver(clientFrame({ type: 'ready' })) + expect(mounted.frames('session-one')).toEqual([ + expect.objectContaining({ type: 'init', route: { pathname: '/h/host-1' } }) + ]) + }) + + it('does not rebuild the host for a route object the caller built again', async () => { + const mounted = await mount(readyState('session-one')) + await mounted.deliver(clientFrame({ type: 'request', id: ID, method: 'status.get' })) + // Same session, re-rendered: the harness passes a fresh `{ pathname }` every time. A rebuilt + // host would have settled that request delivery-unknown on its way out. + await mounted.update(readyState('session-one')) + expect(mounted.frames('session-one').filter((frame) => frame.type === 'error')).toEqual([]) + }) + it('hands a page fault to the screen and asks the client for nothing', async () => { const mounted = await mount(readyState('session-one')) // The grant comes with the session, so the page asks for one before it reports anything. diff --git a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts index 8ec8dcc1632..52e187b0b05 100644 --- a/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts +++ b/mobile/src/mobile-web-shell/use-mobile-web-shell-bridge.ts @@ -5,6 +5,7 @@ import type { } from '../../modules/orca-mobile-web-shell/src' import { useHostClient } from '../transport/client-context' import { createBridgeDiagnosticReporter } from './bridge-diagnostic-log' +import type { BridgeInitRoute } from './bridge/bridge-envelope' import { createBridgeHost, type BridgeHost } from './bridge-host' import type { BridgeErrorCapture } from './bridge/bridge-error-capture' import type { MobileWebShellSessionState } from './mobile-web-shell-session-contract' @@ -53,10 +54,14 @@ export type MobileWebShellBridgeView = { export function useMobileWebShellBridge(args: { hostId: string session: MobileWebShellSessionState + /** The screen the page is standing in for, which the document's own `/` cannot tell it. */ + route: BridgeInitRoute /** The page could not render the generation on screen. Reported, never recovered from here. */ onPageFault: (error: BridgeErrorCapture) => void /** The page asked for a session. Reported so the screen can stop waiting for it. */ onPageReady: () => void + /** This shell named a screen the protocol does not allow, so no session is served. */ + onRouteRefused: (issue: string) => void }): MobileWebShellBridgeView { const { client } = useHostClient(args.hostId) const ready = args.session.kind === 'ready' ? args.session : null @@ -64,16 +69,23 @@ export function useMobileWebShellBridge(args: { const buildId = ready?.buildId ?? null const viewRef = useRef(null) const hostRef = useRef(null) - // Read through a ref: the host is built once per session, and a caller's fresh closure every - // render must not tear one down and settle its pendings. + // Fixed for the life of one host: the page routes once, before its first render, so a route that + // changed afterwards would have nothing left to change. Held in a ref for that reason — an inline + // object in the deps would rebuild the host on every render and settle its pendings each time. + const routeRef = useRef(args.route) + // Read through a ref for the same reason: a caller's fresh closure every render must not tear a + // host down and settle its pendings. const pageFaultRef = useRef(args.onPageFault) const pageReadyRef = useRef(args.onPageReady) - // Commit-phase and declared above the host's effect, so the host is built against the callbacks - // this render passed: a native frame can land between a commit and a passive effect. + const routeRefusedRef = useRef(args.onRouteRefused) + // Commit-phase and declared above the host's effect, so the host is built against what this + // render passed: a native frame can land between a commit and a passive effect. useLayoutEffect(() => { + routeRef.current = args.route pageFaultRef.current = args.onPageFault pageReadyRef.current = args.onPageReady - }, [args.onPageFault, args.onPageReady]) + routeRefusedRef.current = args.onRouteRefused + }, [args.onPageFault, args.onPageReady, args.onRouteRefused, args.route]) // Commit-phase, not passive: a native frame that arrives between the two carries the session id // the handler is fenced on, so only handing the host over here keeps it off the retired client. @@ -85,12 +97,16 @@ export function useMobileWebShellBridge(args: { client, buildId, sessionId, + route: routeRef.current, onPageFault: (error) => { pageFaultRef.current(error) }, onPageReady: () => { pageReadyRef.current() }, + onRouteRefused: (issue) => { + routeRefusedRef.current(issue) + }, post: (json) => { const mounted = viewRef.current return mounted === null || mounted.sessionId !== sessionId diff --git a/mobile/web-entry/index.tsx b/mobile/web-entry/index.tsx index b730bf9320f..b8062a5ec2e 100644 --- a/mobile/web-entry/index.tsx +++ b/mobile/web-entry/index.tsx @@ -35,6 +35,30 @@ function createRootProviders(client: BridgeRpcClient, target: PageMountTarget) { } } +/** + * The whole page for a shell that opened it and then named no screen. + * + * Built as elements rather than markup, and outside React: the route tree is exactly what cannot + * be mounted here, and a panel that needed it would be a second way to fail. The copy names the + * one thing that fixes it, because nothing on this device will. + */ +function renderShellTooOldPanel(container: HTMLElement): void { + const panel = document.createElement('div') + panel.setAttribute('role', 'alert') + panel.style.cssText = + 'font:16px/1.5 system-ui,-apple-system,sans-serif;color:#e6e6e6;background:#141414;' + + 'min-height:100vh;display:flex;flex-direction:column;align-items:center;' + + 'justify-content:center;gap:8px;padding:24px;text-align:center' + const title = document.createElement('div') + title.style.cssText = 'font-weight:600' + title.textContent = 'Update Orca to open this workspace' + const body = document.createElement('div') + body.style.cssText = 'color:#9a9a9a;font-size:14px' + body.textContent = 'This version of the app cannot open the workspace it downloaded.' + panel.append(title, body) + container.replaceChildren(panel) +} + const container = document.getElementById('root') if (!container) { throw new Error('[orca-mobile-web-app] #root missing') @@ -45,6 +69,9 @@ stampPageMountState(target, 'started') bootstrapShellPage({ target, client: createShellPageClient(), + replaceUrl: (href) => { + history.replaceState(null, '', href) + }, mount: (client) => { createRoot(container).render( // Above `ExpoRoot`, not inside its wrapper: a route this bundle cannot resolve or import @@ -54,8 +81,19 @@ bootstrapShellPage({ client.notifyPageFault(error) }} > - + ) + }, + refuseUnroutedShell: () => { + renderShellTooOldPanel(container) } })