Repository navigation
feat: validate generic invoke wire contract and API arguments - #1605
Qianlin1455 wants to merge 3 commits into
Conversation
Align generic invoke wire arguments and validate API inputs against provider metadata. Refs: apache/dubbo-go#3742 (module 2) Signed-off-by: zb1455498510 <1455498510@qq.com>
AlexStocks
left a comment
There was a problem hiding this comment.
结论:设计清晰,建议修一处健壮性问题后合入
这是一次质量较高的加固:把 Validate() 的错误结构化(Field/Expected/Actual),把参数解码错误细分为 type / count / signature,并用 big.Rat 替换 float64 中转来消除 long 解析的精度问题。HTTP 层仍沿用仓库既有的 200 + BizError 约定(与 util.HandleServiceError 一致),兼容性没有被破坏。
已验证
go build ./pkg/console/handler/ ./pkg/console/service/ ./pkg/console/model/→ exit 0go vet→ exit 0go test -count=1 -run 'GenericInvoke|Decode' ./pkg/console/service/ ./pkg/console/model/→ 两个包均okchanged_files=14与实际 Diff 文件数一致;Headf1c7c4e2的 9 项检查全部 success
P2 · big.Rat 让超大指数成为内存/CPU 放大点
parseInt64Text 现在对含 ./e/E 的输入直接交给 new(big.Rat).SetString(text),而 text 来自请求体、没有长度上限。实测(Go 1.27):
1e1000 ok=true 0.0ms 0.0MB
1e100000 ok=true 0.6ms 0.4MB
1e1000000 ok=true 16.6ms 3.1MB
开销随指数线性增长。一个十几字节的 JSON 数字(如 1e999999999)就能让单个请求占用数百 MB 并阻塞数百毫秒;并发请求下这是可利用的资源耗尽点。旧实现走 strconv.ParseFloat 会快速返回 +Inf 与 ErrRange,不存在这个问题。
建议在进入 big.Rat 之前加一个廉价上界,例如:
if len(text) > 32 { // int64 的十进制最多 20 字符,留出符号与指数余量
return 0, fmt.Errorf("invalid integer value %q", text)
}或在 SetString 前拒绝 |exp| > 19 的指数形式。
P3 · java.lang.String 从宽松变为严格
新增的 case "java.lang.String", "String" 使用 json.Unmarshal(raw, &value),要求 JSON 字符串带引号。此前该类型走 decodeJSONValue,[123] 这类未加引号的数字会被接受。改成严格校验本身合理,但属于可观察的行为变化,建议在 PR 描述或变更记录中写明。
未发现问题的项
null只对原始类型(byte/short/int/long/float/double/boolean/char)拒绝,保留装箱类型的null语义,正确。genericJSONType先json.Valid再取raw[0],空输入不会越界。float32的math.Abs(value) > math.MaxFloat32能拦住转成+Inf的输入。fmt.Errorf("%w: %w", bizErr, fieldErr)(Go 1.20+ 多%w)与writeGenericInvokeError里的两层errors.As配套正确,NotFoundError的业务码得以保留。
Thanks for the detailed review, @AlexStocks! I've addressed the P2 resource exhaustion issue in commit I also added regression tests covering excessive positive/negative exponents, oversized numeric inputs, exact For P3, I've updated the PR description to document the stricter Thanks again for the helpful feedback! |
robocanic
left a comment
There was a problem hiding this comment.
Review 摘要
整体方向没有问题:参数类型取自 provider 元数据、调用前先做类型/数量校验、错误结构化。我在 PR head(a9448d14)上运行了 go test ./pkg/console/model ./pkg/console/service ./pkg/console/handler,全部通过,并针对下面两点用临时用例做了验证。发现两个值得处理的问题:
1. 类型错误的具体原因没有返回给调用方(pkg/console/handler/service_generic_invoke_error.go:53-54)
ArgumentErrorType 分支把 message 固定为 "invalid argument type",而真正的失败原因只保存在 GenericInvokeArgumentError.Cause(pkg/console/service/service_generic_invoke_decode.go:45)中,handler 并没有输出。Actual 也只是 JSON 类型(number / string / array),不包含取值信息。因此"类型正确但取值不合法"的场景会给出误导性的响应,例如 byte 参数传 300:
{"code":"InvalidArgument","message":"invalid argument type","data":null,"field":"args[0]","expected":"byte","actual":"number"}
调用方看到 expected=byte, actual=number 会以为类型是对的,却不知道是 value 300 out of range。同样地,"null is not allowed for primitive type int"、int[] 中第几个元素非法(invalid array element at index 1)、float32 越界等原因都会丢失。而改动前 InvokeServiceGeneric 返回的是带 invalid argument at index %d for type %s: <cause> 的完整信息,属于可调试性回退。建议把 Cause 放进 message(或新增 reason 字段)返回,并补一个对应的 handler 测试(范围越界 / primitive 传 null / 数组元素非法)。
2. parseInt64Text 使用 big.Rat.SetString,接受了非 JSON 数字语法,且指数上限可被绕过(pkg/console/service/service_generic_invoke_decode.go:295-303)
long 参数既可以是 JSON number,也可以是 JSON string;当字符串里含 ./e/E 时会走 big.Rat.SetString,它按 Go 字面量规则解析。实测:
"0x1.8p1"被解析为3,"1_0.0"被解析为10(JavaLong.parseLong和 JSON 都不接受这些写法);- 指数检查只查找第一个
e/E,hex 浮点的p指数完全不受maxExponent = 64限制:"0x1.0p9999999"会让big.Rat做 ~1e7 bit 的移位(实测每次约 0.3 ms 并分配约 1.25 MB),与 PR 描述中"限制数值长度和指数大小以避免过度消耗 CPU/内存"的目标不一致。
建议在进入 big.Rat 之前先用十进制语法校验,例如 ^[+-]?\d+(\.\d+)?([eE][+-]?\d+)?$(不匹配则直接返回错误),这样既保持 1E3、9223372036854775807.0 等当前支持的写法,也能让指数检查覆盖所有进入 big.Rat 的输入;并为 "0x1.8p1"、"1_0.0"、"0x1.0p9999999" 补充负向测试。
|
|
感谢 @robocanic 的详细 Review!
|
|
补充说明一下当前的 CI 失败情况:
|



Please provide a description of this PR:
Summary
Implements Module 2: Generic Invoke Wire Contract and API Validation of apache/dubbo-go#3742.
This PR aligns the Dubbo Admin generic invocation request validation with the Dubbo-Go wire contract and provides structured API errors for invalid arguments.
Changes
longparsing withbig.Ratwhile bounding numeric input length and exponent magnitude to prevent excessive CPU and memory consumption.[methodName, types, args].timeoutMsvalues to the existing 3000 ms default and propagate request attachments.code,message,field,expected, andactual.Parameter types are derived from trusted provider metadata rather than supplied independently by the HTTP client.
Compatibility Note
Generic invocation arguments declared as
java.lang.StringorStringnow require JSON string values. Previously accepted non-string JSON values are rejected before RPC invocation. This is an intentional change to enforce type validation against provider metadata.Verification
Unit and handler tests
int64boundaries, and structured error propagation. Relevant service/model tests,go vet, andgo buildpassed locally.go test ./pkg/console/model ./pkg/console/service ./pkg/console/handler -count=1 -timeout 120sAll three packages passed.
Cross-language E2E
Verified against a Java Dubbo 3.3.6 provider using:
Both protocols successfully invoked
example.EchoService.echo("hello")and returned"hello".Additional validation confirmed:
traceId=abcreceived by the Java provider over both protocols (traceidon Triple due to HTTP/2 key normalization).Scope
This PR focuses on generic invocation wire contracts and request validation. It does not introduce streaming support or change the RPC retry policy.
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.
Includes additional unit, integration, and opt-in cross-language E2E coverage.
Refs: apache/dubbo-go#3742 (Module 2 only)