fix(data): a filter the server cannot apply is rejected, not silently ignored (#4181) - #4209
Merged
Merged
Conversation
… ignored (#4181) `?filter={status:done` — one missing quote — answered 200 with the UNFILTERED page. The JSON-parse tolerance (`catch { /* keep as-is */ }`) left the raw string on `where`, a shape no driver consumes, so the filter was dropped whole and the response was byte-for-byte a successful unfiltered query. Worst failure direction in this family: #4134 returned nothing, #4164 dropped one predicate, this returned everything. This was not a design question. The sibling `GET /data/:object/export` route has rejected the same input since it was written (400 INVALID_REQUEST, "filter must be JSON") — the list path was the outlier, and the two routes of one endpoint family answered the same malformed input in opposite ways. The guard moves into the shared normalizer so `GET /data/:object`, `POST /data/:object/query` and the runtime dispatcher inherit one answer. - Unparseable JSON → 400 INVALID_REQUEST naming the parameter, and saying the filter was NOT applied — the failure mode is what makes it urgent. - Parses but is not a filter (`?filter=5` / `"done"` / `null`) → same rejection. Usable JSON is not a usable filter. - Blank `?filter=` stays ABSENT, not malformed — the same `length > 0` guard export applies. `where` is deleted rather than left as `''`. - `filter` / `filters` / `$filter` / `where` are four spellings of ONE slot; `??` silently ran the first and discarded the rest. Different values are now 400 (merging would invent an intent the caller never expressed; picking one IS the silent drop). Identical redundant spellings pass. - Export's `orderby` gets the same rule — a sort that cannot be parsed is refused, not dropped. Lower stakes (row set unchanged) but a caller taking "latest N" via orderby+top silently got an arbitrary N. Because a filter now either parses to an object or 400s, #4164's `typeof explicitWhere === 'object'` merge guard — added precisely because this tolerance could leave a raw string on `where` — is unreachable and removed with it. Its pin test flips from "keeps the garbage" to "rejects it". Verified live on the showcase app: garbage filter 400s alone and with `top`; `?filter=5|"done"|null` 400; blank filter, all four aliases, and valid JSON / object / AST-array shapes still 200; export filter+orderby 400 while a normal export streams 6080B; #4134 (`?pageSize=5`) and #4164 (merged total=1) both unregressed. `turbo run test` 132/132. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKsxtrsCwSL3uAcxAdVj83
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
…own-query-params-6syn6f # Conflicts: # packages/metadata-protocol/src/protocol.ts
Reconciles with #4121 (`fix(metadata)!: a $filter array that is not a filter AST is rejected`), which landed on this same code path while #4181 was in flight and is why this branch conflicted. The two fixes are complementary halves of one condition — #4121 catches the malformed ARRAY shapes, #4181 the non-array ways a filter fails to become one (unparseable JSON; JSON that parses to a number / bare string / null) — so they now share the standard-catalog `INVALID_FILTER` code and a matching pair of helpers (`malformedFilterArrayError` / `unusableFilterError`; the name collision between them is what git could not resolve). A caller asking "did my filter run?" must not have to know which branch caught it, and a new test pins exactly that: four differently-broken filters, one `400/INVALID_FILTER`. The export route's filter guard moves `INVALID_REQUEST` → `INVALID_FILTER` for the same reason. That code WAS pinned by a test — my earlier grep looked for the message string and missed an assertion on the code — so the change is deliberate and the test now records why. Export's `orderby` guard keeps `INVALID_REQUEST`: a sort is not a filter, and it never returned a wrong row set. That guard also gains the test it never had. Ordering in the chain is what makes them compose: blank → absent; string → JSON.parse or reject; array → AST-convert or reject (#4121); scalar → reject (#4181); everything surviving is an object, which is what lets #4164's merge trust `where`. An empty `[]` still means "no filter" and passes. #4121's 12 tests and this branch's 103 both green; turbo test 132/132. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKsxtrsCwSL3uAcxAdVj83
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…ers (#4181) The entry described only "an invalid operator or field in the where clause". After #4121 and #4181 the same code also answers a $filter array that is not a filter AST, a filter parameter that is not valid JSON, and JSON that parses to a non-filter — so a caller who hit one of those found a catalog entry that did not describe their request. Adds the sentence the whole family exists for: a filter the server cannot run is never skipped, because skipping it returns the UNFILTERED set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKsxtrsCwSL3uAcxAdVj83
os-zhuang
marked this pull request as ready for review
July 30, 2026 16:16
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4181。#4134 / #4164 家族第三员,也是方向最恶性的一个:前两案分别是清零和丢一个谓词,这个是返回全部。
问题
?filter={status:done(少一个引号)返回200+ 未过滤整页。归一化层的catch { /* keep as-is */ }把原始字符串留在where上,没有驱动消费这个形状,于是过滤器整个蒸发,响应与一次成功的无过滤查询逐字节相同——调用方无从察觉自己写的过滤器从未生效。这不是设计题:处方早就在同一个文件里
GET /data/:object/export对同一份输入从写下来那天起就拒绝(rest-server.ts)。list 路由才是异类——同一端点家族的两条路由,对同一份畸形输入给出相反答案,而且被硬化的恰好是危害较小的那条(export 一次性、结果可见),没硬化的是常被程序化消费的 list。守卫因此搬进共享归一化层,GET /data/:object、POST /data/:object/query、runtime dispatcher 三条路径继承同一个答案。改动
400 INVALID_FILTER,点名参数并明说过滤器没有生效(It was not applied, and an unapplied filter would have returned the unfiltered result set)——失败模式本身才是这个 bug 的要害,错误信息必须说出来。?filter=5/"done"/null)同样拒绝。能被 JSON 解析 ≠ 能当过滤器用;这一步也是让 REST 列表:显式filter在场时,同时传的字段级参数被静默丢弃(#4134 的邻居) #4164 的合并可以信任where是 object 的前提。?filter=仍是「不存在」而非「畸形」——沿用 export 的length > 0判断,并删掉where而不是留一个''。filter/filters/$filter/where是同一槽位的四种拼法,??取第一个非空 = 静默丢弃其余(body 同时带where和不同的filter时,跑的是filter)。值不同 →400 INVALID_REQUEST;刻意不选 AND 合并——合并会发明调用方没表达过的意图,而选一个就是静默丢弃本身。冗余的相同拼法放行。这里用INVALID_REQUEST而非INVALID_FILTER:每个值单独看都是合法过滤器,含混的是请求,不是过滤器。orderby同规则(原本catch { /* leave undefined */ })。危害低于 filter(结果集不变),但靠orderby+top取「最新 N 条」的调用方拿到的是任意 N 条。这条守卫此前没有任何测试,一并补上。顺带清掉一处刚变成死代码的守卫
#4164 当时加了
typeof explicitWhere === 'object'才允许合并,理由写在注释里:正是因为这个解析容忍可能在where上留下裸字符串。源头修掉后该分支不可达,连同注释删除;那条pinned until #4181的测试也从「保留垃圾」翻转为「拒绝垃圾」。这是当初刻意留的交接点。与 #4121 的对账
git 解不了的是一个函数重名:两边都叫
malformedFilterError,签名和错误码都不同。对账结果:["and"])malformedFilterArrayErrorunusableFilterError两者共用
INVALID_FILTER(标准目录码,#4121 引入)。调用方问的是「我的过滤器跑了吗」,不该需要知道是哪条分支拦下的——新增一条测试专门钉这点:四种不同坏法的 filter,只允许出现一个400/INVALID_FILTER。链条顺序是两者能共存的关键:空 → 视为不存在;字符串 → 解析或拒绝;数组 → AST 转换或拒绝(#4121);标量 → 拒绝(#4181);活下来的一定是 object,这正是 #4164 的合并可以信任
where的依据。空[]仍表示「无过滤器」,原样通过。export 路由的 filter 守卫也从
INVALID_REQUEST改为INVALID_FILTER与之统一。这条错误码此前是有测试钉住的——我先前用消息字符串 grep 没抓到断言 code 的那一行,是全量测试把它抓了出来;改动因此是有意的,测试里记下了理由。验证
单测:#4121 的 12 项 + 本分支 103 项全过。本分支覆盖三个别名各自的解析失败、"没有生效"的措辞、四种「能解析但不是过滤器」的值、三种合法形状(JSON 字符串 / 活对象 / FilterAST 数组)、空 filter、别名冲突/相同/单发、以及跨两案的单一错误码。真实引擎测试先跑 baseline
total=10再证明垃圾 filter 不再返回同样的 10 行——否则「拒绝了」和「过滤器生效了」在断言上无法区分。跑起来的服务器(showcase,已自行关闭;下表跑在合并 #4121 之前的提交上,错误码当时还是
INVALID_REQUEST,行为与现在一致):turbo run test132/132 全绿(合并后重跑);eslint 干净;check:route-envelope/check:error-code-casing/check:doc-authoring全 PASS。文档:
data-api.mdx增加「过滤器要么生效要么失败」小节与对照表;error-catalog.mdx的INVALID_FILTER条目此前只写「where 子句里的操作符或字段非法」,不再覆盖它现在回答的条件,已重写并补上这一族存在的理由。对调用方的影响
带畸形过滤器的请求现在大声失败,而不是收到全部记录。所有合法形状——JSON 字符串、活对象、
FilterConditionAST 数组、四种别名单独使用——不受影响。export?filter=的错误码从INVALID_REQUEST变为INVALID_FILTER(状态码与消息不变)。家族收口
三案修完后,
findData的过滤器表达路径上不再有静默分支:未知参数(#4134)、被挤掉的谓词(#4164)、无法应用的过滤器(#4181/#4121)全部要么生效要么抛错。建议后续补一条家族级 conformance 把这条不变量固化。