perf(json): 统一json调用入口约 250 处 encoding/json 调用到 common 封装 - #5607
Closed
zhangzhichaolove wants to merge 10 commits into
Closed
perf(json): 统一json调用入口约 250 处 encoding/json 调用到 common 封装#5607zhangzhichaolove wants to merge 10 commits into
zhangzhichaolove wants to merge 10 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds ChangesCentralized JSON helper migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR switches the project’s unified JSON wrapper (common/json.go) from Go’s encoding/json to sonic.ConfigStd (for byte-compatible output), and refactors call sites across the codebase to route JSON marshal/unmarshal/decoder/validity checks through common.* APIs for consistent behavior and improved performance.
Changes:
- Update
common/json.goto usesonic.ConfigStd, and addcommon.ValidJsonas a wrapper for JSON validity checks. - Add compatibility/regression tests in
common/json_test.goto lock in byte-level parity withencoding/json. - Replace scattered
encoding/jsonmarshal/unmarshal/decoder usage across settings/services/relay/controllers/models/oauth/middleware/pkg withcommon.Marshal,common.Unmarshal,common.DecodeJson, andcommon.ValidJson.
Reviewed changes
Copilot reviewed 85 out of 86 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| setting/user_usable_group.go | Use common.Marshal/Unmarshal for group JSON serialization. |
| setting/ratio_setting/group_ratio.go | Use common.Unmarshal for ratio validation parsing. |
| setting/rate_limit.go | Use common.Marshal/Unmarshal for rate-limit group serialization/parsing. |
| setting/console_setting/validation.go | Route JSON parsing through common.Unmarshal. |
| setting/config/config.go | Route reflective config JSON serialization/parsing through common.*. |
| setting/chat.go | Use common.Marshal/Unmarshal for chat config JSON. |
| service/webhook.go | Use common.Marshal for webhook payload encoding. |
| service/user_notify.go | Use common.Marshal for gotify payload encoding. |
| service/passkey/session.go | Use common.Marshal/Unmarshal for session payload storage. |
| service/midjourney.go | Use common.DecodeJson/Marshal/Unmarshal on Midjourney HTTP paths. |
| service/download.go | Use common.Marshal for worker request payload. |
| service/convert.go | Replace encoding/json marshal/unmarshal with common.* in conversions. |
| relay/mjproxy_handler.go | Use common.Marshal/Unmarshal for MJ proxy task fields/responses. |
| relay/helper/model_mapped.go | Use common.Unmarshal (aliased) for model mapping parsing. |
| relay/channel/zhipu/relay-zhipu.go | Use common.Marshal/Unmarshal for streaming and handler responses. |
| relay/channel/xunfei/relay-xunfei.go | Use common.Marshal/Unmarshal for streaming and handler responses. |
| relay/channel/volcengine/tts.go | Use common.Marshal/Unmarshal for TTS request/response handling. |
| relay/channel/volcengine/adaptor.go | Use common.Marshal/Unmarshal for audio request conversion. |
| relay/channel/vertex/service_account.go | Use common.DecodeJson for token exchange response decoding. |
| relay/channel/vertex/adaptor.go | Use common.Unmarshal for extra-body parsing. |
| relay/channel/tencent/relay-tencent.go | Use common.Marshal/Unmarshal for payload/signing and response parsing. |
| relay/channel/task/taskcommon/helpers.go | Update helper comment to reflect common.Marshal/Unmarshal pattern. |
| relay/channel/siliconflow/relay-siliconflow.go | Use common.Marshal/Unmarshal for rerank response handling. |
| relay/channel/replicate/adaptor.go | Use common.Unmarshal for output format parsing. |
| relay/channel/palm/relay-palm.go | Use common.Marshal/Unmarshal for stream and non-stream response handling. |
| relay/channel/openai/relay_image.go | Use common.ValidJson + common.Unmarshal/Marshal in image stream handling. |
| relay/channel/openai/adaptor.go | Use common.Unmarshal for THINKING payload parsing. |
| relay/channel/ollama/stream.go | Use common.Marshal/Unmarshal across streaming decode/build steps. |
| relay/channel/ollama/relay-ollama.go | Use common.Unmarshal for schema/tool-call parsing and version decoding. |
| relay/channel/mokaai/relay-mokaai.go | Use common.Unmarshal for response parsing. |
| relay/channel/minimax/tts.go | Use common.Unmarshal for TTS response parsing. |
| relay/channel/minimax/adaptor.go | Use common.Marshal/Unmarshal for request conversion. |
| relay/channel/jimeng/sign.go | Use common.Marshal for signing payload hashing. |
| relay/channel/jimeng/image.go | Use common.Marshal/Unmarshal for response conversion. |
| relay/channel/jimeng/adaptor.go | Use common.Unmarshal for extra fields parsing. |
| relay/channel/gemini/relay-gemini.go | Use common.Marshal/Unmarshal in conversion and tool-call building. |
| relay/channel/dify/relay-dify.go | Use common.DecodeJson/Marshal/Unmarshal for API/stream handling. |
| relay/channel/coze/relay-coze.go | Use common.Marshal/Unmarshal for response parsing and SSE events. |
| relay/channel/coze/adaptor.go | Use common.Unmarshal (aliased) for response parsing. |
| relay/channel/cohere/relay-cohere.go | Use common.Marshal/Unmarshal for stream and handler paths. |
| relay/channel/cloudflare/relay_cloudflare.go | Use common.Marshal/Unmarshal for stream/STT handler paths. |
| relay/channel/claude/relay-claude.go | Use common.Marshal/Unmarshal for tool-call args and response encoding. |
| relay/channel/baidu/relay-baidu.go | Use common.Marshal/Unmarshal/DecodeJson for handlers and token decode. |
| relay/channel/aws/relay-aws.go | Use common.Unmarshal for Nova response parsing. |
| relay/channel/aws/dto.go | Use common.Marshal for beta header JSON encoding. |
| relay/channel/ali/rerank.go | Use common.Marshal/Unmarshal for rerank response handling. |
| pkg/ionet/jsonutil.go | Use common.Marshal/Unmarshal for flexible-time decoding pipeline. |
| pkg/ionet/hardware.go | Use common.Unmarshal for API response parsing. |
| pkg/ionet/deployment.go | Use common.Unmarshal for deployment API response parsing. |
| pkg/ionet/container.go | Use common.Unmarshal for exec result parsing. |
| pkg/ionet/client.go | Use common.Marshal/Unmarshal for request/params and error decoding. |
| pkg/cachex/codec.go | Use common.Marshal/Unmarshal in JSON codec. |
| oauth/oidc.go | Use common.DecodeJson for token/userinfo response decoding. |
| oauth/linuxdo.go | Use common.DecodeJson for token/userinfo response decoding. |
| oauth/github.go | Use common.Marshal for token request and common.DecodeJson for responses. |
| oauth/discord.go | Use common.DecodeJson for token/userinfo response decoding. |
| model/user.go | Use common.Marshal/Unmarshal for user setting/sidebar config JSON. |
| model/pricing.go | Use common.Unmarshal for endpoints parsing. |
| model/prefill_group.go | Use common.Marshal when scanning arbitrary JSON values. |
| model/passkey.go | Use common.Marshal/Unmarshal for transports serialization. |
| model/channel.go | Use common.Marshal for other-info serialization. |
| middleware/turnstile-check.go | Use common.DecodeJson for Turnstile response decoding. |
| middleware/kling_adapter.go | Use common.Marshal for unified request payload. |
| middleware/jimeng_adapter.go | Use common.Marshal for unified request payload. |
| go.mod | Bump/require github.com/bytedance/sonic to v1.15.2 (and loader v0.5.1). |
| go.sum | Update sums for sonic + related dependency versions. |
| dto/openai_request.go | Use common.Marshal/Unmarshal for message/tool-call content helpers. |
| dto/claude.go | Use common.Unmarshal for OutputConfig parsing in helpers. |
| controller/wechat.go | Use common.DecodeJson for login response decoding. |
| controller/user.go | Use common.DecodeJson/Marshal/Unmarshal for request parsing and JSON transforms. |
| controller/uptime_kuma.go | Use common.DecodeJson for HTTP response decoding helper. |
| controller/topup_creem.go | Use common.Marshal/Unmarshal for product config and API payload/response parsing. |
| controller/task_video.go | Use common.Marshal/Unmarshal for task data parsing and response redaction. |
| controller/model_sync.go | Use common.Unmarshal for upstream cache decode fallback logic. |
| controller/model_meta.go | Use common.Marshal for endpoints serialization. |
| controller/misc.go | Use common.DecodeJson for password reset request parsing. |
| controller/midjourney.go | Use common.Marshal/Unmarshal for MJ batch updates and comparisons. |
| controller/deployment.go | Use common.Unmarshal for optional request body parsing. |
| controller/console_migrate.go | Use common.Marshal/Unmarshal for migration transforms. |
| controller/channel.go | Use common.Marshal and common.ValidJson for overrides/stream output; use common.DecodeJson. |
| controller/channel-billing.go | Use common.Unmarshal for balance API response parsing. |
| common/utils.go | Route Any2Type through common.Marshal/Unmarshal. |
| common/topup-ratio.go | Route JSON serialization through common.Marshal/Unmarshal. |
| common/str.go | Route JSON helpers through common.Marshal/Unmarshal. |
| common/json.go | Switch JSON backend to sonic.ConfigStd; add ValidJson wrapper. |
| common/json_test.go | Add byte-compatibility and round-trip tests vs encoding/json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dto/openai_request.go (1)
466-479:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle tool-call JSON errors instead of silently dropping failures.
ParseToolCallsandSetToolCallsboth ignore JSON errors, which can silently lose tool-call payloads and send incomplete upstream requests.Suggested fix
func (m *Message) ParseToolCalls() []ToolCallRequest { if m.ToolCalls == nil { return nil } var toolCalls []ToolCallRequest - if err := common.Unmarshal(m.ToolCalls, &toolCalls); err == nil { - return toolCalls - } - return toolCalls + if err := common.Unmarshal(m.ToolCalls, &toolCalls); err != nil { + common.SysLog("failed to unmarshal tool calls: " + err.Error()) + return nil + } + return toolCalls } func (m *Message) SetToolCalls(toolCalls any) { - toolCallsJson, _ := common.Marshal(toolCalls) - m.ToolCalls = toolCallsJson + toolCallsJSON, err := common.Marshal(toolCalls) + if err != nil { + common.SysLog("failed to marshal tool calls: " + err.Error()) + return + } + m.ToolCalls = toolCallsJSON }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dto/openai_request.go` around lines 466 - 479, The ParseToolCalls method silently returns an empty slice when common.Unmarshal fails, and the SetToolCalls method ignores errors from common.Marshal entirely. Instead of discarding these errors, implement proper error handling in both methods by capturing the errors returned from common.Unmarshal and common.Marshal (remove the blank identifier assignment in SetToolCalls) and either log them using an appropriate logger or modify the method signatures to return errors so callers can handle them appropriately. This ensures JSON parsing/marshaling failures are visible rather than silently losing tool-call payloads.relay/channel/minimax/tts.go (1)
110-118:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose
resp.Bodyon every return path.
defer resp.Body.Close()is registered afterio.ReadAll. If read fails, the function returns without closing the body, which can leak connections/file descriptors.Suggested fix
func handleTTSResponse(c *gin.Context, resp *http.Response, info *relaycommon.RelayInfo) (usage any, err *types.NewAPIError) { + defer resp.Body.Close() body, readErr := io.ReadAll(resp.Body) if readErr != nil { return nil, types.NewErrorWithStatusCode( fmt.Errorf("failed to read minimax response: %w", readErr), types.ErrorCodeReadResponseBodyFailed, http.StatusInternalServerError, ) } - defer resp.Body.Close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/minimax/tts.go` around lines 110 - 118, Move the `defer resp.Body.Close()` statement to appear before the `io.ReadAll(resp.Body)` call in the response body handling block. Currently, if the ReadAll operation fails, the function returns before the defer statement is registered, leaving the response body unclosed and causing resource leaks. By registering the defer before attempting to read the body, it will execute on all return paths regardless of whether the read succeeds or fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/misc.go`:
- Around line 340-341: The error returned from common.DecodeJson on line 340 is
not being checked, allowing malformed JSON to proceed to the field validation
checks. Add an error check immediately after the
common.DecodeJson(c.Request.Body, &req) call to return an early error response
if the decode operation fails, ensuring that only valid JSON data reaches the
subsequent validation logic that checks req.Email and req.Token.
In `@relay/channel/ollama/relay-ollama.go`:
- Around line 34-35: The code is discarding errors from the `common.Unmarshal`
call by using the blank identifier, which silently ignores JSON parsing failures
for the schema and tool arguments. Instead of discarding the error, capture the
error return value from `common.Unmarshal`, check if an error was returned, and
return an appropriate client-facing validation error to indicate that the schema
or tool arguments could not be parsed. This fix needs to be applied at all
occurrences where `common.Unmarshal` is called for JSON schema and tool-call
argument parsing.
In `@relay/channel/volcengine/adaptor.go`:
- Around line 89-93: The issue is that client-provided request.Metadata is being
unmarshalled directly into volcRequest, which already contains server-controlled
authentication fields (AppID and Token). This allows a malicious client to craft
metadata that overwrites these credentials. To fix this, create a separate
temporary struct or map to unmarshal request.Metadata into instead of directly
into volcRequest, then selectively copy only safe, non-sensitive fields from the
unmarshalled data into volcRequest while preserving the server-controlled AppID
and Token fields that were set before this block executes.
In `@setting/rate_limit.go`:
- Line 34: The code takes a read lock (RLock) but attempts to mutate the shared
ModelRequestRateLimitGroup state, which requires a write lock and creates a race
condition. Additionally, clearing the map before validating successful parsing
can lose the previous valid configuration if the Unmarshal operation fails.
Replace the RLock with a write Lock, and restructure the logic to ensure the
ModelRequestRateLimitGroup is only cleared and populated after the Unmarshal
operation in the common.Unmarshal call succeeds, making the entire update atomic
and safe.
In `@setting/user_usable_group.go`:
- Line 42: The userUsableGroups variable is being reset before the
common.Unmarshal call at line 42, which means if the decode fails, the valid
in-memory state is lost despite returning an error. Instead of clearing
userUsableGroups before the unmarshal operation, create a temporary variable to
hold the decoded data, perform the common.Unmarshal on the temporary variable,
and only update userUsableGroups after confirming the decode succeeded with no
error.
---
Outside diff comments:
In `@dto/openai_request.go`:
- Around line 466-479: The ParseToolCalls method silently returns an empty slice
when common.Unmarshal fails, and the SetToolCalls method ignores errors from
common.Marshal entirely. Instead of discarding these errors, implement proper
error handling in both methods by capturing the errors returned from
common.Unmarshal and common.Marshal (remove the blank identifier assignment in
SetToolCalls) and either log them using an appropriate logger or modify the
method signatures to return errors so callers can handle them appropriately.
This ensures JSON parsing/marshaling failures are visible rather than silently
losing tool-call payloads.
In `@relay/channel/minimax/tts.go`:
- Around line 110-118: Move the `defer resp.Body.Close()` statement to appear
before the `io.ReadAll(resp.Body)` call in the response body handling block.
Currently, if the ReadAll operation fails, the function returns before the defer
statement is registered, leaving the response body unclosed and causing resource
leaks. By registering the defer before attempting to read the body, it will
execute on all return paths regardless of whether the read succeeds or fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e72efe7-4f12-4ba4-b1ff-85f9dfc36267
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (85)
common/json.gocommon/json_test.gocommon/str.gocommon/topup-ratio.gocommon/utils.gocontroller/channel-billing.gocontroller/channel.gocontroller/console_migrate.gocontroller/deployment.gocontroller/midjourney.gocontroller/misc.gocontroller/model_meta.gocontroller/model_sync.gocontroller/task_video.gocontroller/topup_creem.gocontroller/uptime_kuma.gocontroller/user.gocontroller/wechat.godto/claude.godto/openai_request.gogo.modmiddleware/jimeng_adapter.gomiddleware/kling_adapter.gomiddleware/turnstile-check.gomodel/channel.gomodel/passkey.gomodel/prefill_group.gomodel/pricing.gomodel/user.gooauth/discord.gooauth/github.gooauth/linuxdo.gooauth/oidc.gopkg/cachex/codec.gopkg/ionet/client.gopkg/ionet/container.gopkg/ionet/deployment.gopkg/ionet/hardware.gopkg/ionet/jsonutil.gorelay/channel/ali/rerank.gorelay/channel/aws/dto.gorelay/channel/aws/relay-aws.gorelay/channel/baidu/relay-baidu.gorelay/channel/claude/relay-claude.gorelay/channel/cloudflare/relay_cloudflare.gorelay/channel/cohere/relay-cohere.gorelay/channel/coze/adaptor.gorelay/channel/coze/relay-coze.gorelay/channel/dify/relay-dify.gorelay/channel/gemini/relay-gemini.gorelay/channel/jimeng/adaptor.gorelay/channel/jimeng/image.gorelay/channel/jimeng/sign.gorelay/channel/minimax/adaptor.gorelay/channel/minimax/tts.gorelay/channel/mokaai/relay-mokaai.gorelay/channel/ollama/relay-ollama.gorelay/channel/ollama/stream.gorelay/channel/openai/adaptor.gorelay/channel/openai/relay_image.gorelay/channel/palm/relay-palm.gorelay/channel/replicate/adaptor.gorelay/channel/siliconflow/relay-siliconflow.gorelay/channel/task/taskcommon/helpers.gorelay/channel/tencent/relay-tencent.gorelay/channel/vertex/adaptor.gorelay/channel/vertex/service_account.gorelay/channel/volcengine/adaptor.gorelay/channel/volcengine/tts.gorelay/channel/xunfei/relay-xunfei.gorelay/channel/zhipu/relay-zhipu.gorelay/helper/model_mapped.gorelay/mjproxy_handler.goservice/convert.goservice/download.goservice/midjourney.goservice/passkey/session.goservice/user_notify.goservice/webhook.gosetting/chat.gosetting/config/config.gosetting/console_setting/validation.gosetting/rate_limit.gosetting/ratio_setting/group_ratio.gosetting/user_usable_group.go
zhangzhichaolove
changed the title
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.
Important
📝 变更描述 / Description
把项目里散落的、绕过封装直接调用
encoding/json的地方统一收口到common.*。为什么这样改：
common/json.go设计成统一的 JSON 出入口（注释里写明"为将来替换更快的 JSON 库预留"）。import "encoding/json"（约 250 处调用），绕过了封装层,所以应该统一入口，方便后续扩展。主要改动：
common/json_test.go：新增与标准库逐字节比对的兼容性测试，覆盖 map key 排序、HTML 转义、RawMessage、json.Number、自定义MarshalJSON/UnmarshalJSON，作为防回归基线。encoding/json→common.*（relay/、controller/、oauth/、middleware/、service/、setting/、model/、pkg/、dto/）。relay/common（其包名也叫common）的文件，顶层 common 改用commonpkg别名引用，避免标识符冲突。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。（本 PR 非 Bug fix）📸 运行证明 / Proof of Work
全量编译通过：
Summary by CodeRabbit