Skip to content

feat: validate generic invoke wire contract and API arguments - #1605

Open
Qianlin1455 wants to merge 3 commits into
apache:developfrom
Qianlin1455:feat/3742-wire-validation
Open

Qianlin1455 wants to merge 3 commits into
apache:developfrom
Qianlin1455:feat/3742-wire-validation

Conversation

@Qianlin1455

@Qianlin1455 Qianlin1455 commented Oct 21, 2026 •

Copy link
Copy Markdown

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

  • Preserve exact Java long parsing with big.Rat while bounding numeric input length and exponent magnitude to prevent excessive CPU and memory consumption.
  • Resolve method signatures and parameter types from provider metadata, rejecting unknown overload signatures.
  • Validate argument counts and decode JSON arguments according to their Java parameter types before initiating RPC calls.
  • Verify the Dubbo generic invocation contract: [methodName, types, args].
  • Verify Triple non-IDL raw argument handling, including the reply argument and unary call semantics.
  • Normalize non-positive timeoutMs values to the existing 3000 ms default and propagate request attachments.
  • Return structured HTTP validation errors with code, message, field, expected, and actual.
  • Add unit tests, HTTP handler tests, wire-contract tests, and opt-in cross-language E2E tests.

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.String or String now 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

  • Regression tests for excessive positive/negative exponents, oversized numeric inputs, exact int64 boundaries, and structured error propagation. Relevant service/model tests, go vet, and go build passed locally.
go test ./pkg/console/model ./pkg/console/service ./pkg/console/handler -count=1 -timeout 120s

All three packages passed.

Cross-language E2E

Verified against a Java Dubbo 3.3.6 provider using:

  • Dubbo protocol on port 20880.
  • Triple protocol on port 50051.
  • SDK-level and Service-level generic invocation.
  • HTTP Handler → Service → Java Provider invocation.

Both protocols successfully invoked example.EchoService.echo("hello") and returned "hello".

Additional validation confirmed:

  • Default timeout configuration of 3000 ms.
  • Attachment traceId=abc received by the Java provider over both protocols (traceid on Triple due to HTTP/2 key normalization).
  • Structured errors for argument count mismatches, invalid parameter types, missing required fields, and unknown signatures.
  • Invalid arguments rejected before RPC invocation.
  • Positive and negative argument decoding boundary tests.

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.

  • Docs
  • Installation
  • User Experience
  • Dubboctl
  • Console
  • Core Component

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)

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 AlexStocks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:设计清晰,建议修一处健壮性问题后合入

这是一次质量较高的加固:把 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 0
  • go vet → exit 0
  • go test -count=1 -run 'GenericInvoke|Decode' ./pkg/console/service/ ./pkg/console/model/ → 两个包均 ok
  • changed_files=14 与实际 Diff 文件数一致;Head f1c7c4e2 的 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 的业务码得以保留。

@Qianlin1455

Copy link
Copy Markdown
Author

结论:设计清晰,建议修一处健壮性问题后合入

这是一次质量较高的加固:把 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 0
  • go vet → exit 0
  • go test -count=1 -run 'GenericInvoke|Decode' ./pkg/console/service/ ./pkg/console/model/ → 两个包均 ok
  • changed_files=14 与实际 Diff 文件数一致;Head f1c7c4e2 的 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 a9448d1 by adding numeric input length and exponent bounds before big.Rat.SetString().

I also added regression tests covering excessive positive/negative exponents, oversized numeric inputs, exact int64 boundaries, and structured error propagation. All relevant tests, go vet, and go build passed locally.

For P3, I've updated the PR description to document the stricter java.lang.String validation as an intentional compatibility change.

Thanks again for the helpful feedback!

@robocanic robocanic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(Java Long.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" 补充负向测试。

@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026

Copy link
Copy Markdown

@Qianlin1455

Copy link
Copy Markdown
Author

感谢 @robocanic 的详细 Review!
已在提交 e97770e 中修复两个问题:

  1. 错误信息丢失:HTTP 响应现在会保留参数解码的具体错误原因,同时保持原有响应结构。
  2. 整数解析校验不严格:增加十进制语法校验,拒绝非法格式及指数绕过,保留原有精度和资源限制。
    已补充回归测试,相关单元测试、Race 检测、go vet 和 go build 均通过。
    修复已推送至原 PR 分支,麻烦您方便时再次 Review。
    感谢您的指导!

@Qianlin1455

Copy link
Copy Markdown
Author

补充说明一下当前的 CI 失败情况:
经过排查,失败用例 TestObservabilityInstanceDashboardDispatch/metric 来自 develop 分支已有的 Observability 测试与实现不一致。
该问题由 PR #1587 和 #1591 的变更引入,最新 develop(0db71b07)的 CI 也出现了相同错误:

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.

4 participants