Skip to content

Honor Discard() in httpFancyWriter.ReadFrom#1110

Merged
VojtechVitek merged 1 commit into
go-chi:masterfrom
DucMinhNe:fix/readfrom-honor-discard
Jun 14, 2026
Merged

Honor Discard() in httpFancyWriter.ReadFrom#1110
VojtechVitek merged 1 commit into
go-chi:masterfrom
DucMinhNe:fix/readfrom-honor-discard

Conversation

@DucMinhNe

Copy link
Copy Markdown
Contributor

httpFancyWriter.ReadFrom only routes through basicWriter.Write when a tee writer is set. When it isn't, it streams straight to the original ResponseWriter's io.ReaderFrom — bypassing the discard flag entirely.

So after a caller invokes Discard() (to suppress the proxied output and write the response itself later), any body sent via ReadFrom/io.Copy — e.g. http.ServeContent, io.Copy(w, file), http.ServeFilestill reaches the client, defeating the purpose of Discard(). The plain Write path already respects discard; only the ReadFrom fast path missed it.

Fix

Extend the slow-path condition to also cover discard, so basicWriter.Write (which honors both tee and discard) handles those cases. This mirrors the existing tee handling in the same function:

if f.basicWriter.tee != nil || f.basicWriter.discard {
    n, err := io.Copy(&f.basicWriter, r)
    return n, err
}

The fast rf.ReadFrom path is now taken only for pure pass-through (no tee, no discard), where it's safe.

Test

Added TestHttpFancyWriterReadFromHonorsDiscard, which writes via ReadFrom after Discard() and asserts the original writer received nothing while BytesWritten() still reflects the consumed bytes. It fails on master (original receives all 11 bytes) and passes with the fix. Full go test ./middleware/ passes; gofmt/go vet clean.

@VojtechVitek VojtechVitek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you

@VojtechVitek VojtechVitek merged commit 2b9fca2 into go-chi:master Jun 14, 2026
8 checks passed
eleboucher pushed a commit to eleboucher/apoci that referenced this pull request Jul 6, 2026
…#132)

This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [github.com/go-chi/chi/v5](https://github.com/go-chi/chi) | `v5.3.0` → `v5.3.1` | ![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-chi%2fchi%2fv5/v5.3.1?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-chi%2fchi%2fv5/v5.3.0/v5.3.1?slim=true) |

---

### Release Notes

<details>
<summary>go-chi/chi (github.com/go-chi/chi/v5)</summary>

### [`v5.3.1`](https://github.com/go-chi/chi/releases/tag/v5.3.1)

[Compare Source](go-chi/chi@v5.3.0...v5.3.1)

#### What's Changed

- Honor Discard() in httpFancyWriter.ReadFrom by [@&#8203;DucMinhNe](https://github.com/DucMinhNe) in [#&#8203;1110](go-chi/chi#1110)
- Tidy build directives by [@&#8203;JRaspass](https://github.com/JRaspass) in [#&#8203;1113](go-chi/chi#1113)
- feat(middleware): add text/xml and application/xml to default compressible types by [@&#8203;VojtechVitek](https://github.com/VojtechVitek) in [#&#8203;1127](go-chi/chi#1127)
- Fix defaultLogEntry.Panic not respecting NoColor setting by [@&#8203;doganarif](https://github.com/doganarif) in [#&#8203;1050](go-chi/chi#1050)
- middleware: document printPrettyStack and harden NoColor panic test by [@&#8203;VojtechVitek](https://github.com/VojtechVitek) in [#&#8203;1131](go-chi/chi#1131)
- feat(mux): support http QUERY method ietf rfc10008 by [@&#8203;VojtechVitek](https://github.com/VojtechVitek) in [#&#8203;1132](go-chi/chi#1132)
- ci: pin GitHub Actions to full commit SHAs by [@&#8203;XananasX7](https://github.com/XananasX7) in [#&#8203;1116](go-chi/chi#1116)

#### New Contributors

- [@&#8203;DucMinhNe](https://github.com/DucMinhNe) made their first contribution in [#&#8203;1110](go-chi/chi#1110)
- [@&#8203;doganarif](https://github.com/doganarif) made their first contribution in [#&#8203;1050](go-chi/chi#1050)
- [@&#8203;XananasX7](https://github.com/XananasX7) made their first contribution in [#&#8203;1116](go-chi/chi#1116)

**Full Changelog**: <go-chi/chi@v5.3.0...v5.3.1>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/apoci/pulls/132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants