From 7346e005a316d992dbfdb1d1309dfdedebe6df46 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 23 Sep 2026 00:16:05 -0400 Subject: [PATCH] refactor(mobile-web): tighten the bundle window contract and pin the range sibling stop The range bomb test reads only its own host's inflations, keyed by the gzip bodies that host sent, and plans twenty reads so a missing sibling stop is visible: with stopped.abort() removed it sends all twenty. The range params are an alias of the chunk params, and the chunk data bound is the exact base64 length of a full chunk. The phone's chunk and range replies share one header shape. The window reader closes over the client and bytes() takes the slot length the fetch computes. The host's positional read is readMobileWebBundleAssetWindow. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-web-bundle-fetch-range.test.ts | 79 +++++++++++++------ .../src/transport/mobile-web-bundle-fetch.ts | 17 +++- .../mobile-web-bundle-reply-schemas.ts | 25 +++--- .../mobile-web-bundle-window-reader.ts | 24 +++--- .../methods/mobile-web-bundle-asset-reader.ts | 2 +- .../runtime/rpc/methods/mobile-web-bundle.ts | 9 ++- .../bundle-rpc-contract.test.ts | 9 +-- .../mobile-web-bundle/bundle-rpc-contract.ts | 5 +- .../rpc-params-catalog.generated.ts | 7 +- 9 files changed, 112 insertions(+), 65 deletions(-) diff --git a/mobile/src/transport/mobile-web-bundle-fetch-range.test.ts b/mobile/src/transport/mobile-web-bundle-fetch-range.test.ts index 09849ab9b4d..bd2bf362b20 100644 --- a/mobile/src/transport/mobile-web-bundle-fetch-range.test.ts +++ b/mobile/src/transport/mobile-web-bundle-fetch-range.test.ts @@ -1,6 +1,6 @@ import { sha256 } from '@noble/hashes/sha256' import { gzipSync } from 'fflate' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { MOBILE_WEB_BUNDLE_CHUNK_BYTES, MOBILE_WEB_BUNDLE_RANGE_BYTES @@ -11,7 +11,13 @@ import { MobileWebBundleFetchError } from './mobile-web-bundle-fetch-refusal' import type { RpcClient } from './rpc-client' import type { RpcResponse } from './types' -const inflations = vi.hoisted(() => ({ outLengths: [] as number[], resultLengths: [] as number[] })) +type Inflation = { readonly outLength: number; readonly resultLength: number } + +/** Every inflation in the process, keyed by the gzip body it was handed; a host reads back only + * the bodies it sent, so a read left running by an earlier test's fetch never lands in its sink. */ +const inflationLog = vi.hoisted( + () => [] as { body: Uint8Array; outLength: number; resultLength: number }[] +) // Observes the bound the decoder hands fflate, and what fflate hands back inside it. vi.mock('fflate', async (importOriginal) => { @@ -19,9 +25,12 @@ vi.mock('fflate', async (importOriginal) => { return { ...fflate, gunzipSync: (data: Uint8Array, options?: { out?: Uint8Array }) => { - inflations.outLengths.push(options?.out?.byteLength ?? -1) const result = fflate.gunzipSync(data, options) - inflations.resultLengths.push(result.byteLength) + inflationLog.push({ + body: data, + outLength: options?.out?.byteLength ?? -1, + resultLength: result.byteLength + }) return result } } @@ -88,6 +97,7 @@ function rangeHost( assets } const calls: HostCall[] = [] + const sentGzipBodies = new Set() let wireBase64Bytes = 0 let inFlight = 0 let peakInFlight = 0 @@ -124,7 +134,11 @@ function rangeHost( const gzipped = gzipSync(slice, { level: 6 }) const encoding = gzipped.byteLength < slice.byteLength ? 'gzip' : 'identity' const body = encodeBase64(encoding === 'gzip' ? gzipped : slice) - return { ...header, encoding, dataBase64: options.tamper?.(call, body) ?? body } + const dataBase64 = options.tamper?.(call, body) ?? body + if (encoding === 'gzip') { + sentGzipBodies.add(dataBase64) + } + return { ...header, encoding, dataBase64 } } const client: RpcClient = { @@ -156,7 +170,22 @@ function rangeHost( client, calls, wireBase64Bytes: () => wireBase64Bytes, - peakInFlight: () => peakInFlight + peakInFlight: () => peakInFlight, + /** Waits until nothing is in flight and nothing new was sent for a few ticks, so a read a + * missing sibling stop would still send is counted instead of left pending. */ + drain: async (): Promise => { + let quiet = 0 + for (let tick = 0; tick < 2000 && quiet < 10; tick += 1) { + const sent = calls.length + await new Promise((resolve) => setTimeout(resolve, 0)) + quiet = inFlight === 0 && calls.length === sent ? quiet + 1 : 0 + } + }, + /** This host's inflations only: those whose body is one it sent. */ + inflations: (): Inflation[] => + inflationLog + .filter((entry) => sentGzipBodies.has(encodeBase64(entry.body))) + .map(({ outLength, resultLength }) => ({ outLength, resultLength })) } } @@ -192,11 +221,6 @@ async function refusalOf(failed: Promise): Promise { return error instanceof MobileWebBundleFetchError ? error.refusal : null } -beforeEach(() => { - inflations.outLengths.length = 0 - inflations.resultLengths.length = 0 -}) - describe('fetchMobileWebBundle from a host whose manifest names a range grid', () => { it('pages every asset in ranges on that grid and returns the verified bytes', async () => { const host = rangeHost(FILES) @@ -262,19 +286,30 @@ describe('fetchMobileWebBundle from a host whose manifest names a range grid', ( expect(await refusalOf(fetchMobileWebBundle({ client: host.client }))).toBe('range-undecodable') }) - // A 4 MiB inflation answering a 600-byte window: fflate fills the bounded buffer and stops there. - it('refuses a gzip bomb as overlong without inflating past one byte over the window', async () => { - const bomb = gzipSync(new Uint8Array(4 * 1024 * 1024), { level: 9 }) - const host = rangeHost(FILES, { - tamper: (call, body) => (call.params.path === 'index.html' ? encodeBase64(bomb) : body) + // A 4 MiB inflation answering the first 384 KiB window: fflate fills the bounded buffer and stops + // there. Twenty reads are planned, so a fetch that kept going after the refusal is visible. + it('refuses a gzip bomb at one byte over the window and sends little after it', async () => { + const bomb = encodeBase64(gzipSync(new Uint8Array(4 * 1024 * 1024), { level: 9 })) + const many = Object.fromEntries( + Array.from({ length: 10 }, (_, index) => [ + `assets/part-${String(index)}.js`, + scriptBytes(MOBILE_WEB_BUNDLE_RANGE_BYTES * 2, index) + ]) + ) + const host = rangeHost(many, { + tamper: (call, body) => + call.params.path === 'assets/part-0.js' && call.params.offset === 0 ? bomb : body }) - expect(await refusalOf(fetchMobileWebBundle({ client: host.client }))).toBe('asset-overlong') - // Order-free: a stray read from an earlier test may inflate into this test's record too. - expect(inflations.resultLengths).toContain(601) - for (const [call, resultLength] of inflations.resultLengths.entries()) { - expect(resultLength).toBeLessThanOrEqual(inflations.outLengths[call]!) - } + expect(await refusalOf(fetchMobileWebBundle({ client: host.client }))).toBe('chunk-oversize') + await host.drain() + const window = MOBILE_WEB_BUNDLE_RANGE_BYTES + 1 + // Exactly one body filled the spare byte: the bomb, stopped there. + expect(host.inflations().filter((inflation) => inflation.resultLength === window)).toEqual([ + { outLength: window, resultLength: window } + ]) + // The sibling stop: at most the four in flight and one more each, never all twenty. + expect(readsOf(host.calls, 'mobileWeb.bundle.range').length).toBeLessThanOrEqual(8) }) it('refuses a range that inflates short of its window as short', async () => { diff --git a/mobile/src/transport/mobile-web-bundle-fetch.ts b/mobile/src/transport/mobile-web-bundle-fetch.ts index fec4d5e0d4f..fbc4db922bc 100644 --- a/mobile/src/transport/mobile-web-bundle-fetch.ts +++ b/mobile/src/transport/mobile-web-bundle-fetch.ts @@ -59,13 +59,13 @@ export async function fetchMobileWebBundle(args: { throwIfCallerAborted(args.signal) const opened = await runRpcOperation(args.client, mobileWebBundleManifestRead, null) const manifest = opened.manifest - const reader = mobileWebBundleWindowReader(opened) + const reader = mobileWebBundleWindowReader(args.client, opened) const queue = planWindowReads(manifest.assets, reader.windowBytes) const assets = new Map() let receivedBytes = 0 const readWindow = async ({ asset, offset }: WindowRead): Promise => { - const reply = await reader.read(args.client, { + const reply = await reader.read({ buildId: manifest.buildId, path: asset.entry.path, offset @@ -75,7 +75,7 @@ export async function fetchMobileWebBundle(args: { return } assertWindowDescribesAsset(reply.header, asset.entry, manifest.buildId, offset) - const bytes = reply.bytes() + const bytes = reply.bytes(windowSlotBytes(asset.entry, offset, reader.windowBytes)) assertWindowFillsItsSlot( asset.entry, offset, @@ -141,6 +141,15 @@ function planWindowReads( }) } +/** The bytes the grid slot at `offset` holds: a whole window, or the asset's tail. */ +function windowSlotBytes( + entry: MobileWebBundleAssetRead, + offset: number, + windowBytes: number +): number { + return Math.min(windowBytes, entry.byteLength - offset) +} + /** Offsets are planned, so a reply is accepted only if it fills exactly its slot of the grid. */ function assertWindowFillsItsSlot( entry: MobileWebBundleAssetRead, @@ -150,7 +159,7 @@ function assertWindowFillsItsSlot( windowBytes: number ): void { const { path, byteLength: declared } = entry - const expected = Math.min(windowBytes, declared - offset) + const expected = windowSlotBytes(entry, offset, windowBytes) if (byteLength === expected && eof === offset + windowBytes >= declared) { return } diff --git a/mobile/src/transport/mobile-web-bundle-reply-schemas.ts b/mobile/src/transport/mobile-web-bundle-reply-schemas.ts index 481c708578c..e8bcb3c3230 100644 --- a/mobile/src/transport/mobile-web-bundle-reply-schemas.ts +++ b/mobile/src/transport/mobile-web-bundle-reply-schemas.ts @@ -129,29 +129,28 @@ export const MobileWebBundleManifestReplySchema = z.looseObject({ /** Self-describing on purpose: `buildId`, `path` and `offset` are echoed so a reassembler cannot * misplace a reply, and `sha256`/`assetByteLength` describe the whole asset rather than this - * chunk, which is what lets the fetch verify without a second index. */ -export const MobileWebBundleChunkReplySchema = z.looseObject({ + * window, which is what lets the fetch verify without a second index. Shared by both read replies. */ +const windowHeaderShape = { buildId: z.string().regex(SHA256_PATTERN), path: MobileWebBundleAssetPathSchema, offset: z.number().int().nonnegative().max(MOBILE_WEB_BUNDLE_MAX_ASSET_BYTES), assetByteLength: z.number().int().nonnegative().max(MOBILE_WEB_BUNDLE_MAX_ASSET_BYTES), sha256: z.string().regex(SHA256_PATTERN), - dataBase64: z.string().max(MAX_DATA_BASE64_LENGTH), eof: z.boolean() +} + +export const MobileWebBundleChunkReplySchema = z.looseObject({ + ...windowHeaderShape, + dataBase64: z.string().max(MAX_DATA_BASE64_LENGTH) }) -/** A chunk's self-description plus the encoding of `dataBase64`. `encoding` is read as a string, not - * a closed enum: an encoding this build cannot decode is a typed refusal at the decoder, which - * names it, rather than a reply-shape failure that names nothing. */ +/** The window header plus the encoding of `dataBase64`. `encoding` is read as a string, not a closed + * enum: an encoding this build cannot decode is a typed refusal at the decoder, which names it, + * rather than a reply-shape failure that names nothing. */ export const MobileWebBundleRangeReplySchema = z.looseObject({ - buildId: z.string().regex(SHA256_PATTERN), - path: MobileWebBundleAssetPathSchema, - offset: z.number().int().nonnegative().max(MOBILE_WEB_BUNDLE_MAX_ASSET_BYTES), - assetByteLength: z.number().int().nonnegative().max(MOBILE_WEB_BUNDLE_MAX_ASSET_BYTES), - sha256: z.string().regex(SHA256_PATTERN), + ...windowHeaderShape, encoding: z.string().min(1).max(32), - dataBase64: z.string().max(MOBILE_WEB_BUNDLE_RANGE_MAX_DATA_BASE64_LENGTH), - eof: z.boolean() + dataBase64: z.string().max(MOBILE_WEB_BUNDLE_RANGE_MAX_DATA_BASE64_LENGTH) }) export type MobileWebBundleManifestReply = z.output diff --git a/mobile/src/transport/mobile-web-bundle-window-reader.ts b/mobile/src/transport/mobile-web-bundle-window-reader.ts index 959904250ad..3eb3b1a0572 100644 --- a/mobile/src/transport/mobile-web-bundle-window-reader.ts +++ b/mobile/src/transport/mobile-web-bundle-window-reader.ts @@ -16,29 +16,32 @@ export type MobileWebBundleWindowHeader = { export type MobileWebBundleWindowReply = { readonly header: MobileWebBundleWindowHeader - /** Decoded on demand, so a reply the fetch discards or refuses by its header is never inflated. */ - readonly bytes: () => Uint8Array + /** Decoded on demand, so a reply the fetch discards or refuses by its header is never inflated. + * `expected` is the slot length on the grid; a range inflates into at most one byte past it. */ + readonly bytes: (expected: number) => Uint8Array } -/** One grid and one read method, fixed for a whole fetch by what the manifest reply named. */ +/** One client, one grid and one read method, fixed for a whole fetch by the manifest reply. */ export type MobileWebBundleWindowReader = { readonly windowBytes: number - read( - client: RpcClient, - window: { buildId: string; path: string; offset: number } - ): Promise + read(window: { + buildId: string + path: string + offset: number + }): Promise } /** Ranges when the host named a range grid, chunks otherwise: a host that predates the range * method names none, and every bundle host serves chunks. */ export function mobileWebBundleWindowReader( + client: RpcClient, opened: Pick ): MobileWebBundleWindowReader { const { rangeBytes } = opened if (rangeBytes === undefined) { return { windowBytes: opened.chunkBytes, - read: async (client, window) => { + read: async (window) => { const { dataBase64, ...header } = await runRpcOperation( client, mobileWebBundleChunkRead, @@ -50,16 +53,15 @@ export function mobileWebBundleWindowReader( } return { windowBytes: rangeBytes, - read: async (client, window) => { + read: async (window) => { const { dataBase64, encoding, ...header } = await runRpcOperation( client, mobileWebBundleRangeRead, window ) - const expected = Math.max(0, Math.min(rangeBytes, header.assetByteLength - header.offset)) return { header, - bytes: () => + bytes: (expected) => decodeMobileWebBundleRange( { path: header.path, offset: header.offset, encoding }, decodeBase64(dataBase64), diff --git a/src/main/runtime/rpc/methods/mobile-web-bundle-asset-reader.ts b/src/main/runtime/rpc/methods/mobile-web-bundle-asset-reader.ts index 1cc87e02389..9979e6bdc85 100644 --- a/src/main/runtime/rpc/methods/mobile-web-bundle-asset-reader.ts +++ b/src/main/runtime/rpc/methods/mobile-web-bundle-asset-reader.ts @@ -60,7 +60,7 @@ async function hashAsset(root: string, asset: MobileWebBundleAsset): Promise( throw bundleError('mobile_web_bundle_asset_changed') } abortIfDisconnected(ctx) - const data = await readMobileWebBundleAssetChunk(bundle.root, asset, params.offset, windowBytes) + const data = await readMobileWebBundleAssetWindow( + bundle.root, + asset, + params.offset, + windowBytes + ) abortIfDisconnected(ctx) return await encode({ header: { diff --git a/src/shared/mobile-web-bundle/bundle-rpc-contract.test.ts b/src/shared/mobile-web-bundle/bundle-rpc-contract.test.ts index 1b4e54c8e4e..0bf0b21b8af 100644 --- a/src/shared/mobile-web-bundle/bundle-rpc-contract.test.ts +++ b/src/shared/mobile-web-bundle/bundle-rpc-contract.test.ts @@ -19,7 +19,7 @@ import { import { MOBILE_WEB_BUNDLE_CAPABILITY } from './mobile-web-bundle-capability' const BUILD_ID = 'a'.repeat(64) -const MAX_DATA_BASE64_LENGTH = Math.ceil(MOBILE_WEB_BUNDLE_CHUNK_BYTES / 3) * 4 + 8 +const MAX_DATA_BASE64_LENGTH = Math.ceil(MOBILE_WEB_BUNDLE_CHUNK_BYTES / 3) * 4 function hexDigest(input: string): string { return Array.from(sha256(new TextEncoder().encode(input)), (byte) => @@ -168,12 +168,11 @@ describe('mobileWeb.bundle.chunk result', () => { ).toBe(false) }) - it('leaves the padding slack the +8 term buys, so the host enforces the chunk size', () => { + it('is exact: base64 of one byte past a full chunk is refused', () => { + expect(MAX_DATA_BASE64_LENGTH).toBe(65536) const overshoot = Buffer.alloc(MOBILE_WEB_BUNDLE_CHUNK_BYTES + 1).toString('base64') - expect(overshoot.length).toBeLessThanOrEqual(MAX_DATA_BASE64_LENGTH) - const wellPast = Buffer.alloc(MOBILE_WEB_BUNDLE_CHUNK_BYTES + 64).toString('base64') expect( - MobileWebBundleChunkResultSchema.safeParse(chunkResult({ dataBase64: wellPast })).success + MobileWebBundleChunkResultSchema.safeParse(chunkResult({ dataBase64: overshoot })).success ).toBe(false) }) diff --git a/src/shared/mobile-web-bundle/bundle-rpc-contract.ts b/src/shared/mobile-web-bundle/bundle-rpc-contract.ts index b6654633df1..d740e78fee9 100644 --- a/src/shared/mobile-web-bundle/bundle-rpc-contract.ts +++ b/src/shared/mobile-web-bundle/bundle-rpc-contract.ts @@ -27,7 +27,8 @@ export const MOBILE_WEB_BUNDLE_RANGE_METHOD = 'mobileWeb.bundle.range' export const MOBILE_WEB_BUNDLE_RANGE_ENCODINGS = ['gzip', 'identity'] as const export type MobileWebBundleRangeEncoding = (typeof MOBILE_WEB_BUNDLE_RANGE_ENCODINGS)[number] -const MAX_DATA_BASE64_LENGTH = Math.ceil(MOBILE_WEB_BUNDLE_CHUNK_BYTES / 3) * 4 + 8 +/** Exact base64 lengths of a full window: these bound only what this host produces. */ +const MAX_DATA_BASE64_LENGTH = Math.ceil(MOBILE_WEB_BUNDLE_CHUNK_BYTES / 3) * 4 export const MOBILE_WEB_BUNDLE_RANGE_MAX_DATA_BASE64_LENGTH = Math.ceil(MOBILE_WEB_BUNDLE_RANGE_BYTES / 3) * 4 @@ -74,7 +75,7 @@ const windowParamsShape = { export const MobileWebBundleChunkParamsSchema = z.object(windowParamsShape).strict() /** The chunk params exactly, on the `rangeBytes` grid the manifest reply advertised. */ -export const MobileWebBundleRangeParamsSchema = z.object(windowParamsShape).strict() +export const MobileWebBundleRangeParamsSchema = MobileWebBundleChunkParamsSchema /** What every chunk or range reply restates about the window it answers. */ const windowHeaderShape = { diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index cb399cac791..e27aaa43ff1 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -22,10 +22,7 @@ import { PairingGetEndpointsParamsSchema, PairingProvisionRelayParamsSchema } from '../mobile-relay-credential-contract' -import { - MobileWebBundleChunkParamsSchema, - MobileWebBundleRangeParamsSchema -} from '../mobile-web-bundle/bundle-rpc-contract' +import { MobileWebBundleChunkParamsSchema } from '../mobile-web-bundle/bundle-rpc-contract' import { pluginConsentRequestSchema } from '../plugins/plugin-consent-request' import { AccountsUnsubscribeParams, @@ -967,7 +964,7 @@ export const RPC_PARAMS_BY_METHOD = { 'markdown.saveTab': SaveMarkdownTab, 'mobileWeb.bundle.chunk': MobileWebBundleChunkParamsSchema, 'mobileWeb.bundle.manifest': null, - 'mobileWeb.bundle.range': MobileWebBundleRangeParamsSchema, + 'mobileWeb.bundle.range': MobileWebBundleChunkParamsSchema, 'nativeChat.readSession': NativeChatSession, 'nativeChat.subscribe': NativeChatSession, 'nativeChat.unsubscribe': NativeChatUnsubscribe,