fix(http): handle oversized response header in FileCache send path (minimal alt to #823) - #866
Merged
Merged
Conversation
FileCache reserves a fixed block before the file content so header+body can be sent as one buffer. prepend_header() previously returned void and silently did nothing when the header exceeded the reserve (1K): the file was then served with a missing/garbage header and the caller had no way to know. - Bump the reserve from 1K to 4K (covers normal responses incl. long cookies / CORS / CSP headers), so the fast single-buffer path is taken in practice. - prepend_header() now returns bool. When the header still doesn't fit, HttpHandler falls back to sending the header first, then the file content (pResp->content already points at fc->filebuf) via the existing SEND_BODY state -- correct instead of silently broken. Verified: normal static file serve unchanged; a response with a >4K header still delivers both the full header and the intact file body. Minimal alternative to #823 (avoids its LRU/concurrency/config rework).
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
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.
Minimal fix for the FileCache oversized-header problem that #823 targets, without #823's larger rework (LRU/concurrency/config changes).
Problem
FileCachereserves a fixed block before the file content so header+body can be sent as one buffer.prepend_header()returnedvoidand silently did nothing when the header exceeded the reserve (1K) — the file was then served with a missing/garbage header, and the caller couldn't tell.Fix
prepend_header()now returnsbool. When the header still doesn't fit,HttpHandlerfalls back to sending the header first, then the file content (via the existingSEND_BODYstate;pResp->contentalready points atfc->filebuf) — correct instead of silently broken.Verified
Relation to #823
This is a smaller, targeted alternative. It fixes the actual correctness bug (silent header drop) with ~20 lines and no changes to LRU / cache concurrency / configurability. If instance-level configurable reserve/size is wanted later, that can be a separate, focused change.