chore: simplify, modernize (Go 1.26), update deps#139
Conversation
- Move Content-Type Set before the write loop (was after Write, so headers were never sent) - Re-slice buf to full cap each iteration so multi-chunk reads work correctly - Capture Header().Get(xSendHeader) once; eliminate duplicated header-copy loop - Replace os.OpenFile(O_RDONLY,0) with os.Open; simplify defer f.Close() - Use min() for buffer allocation; clear(w.hdrToSend); w.data[:0] in putWriter - Restructure EOF handling to remove goto; shadow-free Flusher variable (fl) - Harden path-traversal check with filepath.Clean - Update all dependencies
|
Warning Review limit reached
More reviews will be available in 25 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ 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 modernizes the project to Go 1.26, simplifies the X-Sendfile middleware logic, and refreshes module dependencies. It primarily focuses on correctness/performance improvements in plugin.go’s sendfile path and some cleanup in pooled writer reuse.
Changes:
- Refactored X-Sendfile handling to avoid duplicated header-copy logic and capture the sendfile path once.
- Adjusted sendfile streaming to set headers before writing and improved buffer/write flow.
- Updated indirect dependencies (
go.sum) aftergo get -u all && go mod tidy.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugin.go | Refactors X-Sendfile flow, adjusts header handling timing, and updates the file streaming loop + writer pooling cleanup. |
| go.sum | Dependency checksum updates from module upgrades/tidy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Deduplicate the two identical worker-header copy loops into a single helper, which also removes the nested for-loops from the path=="" branch and the need to silence the nestif linter.
…inal chunk - filepath.Clean result is now stored in cleanPath and used for both the traversal check and all subsequent Stat/Open calls, so the validated path is always the one actually accessed. - Empty files (size == 0) are now returned immediately with 200 OK; previously ReadAt on a zero-length buffer returned (0, nil) and the loop never advanced, spinning forever. - On io.EOF with a final non-empty chunk, a write error now returns early (consistent with the mid-stream path) and the flusher is called after the last successful write.
Applied fixes
R/High
plugin.go:201—Header().Set(Content-Type)was called afterw.Write, so the header was never sent. Moved before the write loop.R/Med
plugin.go:186—buf = buf[:n]permanently shrunk the loop buffer on each iteration, degrading multi-chunk reads of large files. Addedbuf = buf[:cap(buf)]beforeReadAteach iteration.R/Low
plugin.go:136— Hardened path-traversal check:strings.Contains(path, "..")had false positives/negatives; replaced withstrings.Contains(filepath.Clean(path), "..").S/Med
plugin.go:99—Header().Get(xSendHeader)was called twice (lines 99 and 121). Captured once at the top of the decision block.S/Low
plugin.go:99— Duplicated nested header-copy loop in both branches; deduplicated (range over value, not index).S/Low
plugin.go:148—os.OpenFile(path, os.O_RDONLY, 0)→os.Open(path).S/Low
plugin.go:154—defer func() { _ = f.Close() }()→defer f.Close().S/Low
plugin.go:170— Removedgoto out; restructured EOF path into a straightforward if block.M/Low
plugin.go:159— Buffer allocation:if/elsefor small/large →make([]byte, min(size, int64(bufSize))).M/Low
plugin.go:217—for k := range w.hdrToSend { delete(...) }→clear(w.hdrToSend).M/Low
plugin.go:214—w.data = make([]byte, 0, 10)inputWriterdiscards backing array →w.data = w.data[:0].Dependencies
go get -u all && go mod tidy— updated indirect dependencies.