diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d24842f1..871d1d794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,13 @@ history. ## Unreleased -0.44.0 +- `fjs/protocol/json_rpc` `dispatch` looks up handlers by own property. + An `Object.prototype` method name arriving as `method` no longer throws + or emits a malformed response; it answers `-32601` like any other + unknown method + [#1514](https://github.com/functionalscript/functionalscript/pull/1514) + +## 0.44.0 - **BREAKING CHANGES:** `fjs/ci/deno` no longer exports `coverageInclude`. The Deno CI job runs `deno task cov`, so `deno.json` owns the coverage diff --git a/fjs/protocol/json_rpc/module.f.mjs b/fjs/protocol/json_rpc/module.f.mjs index 250846741..f96ab53af 100644 --- a/fjs/protocol/json_rpc/module.f.mjs +++ b/fjs/protocol/json_rpc/module.f.mjs @@ -15,9 +15,10 @@ * @module * * @import { Unknown } from '../../media/json/types.ts' - * @import { Id, RpcError, Handler, Handlers, Response } from './types.ts' + * @import { Id, RpcError, Handlers, Response } from './types.ts' */ +import { at } from '../../types/object/module.f.mjs' import { number, string, or, option } from '../../types/rtti/module.f.mjs' import { validate } from '../../types/rtti/validate/module.f.mjs' import { unknown } from '../../media/json/rtti/module.f.mjs' @@ -100,9 +101,11 @@ export const dispatch = handlers => value => { if (id === undefined) { return null } - /** @type {Handler | undefined} */ - const handler = handlers[method] - if (handler === undefined) { + // `at`, not `handlers[method]`: `method` is untrusted wire data, and a + // bracket lookup on a plain object resolves inherited `Object.prototype` + // names (`constructor`, `toString`, …) to callables. + const handler = at(method)(handlers) + if (handler === null) { return errorResponseOf(id)(methodNotFound) } const [t2, result] = handler(params) diff --git a/fjs/protocol/json_rpc/proof.f.mjs b/fjs/protocol/json_rpc/proof.f.mjs index 4b7fa5944..41f0eeb38 100644 --- a/fjs/protocol/json_rpc/proof.f.mjs +++ b/fjs/protocol/json_rpc/proof.f.mjs @@ -25,6 +25,9 @@ const handlers = { } const d = dispatch(handlers) +/** A handler table that does own the `Object.prototype` name `toString`. */ +const dOwnPrototypeName = dispatch({ toString: () => ok('own') }) + export const proof = { schema: { request: { @@ -72,6 +75,21 @@ export const proof = { const r = d({ jsonrpc: '2.0', method: 'nope', id: 4 }) assert(r !== null && 'error' in r && r.error.code === -32601) }, + // `method` is untrusted wire data: an inherited `Object.prototype` + // name must not resolve to a callable. + inheritedToString: () => { + const r = d({ jsonrpc: '2.0', method: 'toString', id: 4 }) + assert(r !== null && 'error' in r && r.error.code === -32601) + }, + inheritedConstructor: () => { + const r = d({ jsonrpc: '2.0', method: 'constructor', id: 4 }) + assert(r !== null && 'error' in r && r.error.code === -32601) + }, + // ... while an *own* property of that name is an ordinary method. + ownPrototypeName: () => { + const r = dOwnPrototypeName({ jsonrpc: '2.0', method: 'toString', id: 4 }) + assert(r !== null && 'result' in r && r.result === 'own') + }, invalidRequest: () => { const r = d({ jsonrpc: '1.0', method: 'ping', id: 5 }) assert(r !== null && 'error' in r && r.error.code === -32600 && r.id === null) diff --git a/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md b/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md deleted file mode 100644 index a9fb14ce9..000000000 --- a/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md +++ /dev/null @@ -1,53 +0,0 @@ -## `dispatch` looks up handlers with bracket indexing - -**Priority:** P2 -**Status:** open - -### Problem - -`module.f.mjs:103-107`: - -```js -/** @type {Handler | undefined} */ -const handler = handlers[method] -if (handler === undefined) { - return errorResponseOf(id)(methodNotFound) -} -``` - -`method` is untrusted wire data and `handlers` is an ordinary object, so an -inherited `Object.prototype` name resolves to a callable. Reproduced against -the real module: - -``` -{method: 'constructor'} → TypeError thrown out of dispatch -{method: 'toString'} → {"jsonrpc":"2.0","error":"o","id":1} -``` - -(the second destructures `"[object Undefined]"` character-wise). A pure -dispatcher documented to answer `-32601` instead throws out of the caller or -emits a schema-violating response. - -Every other dispatch site in the repo already uses `at` from -`fjs/types/object`, each with a comment naming this exact hazard — -`match` (`fjs/effects/module.f.mjs:344-355`), `cli/dispatch` -(`fjs/cli/module.f.mjs:36-45`), `addRevisionToCache` -(`fjs/cas/evo/module.f.mjs:143-149`). `json_rpc` is the one that reads raw. - -### Proposal - -`const handler = at(method)(handlers)`, testing `=== null`. The hand-written -`/** @type {Handler | undefined} */` annotation (needed only because the -index-signature type lies) goes away too. Add `throw`-free proofs for -`constructor` / `toString` methods answering `methodNotFound`. - -### Tasks - -- [ ] Switch the lookup to `at` and drop the annotation -- [ ] Add proofs for prototype-name methods - -### Related - -- [effectful-dispatch-skeleton](effectful-dispatch-skeleton.md) — quotes - these lines but leaves the lookup as-is; whichever lands first should carry - the fix diff --git a/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md b/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md index 0d093c36b..6b6659478 100644 --- a/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md +++ b/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md @@ -9,20 +9,20 @@ The JSON-RPC request preamble — decode the envelope, answer a malformed one with `Invalid Request` (`id: null`), and split notifications (`id === undefined`) from requests — is spelled out twice. -Pure `dispatch` (`fjs/protocol/json_rpc/module.f.mjs:94-112`): +Pure `dispatch` (`fjs/protocol/json_rpc/module.f.mjs:95-110`): -```ts +```js const [t, message] = decodeRequest(value) if (t === 'error') { return errorResponseOf(null)(invalidRequest) } const { id, method, params } = message if (id === undefined) { return null } -const handler: Handler | undefined = handlers[method] -if (handler === undefined) { return errorResponseOf(id)(methodNotFound) } +const handler = at(method)(handlers) +if (handler === null) { return errorResponseOf(id)(methodNotFound) } ``` Effectful `mcpStep` (`fjs/protocol/mcp/module.f.mjs:264-287`): -```ts +```js const [t, message] = decodeRequest(value) if (t === 'error') { return pure(_errResponse(null)(invalidRequest)) } const { id, method, params } = message