fix(middleware): detect and handle gzip/zstd automatically and return 415 for invalid encodings#4936
Open
nerimoe wants to merge 1 commit into
Open
fix(middleware): detect and handle gzip/zstd automatically and return 415 for invalid encodings#4936nerimoe wants to merge 1 commit into
nerimoe wants to merge 1 commit into
Conversation
Contributor
WalkthroughAdds buffered reading and Content-Encoding normalization to DecompressRequestMiddleware, auto-detects gzip/zstd by peeking magic bytes when the header is empty/identity, adjusts decoder paths and close semantics, returns structured JSON errors for invalid encodings, and marks the compress module as a direct go.mod dependency. ChangesRequest Decompression Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
middleware/gzip.go (1)
73-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear the compressed
Content-Lengthafter swapping in a decompressed body.These branches replace
c.Request.Bodywith a decompressor, butc.Request.ContentLengthand theContent-Lengthheader still describe the compressed payload. Downstream code that trusts those values can under-read the body or forward an invalid length.💡 Suggested fix
origBody := c.Request.Body wrapMaxBytes := func(body io.ReadCloser) io.ReadCloser { return http.MaxBytesReader(c.Writer, body, maxBytes) } + clearContentLength := func() { + c.Request.ContentLength = -1 + c.Request.Header.Del("Content-Length") + } @@ case "gzip": gzipReader, err := gzip.NewReader(br) @@ c.Request.Body = wrapMaxBytes(&readCloser{ Reader: gzipReader, @@ }) + clearContentLength() c.Request.Header.Del("Content-Encoding") case "br": @@ c.Request.Body = wrapMaxBytes(&readCloser{ Reader: reader, @@ }) + clearContentLength() c.Request.Header.Del("Content-Encoding") case "zstd": @@ c.Request.Body = wrapMaxBytes(&readCloser{ Reader: decoder, @@ }) + clearContentLength() c.Request.Header.Del("Content-Encoding")🤖 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 `@middleware/gzip.go` around lines 73 - 109, After replacing c.Request.Body with a decompressor (in the gzip, br, and zstd branches using gzipReader, brotli.NewReader, and zstd.NewReader via wrapMaxBytes/readCloser), clear the compressed length so downstream consumers don't trust stale values: delete the "Content-Length" header (c.Request.Header.Del("Content-Length")) and set c.Request.ContentLength = -1 (unknown) immediately after swapping in the decompressed body in each branch.
🤖 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 `@middleware/gzip.go`:
- Around line 91-100: zstd.NewReader(br) defers validation until Read, so create
the decoder as you do (decoder := zstd.NewReader(br)) and then perform an
immediate small validation read from decoder (e.g., try reading a single byte or
call a decoder-specific validate method) and if that read returns an error close
origBody and decoder and call c.AbortWithStatusJSON with the same invalid zstd
response; ensure you also close decoder on success/failure to avoid leaks and
keep the same error message/path used in the current block (origBody.Close(),
c.AbortWithStatusJSON(...)).
- Around line 65-70: Replace the direct c.AbortWithStatusJSON(...) calls in
middleware/gzip.go (the blocks emitting JSON for "invalid gzip body" and the
similar responses at the other two locations) with manual response writing that
uses common.Marshal() to serialize the body, sets the "Content-Type" header to
"application/json", writes the appropriate HTTP status code, writes the
marshaled bytes to c.Writer, and then calls c.Abort(); if common.Marshal()
returns an error, write a 500 status (and optionally an empty body) to fail
safely. Locate the response blocks by the error JSON structure (message/type
keys) and update them to use common.Marshal() for marshaling instead of
AbortWithStatusJSON.
---
Outside diff comments:
In `@middleware/gzip.go`:
- Around line 73-109: After replacing c.Request.Body with a decompressor (in the
gzip, br, and zstd branches using gzipReader, brotli.NewReader, and
zstd.NewReader via wrapMaxBytes/readCloser), clear the compressed length so
downstream consumers don't trust stale values: delete the "Content-Length"
header (c.Request.Header.Del("Content-Length")) and set c.Request.ContentLength
= -1 (unknown) immediately after swapping in the decompressed body in each
branch.
🪄 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: 4556e208-bffc-40ec-9227-4f8ebaa31619
📒 Files selected for processing (2)
go.modmiddleware/gzip.go
… 415 for invalid encodings
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
1 participant
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.
📝 变更描述 / Description
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
Bug Fixes
Chores