Skip to content

fix: resolve node references only for a trusted document - #77

Merged
swordqiu merged 2 commits into
yunionio:masterfrom
swordqiu:hotfix/qj-node-reference
Sep 16, 2026
Merged

swordqiu merged 2 commits into
yunionio:masterfrom
swordqiu:hotfix/qj-node-reference

Conversation

@swordqiu

Copy link
Copy Markdown
Member

背景

Marshal 对循环对象写入节点引用语法(对象内的 ___jnid_ 键 + 裸 <N> 值),使循环能够终止。解析器此前无条件识别这套语法并把它还原成对象引用。

还原一个引用会让目标结构体的两个字段指向同一个对象。循环对象的往返需要这个行为,但来自不可信来源的文档同样可以利用它 —— 构造 {"a":{"___jnid_":1,...},"c":<1>} 就能让 a 与 c 两个字段互为别名,且不产生任何错误。

合法往返与伪造文档在结构上完全同形,无法从文档本身区分,因此改为按来源区分。

变更

新增受信任入口:

func ParseTrusted(str []byte) (JSONObject, error)
func ParseTrustedString(str string) (JSONObject, error)

Parse / ParseString / ParseStream 不再把 ___jnid_ 与裸 <N> 当作引用:

  • <N> 按既有规则回落为普通字符串,反序列化到原目标字段时明确报类型错误,不会静默产生别名
  • ___jnid_ 作为普通键保留在对象里
  • 引用类型断言改为 comma-ok 形式

Marshal 不变 —— 循环对象必须有这套语法才能终止。

破坏性变更

场景 变化
循环图的文本往返(Marshal → String → Parse → Unmarshal) 需改用 ParseTrusted 系列
普通 JSON(不含 ___jnid_ / <N>) 无影响
内存内往返(Marshal(x).Unmarshal(&y)) 无影响,不经过解析器

测试

新增 node_reference_test.go:

  • TestNodeReferenceNotResolved:默认入口下 <1> 是普通字符串、___jnid_ 是普通键、两字段不互为别名
  • TestNodeReferenceDoesNotAlias:伪造引用不得产生别名
  • TestMarshalUnmarshalKeepsCycle:内存内往返仍能还原环
  • TestParseTrustedResolvesReference:受信任入口正常还原引用并消费保留键
  • TestParseTrustedRoundTrip:循环图经文本往返仍还原

marshal_test.go 的 TestMarshalLoop 有 1 行改动(ParseString → ParseTrustedString),该用例本就断言循环图的文本往返,属受信任场景。

go test ./... 全量通过。

合并提示

本 PR 与 #69 在 jsonutils.go 同样两行存在冲突(parseJSONValue 的 <N> 判断、parseDict 的 ___jnid_ 分支)。解析方式:采用本 PR 的版本,其中已包含 #69 的 len(val) > 1 边界检查与 comma-ok 断言。

Qiu Jian added 2 commits September 16, 2026 03:48
Reading a node reference back makes two fields of the target struct point
at the same object.  That is what the round trip of a cyclic object needs,
but a document from an untrusted source could use it the same way.

The parser now reads the ___jnid_ key and a bare <N> value as node
references only for a document passed to ParseTrusted.  Parse, ParseString
and ParseStream keep them as ordinary values, so a forged reference becomes
a plain string that fails to unmarshal into the field it targets.

Marshal is unchanged: it needs this syntax to terminate on a cyclic object.
Resolve the conflicts in jsonutils.go and parse_session.go.

Move the tests that exercise node references in parse_test.go,
jsonpointer_test.go and robust_test.go to the trusted entry point, which
is where references are resolved now.
@swordqiu
swordqiu merged commit 78deb19 into yunionio:master Sep 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant