fix: avoid get model consuming body - #1994
Conversation
WalkthroughPOST /v1/videos now parses multipart form data using common.ParseMultipartFormReusable, handles parsing errors with a Chinese message, defers cleanup via RemoveAll, and reads the model from form.Value["model"] into modelRequest.Model before proceeding with existing relay_mode logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant M as Middleware (distributor)
participant P as common.ParseMultipartFormReusable
participant H as Downstream handler
C->>M: POST /v1/videos (multipart)
M->>P: ParseMultipartFormReusable(req)
alt Parse fails
P-->>M: error
M-->>C: 4xx/5xx with Chinese error message
else Parse succeeds
P-->>M: form
note right of M: defer form.RemoveAll()
M->>M: model = form.Value["model"][0] (if present)
M->>H: Continue with relay_mode logic
H-->>C: Response
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
middleware/distributor.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
middleware/distributor.go (1)
common/gin.go (1)
ParseMultipartFormReusable(118-139)
🔇 Additional comments (1)
middleware/distributor.go (1)
177-186: Confirm downstream multipart reuse
Downstream handlers in relay/common/relay_utils.go, relay/helper/valid_request.go, and various channel adaptors invokec.MultipartForm()orc.Request.ParseMultipartForm; sinceParseMultipartFormReusablesetsc.Request.MultipartFormand resets the body, each downstream call will reuse the parsed form. Verify via integration tests or manual testing that the/v1/videossubmit flow functions as intended.
| form, err := common.ParseMultipartFormReusable(c) | ||
| if err != nil { | ||
| return nil, false, errors.New("无效的video请求, " + err.Error()) | ||
| } | ||
| defer form.RemoveAll() | ||
| if form != nil { | ||
| if values, ok := form.Value["model"]; ok && len(values) > 0 { | ||
| modelRequest.Model = values[0] | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move defer inside nil check for consistency and safety.
The code registers defer form.RemoveAll() on line 181 before checking if form is nil on line 182. While ParseMultipartFormReusable should return a valid form when err == nil, the defensive nil check on line 182 suggests uncertainty. If form were somehow nil at line 181, the deferred RemoveAll() would panic on function return.
Apply this diff to move the defer inside the nil check:
form, err := common.ParseMultipartFormReusable(c)
if err != nil {
return nil, false, errors.New("无效的video请求, " + err.Error())
}
-defer form.RemoveAll()
if form != nil {
+ defer form.RemoveAll()
if values, ok := form.Value["model"]; ok && len(values) > 0 {
modelRequest.Model = values[0]
}
}Alternatively, if you're confident form is never nil when err == nil, remove the nil check entirely:
form, err := common.ParseMultipartFormReusable(c)
if err != nil {
return nil, false, errors.New("无效的video请求, " + err.Error())
}
defer form.RemoveAll()
-if form != nil {
- if values, ok := form.Value["model"]; ok && len(values) > 0 {
- modelRequest.Model = values[0]
- }
+if values, ok := form.Value["model"]; ok && len(values) > 0 {
+ modelRequest.Model = values[0]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| form, err := common.ParseMultipartFormReusable(c) | |
| if err != nil { | |
| return nil, false, errors.New("无效的video请求, " + err.Error()) | |
| } | |
| defer form.RemoveAll() | |
| if form != nil { | |
| if values, ok := form.Value["model"]; ok && len(values) > 0 { | |
| modelRequest.Model = values[0] | |
| } | |
| } | |
| form, err := common.ParseMultipartFormReusable(c) | |
| if err != nil { | |
| return nil, false, errors.New("无效的video请求, " + err.Error()) | |
| } | |
| if form != nil { | |
| defer form.RemoveAll() | |
| if values, ok := form.Value["model"]; ok && len(values) > 0 { | |
| modelRequest.Model = values[0] | |
| } | |
| } |
🤖 Prompt for AI Agents
In middleware/distributor.go around lines 177 to 186, the code defers
form.RemoveAll() before checking if form is nil which can panic if form is
unexpectedly nil; move the defer inside the subsequent if form != nil { ... }
block (i.e., first check form != nil, then call defer form.RemoveAll() and
proceed to read form.Value["model"]) — alternatively, if you guarantee
ParseMultipartFormReusable never returns nil when err == nil, simply remove the
nil check and keep the defer where it is.
fix: avoid get model consuming body
2 participants
修复postForm导致form无法被重复获取的问题
Summary by CodeRabbit