feat(rpc,openapi): add end-to-end HTTP QUERY support - #1844
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
This PR introduces a security boundary bypass: a malicious client can tunnel an unsafe POST sub-request through a safe outer QUERY batch, and the server executes it. The QUERY batch path does not enforce the "safe batch only carries safe sub-requests" rule that the GET path already enforces and that this PR's docs claim.
Reviewed changes — thorough end-to-end HTTP QUERY (RFC 10008) support across client, server, and openapi in 33 files:
- RPC transport —
RPCLink/RPCLinkCodecacceptmethod: 'QUERY'andfallbackMethod: 'QUERY'(body-encoded GET fallback); RPC handler keepsQUERYout of defaultallowMethods(opt-in), andRPC_DEFAULT_ALLOW_METHODSis unchanged. - OpenAPI —
openapi()meta acceptsmethod: 'QUERY'; the generator emits aqueryPath Item and hard-errors unlessbase.openapiis'3.2.0';OpenAPIDocumentnarrowsopenapito add'3.2.0'and addsOpenAPIPathItemObject.query. - Deduplication — default filter now covers both
GETandQUERY. - Batching — client partitions
GET/QUERY/unsafe into separate outer batches; server validates safe outerGET/QUERYbatches contain only same-method sub-requests (see inline finding). - CORS — default
Access-Control-Allow-MethodsaddsQUERY. - Docs + playground + e2e — routines,
searchPlanetsexample, and a node-http QUERY batch round-trip test.
Despite the breadth, this is well-tested and clearly written; the batching security boundary is the one real issue. Everything else — codec fallback, OpenAPI 3.2 gating, dedupe default, CORS default, and the node-http/OpenAPI round trips — checked out against the implementation.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — single new commit (9f470f98 fix(server): validate body-encoded batch messages) addressing the prior security finding:
- Closed the QUERY batch smuggling hole — the QUERY/body parse branch now applies the same
isClientPeerSendMessageguard already used by the GET branch and the WebSocket/message-port/crossws handlers, so akind: 'response'(or other non-request) message can no longer be executed as aPOSTsub-request under a safe outerQUERYbatch. - Unified the safe-batch method check — the GET-only mismatch check was consolidated into one outer-method validation (
GET/QUERYbatches carry only same-method request sub-requests, else400), preserving the prior GET behavior while extending it toQUERY. - Added regression coverage — a new test asserts a
QUERYbatch containing a{ kind: 'response', method: 'POST', url: '/ping' }message returns400before any handler runs.
The bottleneck change is minimal, correct, and covered by a test that fails without it. I verified the supplementary guard matches how the other adapters validate inbound messages, and that the unified check does not disturb the streaming/buffered paths. All 73 affected tests (server batch, client batch, CORS, e2e batch) pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
I don't think we need to document the |
dinwwwh
changed the title
There was a problem hiding this comment.
ℹ️ No code issues in this delta — but it strips the QUERY documentation, leaving the shipped feature undocumented and one doc line now contradicting the code.
Reviewed changes — single commit (2a0ccdab docs: remove QUERY method guidance) since the prior Pullfrog review. It reverts the QUERY documentation added earlier in this PR across eight docs files (routing.mdx, specification.mdx, batch.mdx, cors.mdx, dedupe.mdx, handler.mdx, link.mdx, protocol.mdx). Net effect against main: the PR's docs delta is now empty.
ℹ️ Feature ships with no documentation while the PR advertises it
The code in this PR still implements end-to-end QUERY support (RPCLink/fallbackMethod, the OpenAPI 3.2 query Path Item and its hard '3.2.0' gate, method-partitioned batching, CORS default, dedupe default), but this commit removes every QUERY section the PR added. The PR body explicitly lists "Documentation" as a deliverable ("The documentation now covers: …"), so the diff now contradicts the description. This looks like either an accidental revert of the docs work or an intentional deferral to a separate PR. If it's intentional, updating the PR description avoids the mismatch; if not, the QUERY docs should be restored before merge — otherwise users get a substantial feature with no way to discover how to enable or reason about it.
ℹ️ dedupe.mdx reverts to a statement that contradicts the shipped behavior
The revert restores apps/content/docs/plugins/dedupe.mdx to "By default, the plugin deduplicates only GET requests." That is now false for this PR: DedupeLinkPlugin's default filter is ({ request }) => request.method === 'GET' || request.method === 'QUERY' (packages/client/src/plugins/dedupe.ts:51), so the default deduplicates GET and QUERY. A reader relying on the docs would be misled about whether their safe QUERY requests are collapsed. (This line can't be anchored inline because it no longer differs from main in the PR diff.)
Technical details
# Restore or explicitly defer QUERY documentation
## Affected sites
- apps/content/docs/openapi/routing.mdx — "QUERY Method" section removed
- apps/content/docs/openapi/specification.mdx — "QUERY Operations" section removed (incl. the `base.openapi: '3.2.0'` requirement)
- apps/content/docs/plugins/batch.mdx — "Request Methods" section removed
- apps/content/docs/plugins/cors.mdx — note that the default `allowMethods` includes QUERY removed
- apps/content/docs/plugins/dedupe.mdx — reverted to "only GET", contradicting code default (dedupe.ts:51)
- apps/content/docs/rpc/handler.mdx, rpc/link.mdx, rpc/protocol.mdx — QUERY guidance removed
## Required outcome
- A user of the shipped QUERY feature must be able to discover, configure, and understand its semantics from the docs — or the author must confirm the docs are intentionally deferred and align the PR description accordingly (and the dedupe default statement must not contradict the code).
## Open questions for the human
- Is the docs removal intentional (separate docs PR coming), or an accidental revert that should be restored before merge?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues in this delta — a clean upstream-
mainmerge.
Reviewed changes — single commit (aebd399b Merge upstream main into feature/query-requests) since the prior Pullfrog review. It syncs main into the feature branch, bringing upstream's SimpleCsrfProtection Sec-Fetch-Site rework, a CORS origin/timingOrigin change (now string | undefined, reflected only when defined), and a batch sub-request-headers test. None of this is authored for the QUERY feature and none of it touches QUERY semantics.
I spot-checked the merged files to confirm the QUERY work survived intact: the CORS default still includes QUERY (cors.ts:83), the batch outer-method validation still covers GET/QUERY (batch.ts:175), and the codec method/fallbackMethod unions still accept QUERY (rpc-link-codec.ts:35,43). All 112 affected tests (dedupe, client + server batch, CORS, OpenAPI generator, e2e batch) pass on the merged tree.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/bun
@orpc/client
@orpc/cloudflare
@orpc/contract
@orpc/experimental-effect
@orpc/evlog
@orpc/hibernation
@orpc/json-schema
@orpc/nest
@orpc/next
@orpc/node
@orpc/openapi
@orpc/opentelemetry
@orpc/pinia-colada
@orpc/pino
@orpc/publisher
@orpc/ratelimit
@orpc/server
@orpc/shared
@orpc/swr
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/zod
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
✅ No new issues in this delta — the QUERY feature remains intact and the code is clean.
Reviewed changes — since the prior Pullfrog review, four QUERY-authored commits plus a main sync landed on feature/query-requests:
- Corrected the dedupe docs (
2ac80f40) —dedupe.mdxnow states the default dedupesGETandQUERY, matchingdedupe.ts:51and resolving the doc/code contradiction flagged earlier. - Restored OpenAPI QUERY docs + narrowed the document type (
4110b782) —specification.mdxagain documents the OpenAPI 3.2QUERYrequirement, andOpenAPIDocumentnow inlines the path-itemqueryfield (the intermediateOpenAPIPathItemObject/OpenAPIPathsObjectexports are unreferenced anywhere, so removing them is safe). - Removed the Bun playground
searchPlanetsexample (01bdcd46) — the playground generator was also reverted to the default OpenAPI 3.1. - Added a nest
@ImplementQUERY guard (3469719f) — a clearTypeErrorinstead of the obscureMethodDecoratorMap[method] is not a function, since NestJS has noQUERYroute decorator. - Merged more upstream
main(beb04e70) — CSRF-rework and client batch fixes that are upstream-authored, not part of the QUERY feature.
I verified the nest guard against MethodDecoratorMap (no QUERY key), confirmed the inlined path-item type change with the updated types.test-d.ts assertion, and ran the affected tests (108 pass) plus pnpm type:check.
ℹ️ PR description is now out of sync with the code
The description's "Bun playground" section still claims the searchPlanets demo is present, was "verified through OpenAPIHandler with a real QUERY request", and that the playground "generates its reference document as OpenAPI 3.2.0". Commit 01bdcd46 removes that procedure and reverts the playground to the default 3.1. The feature and its tests stand on their own, so this is a description-vs-code mismatch rather than a functional issue — but the description should be updated (or the example restored) before this is presented as shipped.
Technical details
# Restore the playground example or update the PR description
## Affected sites
- playgrounds/bun/src/routers/planet.ts — `searchPlanets` (QUERY /planets/search) removed
- playgrounds/bun/src/routers/index.ts — `search: searchPlanets` removed
- playgrounds/bun/src/index.ts — `openapi: '3.2.0'` reverted to default 3.1
- PR description "Bun playground" / "Scalar API Reference" sections — still describe the example as delivered
## Required outcome
- Either keep the playground `searchPlanets` demo (and its OpenAPI 3.2.0 generation) and test, or update the PR description so readers are not told a delivered example exists when it does not.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end support for the HTTP QUERY method across oRPC’s client/server transport stack and OpenAPI integration, aligning batching, deduplication, CORS defaults, and tests/docs with RFC 10008 semantics (safe + idempotent, body-bearing).
Changes:
- Add
QUERYsupport to client plugins (batching + dedupe) and transport codecs/links, with corresponding test coverage. - Extend server-side batching validation and CORS defaults to include
QUERY. - Add partial OpenAPI 3.2 typing/generation support for
queryoperations, plus documentation updates and NestJS guardrails.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/batch/batch-plugin.test.ts | Adds an e2e batch test asserting QUERY batching over node-http. |
| tests/batch/shared/client-server.ts | Extends shared test helper options to allow selecting QUERY. |
| tests/batch/shared/client-server.node-http.ts | Allows configuring link method; updates handler allowed methods (but currently misses GET). |
| tests/batch/shared/client-server.hono-fetch.ts | Makes method configurable (default GET) and allows QUERY. |
| packages/server/src/plugins/cors.ts | Adds QUERY to the default CORS Access-Control-Allow-Methods set. |
| packages/server/src/plugins/cors.test.ts | Updates expected allow-methods string and adds an explicit allowMethods override test. |
| packages/server/src/plugins/batch.ts | Tightens validation for body batches and enforces method-consistency for outer GET/QUERY batches. |
| packages/server/src/plugins/batch.test.ts | Adds QUERY batch request helpers/tests and strengthens invalid-body assertions. |
| packages/openapi/src/types.ts | Introduces an OpenAPIDocument type that supports OpenAPI 3.2 query operations in paths. |
| packages/openapi/src/types.test-d.ts | Adds type tests to ensure query path items are supported under OpenAPI 3.2. |
| packages/openapi/src/openapi-generator.ts | Rejects QUERY operations unless the base document is OpenAPI 3.2.0. |
| packages/openapi/src/openapi-generator.test.ts | Adds tests for generating and rejecting QUERY operations depending on OpenAPI version. |
| packages/openapi/src/meta.ts | Extends OpenAPI meta method union to include QUERY. |
| packages/openapi/src/adapters/fetch/openapi-link.test.ts | Adds a test verifying QUERY OpenAPI requests carry input in the body. |
| packages/nest/src/implement.ts | Throws if @Implement is used with QUERY since NestJS doesn’t support that method. |
| packages/nest/src/implement.test.ts | Adds coverage for the NestJS QUERY rejection behavior. |
| packages/client/src/plugins/dedupe.ts | Dedupe plugin defaults expanded to include QUERY requests. |
| packages/client/src/plugins/dedupe.test.ts | Adds tests for deduping QUERY requests (same vs different bodies). |
| packages/client/src/plugins/batch.ts | Splits batching into GET, QUERY, and unsafe (POST outer) batches. |
| packages/client/src/plugins/batch.test.ts | Updates tests to assert distinct GET/QUERY/unsafe batch grouping. |
| packages/client/src/adapters/standard/rpc-link-codec.ts | Extends method/fallback typing to allow QUERY. |
| packages/client/src/adapters/standard/rpc-link-codec.test.ts | Adds a test for GET URL overflow fallback to QUERY. |
| packages/client/src/adapters/fetch/rpc-link.test.ts | Adds a test asserting QUERY requests send body-encoded input. |
| packages/client/src/adapters/fetch/rpc-link.test-d.ts | Adds type assertions around allowed QUERY usage and disallowed fallbacks. |
| apps/content/docs/plugins/dedupe.mdx | Updates docs to reflect default dedupe for GET + QUERY. |
| apps/content/docs/plugins/cors.mdx | Updates docs to include QUERY in the CORS allowMethods example. |
| apps/content/docs/openapi/specification.mdx | Documents partial OpenAPI 3.2 support and the requirement for 3.2.0 when using QUERY. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fixes #1841
This PR adds end-to-end support for the HTTP
QUERYmethod across oRPC RPC links, OpenAPI routes, request deduplication, batching, CORS, and tests.QUERYis defined by RFC 10008 as a safe and idempotent method that carries query input in the request body. It fills the gap betweenGET, which is safe but puts input in the URL, andPOST, which supports a body but does not communicate safe or idempotent semantics.