Detach quoted filename strings from pooled buffers#4374
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4374 +/- ##
=======================================
Coverage 91.39% 91.39%
=======================================
Files 132 132
Lines 13098 13098
=======================================
Hits 11971 11971
Misses 710 710
Partials 417 417
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes a correctness bug where App.quoteString() and App.quoteRawString() returned strings backed by pooled bytebufferpool buffers via app.toString (unsafe zero-copy). Once the buffer was returned to the pool and reused by a subsequent call, previously returned filenames could be mutated, leading to corrupted Content-Disposition headers in Attachment()/Download().
Changes:
- Replace
app.toString(...)withstring(...)in both quoting helpers so the returned string is detached from the pooled buffer beforePut. - Drop the now-unused receiver name on
quoteRawString. - Add regression tests asserting earlier results are stable across subsequent quote calls.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| helpers.go | Materialize quoted results into independent Go strings before returning the pooled buffer. |
| helpers_test.go | Add regression tests verifying detachment from the pooled buffer across successive calls. |
Description
quoteString()andquoteRawString()were returning strings backed by pooled buffers. Under concurrent reuse,Attachment()/Download()could emit corruptedContent-Dispositionfilenames.Detachment at return
quoteString()now materializes the quoted result as a Go string before returning the buffer tobytebufferpool.quoteRawString()now returns a copied string instead of a zero-copy alias into pooled storage.Regression coverage
Changes introduced
Core fix
Tests
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md