Skip to content

fix(mcp-proxy): preserve large integer params without scientific notation - #2773

Merged
Han-Ya-Jun merged 3 commits into
TencentBlueKing:release/1.21from
Han-Ya-Jun:fix-mcpserver-path-paranm
May 27, 2026
Merged

fix(mcp-proxy): preserve large integer params without scientific notation#2773
Han-Ya-Jun merged 3 commits into
TencentBlueKing:release/1.21from
Han-Ya-Jun:fix-mcpserver-path-paranm

Conversation

@Han-Ya-Jun

Copy link
Copy Markdown
Member

Summary

  • Fix path/query/header parameters being formatted in scientific notation (e.g., 2.005000002e+09 instead of 2005000002) when large integers are passed via MCP tool calls.
  • Introduce StringParamMap custom type with UnmarshalJSON that converts all parameter values to strings at decode time using json.Number for precision.
  • Add unit tests for StringParamMap, stringifyRequestParamValue, and decodeHandlerRequest.
  • Add integration test with path parameter endpoint to verify large integers are preserved in URLs.

Test plan

  • Unit tests pass (go test -mod=mod ./pkg/infra/proxy/)
  • Integration tests pass with docker-compose environment
  • Verify large integer path params (e.g., bk_biz_id=2005000002) appear correctly in request URLs

Made with Cursor

…tion

Use custom StringParamMap type with UnmarshalJSON to convert all
header/query/path parameter values to strings at decode time, preventing
Go's default float64 formatting from producing scientific notation for
large integers like 2005000002.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Han-Ya-Jun Han-Ya-Jun closed this May 26, 2026
@Han-Ya-Jun Han-Ya-Jun reopened this May 26, 2026
@Han-Ya-Jun
Han-Ya-Jun changed the base branch from master to release/1.21 May 26, 2026 06:47
@Han-Ya-Jun Han-Ya-Jun closed this May 26, 2026
@Han-Ya-Jun Han-Ya-Jun reopened this May 26, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@Han-Ya-Jun
Han-Ya-Jun requested review from cszmzh and wklken May 26, 2026 07:01
wklken

This comment was marked as outdated.

@wklken wklken left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR #2773 Code Review 汇总报告

由 codex-internal (gpt-5.4) + Claude Opus 4.7 双模型 review,主 agent 汇总整理。

变更概述

背景: 当 MCP 工具调用传入大整数参数(如 bk_biz_id=2005000002)时,Go 的 json.Unmarshal 默认将数字解码为 float64,再通过 fmt.Sprintf("%v", v) 转为字符串时会变成科学计数法 2.005000002e+09,导致下游 HTTP 请求 URL 被破坏。

解决方案:

  1. 引入 StringParamMap 自定义类型(map[string]string),通过自定义 UnmarshalJSON 使用 json.Decoder.UseNumber() 保持数值精度,并在解码时统一将值转为字符串
  2. 提取 stringifyRequestParamValue 工具函数,处理 json.Numberfloat64float32stringbool 五种类型的转换
  3. HandlerRequestHeaderParam/QueryParam/PathParam 字段类型从 map[string]any 改为 StringParamMap
  4. 提取 decodeHandlerRequest 函数,统一处理请求解码
  5. 移除调用处 fmt.Sprintf("%v", v) 中间层
  6. 新增充分的单元测试和集成测试

变更规模: 502 行 diff,6 个文件(+340 / -31)


问题列表

🔴 Blocking

无。

🟠 Major

1. decodeHandlerRequestUseNumber()BodyParam 的向后兼容性影响 [codex][claude]

HandlerRequest.BodyParam 仍为 any 类型。当 json.Decoder.UseNumber() 启用时,JSON 数字会被解码为 json.Number 而非 float64,导致:

  • 调用方如果期望 float64(原有行为),会收到 json.Number,需要额外处理
  • 测试代码中需做 body["bk_biz_id"].(json.Number) 类型断言,与原有 float64 不兼容

建议: 采用分段解析方案——header_param/query_param/path_paramUseNumber() 反序列化,body_param 用标准 json.Unmarshal(不启用 UseNumber())单独处理,保持 BodyParamfloat64 行为不变。

func decodeHandlerRequest(arguments any) (HandlerRequest, error) {
    argsBytes, err := json.Marshal(arguments)
    if err != nil {
        return HandlerRequest{}, err
    }

    var rawMap map[string]json.RawMessage
    if err := json.Unmarshal(argsBytes, &rawMap); err != nil {
        return HandlerRequest{}, err
    }

    var handlerRequest HandlerRequest

    // Header/Query/Path 用 UseNumber 反序列化
    if raw, ok := rawMap["header_param"]; ok {
        if err := json.Unmarshal(raw, &handlerRequest.HeaderParam); err != nil {
            return HandlerRequest{}, err
        }
    }
    // ... 类似处理 query_param, path_param

    // BodyParam 用标准 Unmarshal(不启用 UseNumber,保持 float64)
    if raw, ok := rawMap["body_param"]; ok {
        var body any
        if err := json.Unmarshal(raw, &body); err != nil {
            return HandlerRequest{}, err
        }
        handlerRequest.BodyParam = body
    }

    return handlerRequest, nil
}

2. stringifyRequestParamValuefloat64/float32 分支实际可达性存疑 [codex][claude]

由于 StringParamMap.UnmarshalJSON 中已使用 UseNumber()json.Number 分支会优先命中,float64/float32 分支在实际 JSON 反序列化链路中不可达。但 stringifyRequestParamValue 暴露为公共函数,若从其他路径(非 JSON 反序列化)传入 float64,仍存在精度陷阱(value == math.Trunc(value) 对超过 2^53 的整数仍为 true,但 int64(value) 会精度丢失)。

建议: 评估是否真的需要覆盖 float64/float32 路径,若不需要则移除未使用逻辑;若保留,需增加详细注释说明函数作用和限制。

🟡 Minor

3. decodeHandlerRequest 的 Marshal + Decode 双重序列化开销 [codex][claude]

当前实现:arguments (map[string]any)json.Marshalbytes.NewReaderjson.NewDecoderDecode,经历了两次完整 JSON 序列化/反序列化。对于高频 MCP 调用路径有一定性能开销。

建议: 采用上述分段解析方案可同时解决此问题(仅需一次 Marshal),或至少添加注释说明当前设计的权衡。

4. maxExactFloatInteger 命名不够精确 [codex][claude]

该常量表示 IEEE 754 双精度浮点数能精确表示的最大连续整数(2^53-1 = 9007199254740991),命名容易让人误解为"最大可精确表示的整数"。

建议: 改名为 maxConsecutiveExactFloat64Integer 或增加详细注释说明其 IEEE 754 语义及用法限制。

5. HandlerRequest 结构体缺少文档注释 [codex]

字段类型做了重大变更(从 map[string]any 变为 StringParamMap),但文档注释未更新,缺少关于 BodyParam 数值类型变化的说明。

建议: 补充结构体注释,说明 Header/Query/Path 参数的字符串转换策略,以及 BodyParam 的数值类型行为。

6. 集成测试 mock 服务依赖未说明 [claude]

streamable_http_test.go 中的集成测试依赖 mock API 的 URL echo 行为来验证路径参数。如果 mock 服务行为变更,测试会直接失败。

建议: 在注释中说明 mock 服务的依赖关系,便于后续维护。

💬 Nit

7. StringParamMap.UnmarshalJSONstring(data) 转换 [codex]

每次反序列化都会做一次 []bytestring 的转换检查 null。虽然是微优化级别的代价,但 Go 标准库常用模式是 bytes.Equal(data, []byte("null")) 来避免类型转换。

建议: 保持现状(可读性优于微优化),或可改为 bytes.Equal

8. stringifyRequestParamValuedefault 分支缺少日志 [claude]

default 分支(return fmt.Sprintf("%v", value))对意外类型静默转换为字符串,若传入非常规类型([]intnilstruct),无调试信息。

建议: 在 default 分支加一条 debug 级别日志。

9. 测试中 decodeHandlerRequest case 的 body_param 断言方式与实现细节耦合较紧 [codex]

测试依赖 json.Number 的内部行为,若未来改变 BodyParam 的处理方式,测试也需要跟着改。

建议: 考虑用 BeEquivalentTo 或减少与实现细节的耦合。


优点

  1. 问题定位准确,根因分析到位 — 科学计数法破坏 URL 参数是 Go json.Unmarshal 的经典陷阱,PR 精准定位并修复了它。

  2. 解决方案优雅StringParamMap 自定义 UnmarshalJSON + json.Decoder.UseNumber() 的组合是处理此类问题的标准解法,方案选择恰当。

  3. 类型安全提升 — 从 map[string]any 改为 StringParamMapmap[string]string),后续使用 HeaderParam/QueryParam/PathParam 时不再需要做类型断言,提升了代码健壮性和可读性。

  4. 消除了大量 fmt.Sprintf("%v", v) 调用 — 原来每个参数设置处都做了运行时类型转换,现在统一在 StringParamMap.UnmarshalJSON 中处理,消除了重复代码和维护成本。

  5. 测试覆盖充分stringifyRequestParamValue 单元测试覆盖了 float64json.Number、string、bool 等类型;StringParamMap.UnmarshalJSON 覆盖了大整数、小数、字符串、布尔、null、零/负数、混合类型等 7 个场景;decodeHandlerRequest 覆盖了所有参数组(header/query/path/body)的组合场景;集成测试端到端验证 path/query 参数在大整数下的行为。

  6. 重构有度decodeHandlerRequest 的提取虽然增加了序列化开销,但消除了重复的 marshal 代码和错误处理,提升了代码的局部性和可测试性。


综合建议

必须处理

# 问题 建议
1 BodyParamUseNumber() 模式下行为不兼容,向后兼容风险 采用分段解析方案,将 BodyParam 的处理与 Header/Query/Path 隔离,保持 BodyParamfloat64 行为不变

建议处理

# 问题 建议
2 stringifyRequestParamValuefloat64/float32 分支实际不可达或存在精度陷阱 评估必要性,不需要则移除;保留则加详细注释
3 Marshal + Decode 双重序列化开销 分段解析方案可同时解决此问题
4 maxExactFloatInteger 命名不够精确 改名或增加详细注释
5 HandlerRequest 缺少文档注释 补充字段说明和类型策略

总体评估

该 PR 解决的问题是真实且重要的 — 科学计数法破坏 URL 参数是 Go JSON 解码的经典陷阱。解决方案(StringParamMap + UseNumber())方向正确,类型系统的提升(anystring)是净收益,测试覆盖也足够充分。

核心待解决的设计矛盾decodeHandlerRequestUseNumber()BodyParam 的影响。建议采用分段解析方案,将 BodyParam 的处理与 Header/Query/Path 隔离开来,保持向后兼容。

其余问题均为 minor/nit 级别,不影响合入,但建议在后续迭代中完善。


由 codex-internal (gpt-5.4) + Claude Opus 4.7 双模型 review,主 agent 汇总 | [from openclaw-internal]

- Remove UseNumber() from decodeHandlerRequest so BodyParam retains
  standard json.Unmarshal behavior (float64), avoiding backward
  compatibility issues.
- StringParamMap.UnmarshalJSON still uses UseNumber() internally for
  header/query/path params to preserve large integer precision.
- Rename maxExactFloatInteger to maxSafeFloat64Integer with IEEE 754 docs.
- Add comprehensive comments to HandlerRequest and stringifyRequestParamValue.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Han-Ya-Jun
Han-Ya-Jun merged commit e084c0e into TencentBlueKing:release/1.21 May 27, 2026
5 checks 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.

3 participants